Cleanup GDI interop in System.Drawing.Common #39727
Conversation
|
@hughbe could you please resolve conflicts then ping Jeremy? |
|
@JeremyKuhne ready for your eyes :) |
There was a problem hiding this comment.
Thanks for doing this! I left some comments on existing code that would be nice to clean up, but I don't think it's blocking. The try blocks in particular would be good to clean up, but can be done in another change. It would be nice to make the arguments for the imports match the current documentation, but again, I don't think it is blocking.
| internal static partial class Gdi32 | ||
| { | ||
| [DllImport(Libraries.Gdi32, ExactSpelling = true, CharSet = CharSet.Unicode)] | ||
| public static extern IntPtr CreateFontIndirectW(ref User32.LOGFONT lf); |
| internal static partial class Gdi32 | ||
| { | ||
| [DllImport(Libraries.Gdi32, ExactSpelling = true, CharSet = CharSet.Unicode)] | ||
| public static extern IntPtr CreateICW(string lpszDriverName, string lpszDeviceName, string lpszOutput, IntPtr lpInitData); |
There was a problem hiding this comment.
nit: arguments are pszDriver, pszDevice, pszPort, pdm
| internal static partial class Gdi32 | ||
| { | ||
| [DllImport(Libraries.Gdi32, ExactSpelling = true)] | ||
| public static extern bool DeleteObject(IntPtr hObject); |
| } | ||
|
|
||
| [DllImport(Libraries.Gdi32, ExactSpelling = true)] | ||
| public static extern IntPtr GetStockObject(StockObject nIndex); |
| { | ||
| IntPtr hTempRgn = Interop.Gdi32.CreateRectRgn(0, 0, 0, 0); | ||
| IntPtr hSaveRgn = IntPtr.Zero; | ||
| try |
There was a problem hiding this comment.
The try..finally here isn't useful. There would have to be some catastrophic failure (no memory maybe) and at that point a dangled GDI object isn't a terrible thing, if we could manage to delete it anyway.
| { | ||
| try | ||
| { | ||
| Interop.Gdi32.SelectClipRgn(new HandleRef(null, hDC), new HandleRef(null, hRgn)); |
There was a problem hiding this comment.
HandleRef with a null is pointless. The try block doesn't help here either as it shouldn't throw "normally" (see the comment above).
If these methods are only called in one place it might be better to move them inline.
| { | ||
| return Interop.Gdi32.GetDeviceCaps(new HandleRef(dc, dc.Hdc), capability); | ||
| } | ||
| catch (InvalidPrinterException) |
There was a problem hiding this comment.
This can't happen here- we should remove the try..catch.
| } | ||
| #if TRACK_HDC | ||
| Debug.WriteLine( DbgUtil.StackTraceToStr( string.Format("DeviceContext( hDC=0x{0:X8}, Type={1} )", unchecked((int) hDC), dcType) )); | ||
| Debug.WriteLine(DbgUtil.StackTraceToStr($"DeviceContext(hDC=0x{unchecked((int)hDC):X8}, Type={dcType})")); |
|
I pushed to address feedback and unblock this PR. |
* Cleanup GDI object Interop * Cleanup GDI region interop * Cleanup GDI font interop * Cleanup GDI DC interop * Address PR feedback Commit migrated from dotnet/corefx@f807df6
No description provided.