API Proposal: Expose `System.Drawing.Graphics.NativeGraphics`.

[JeremyKuhne]

Background and Motivation

In Windows Forms painting it would be helpful if we had the option to plug our own P/Invokes calls into existing GDI+ objects. We could, for example, have lighter-weight GpPen and GpBrush wrappers (structs perhaps) and invoke the GDI+ API directly with them via the exposed Graphics pointer.

While we could create our own GpGraphics object that usually isn't practical as we often have to interop via the System.Drawing types.

The form of the API here follows what Icon does (which is a GDI wrapper). Having a static construction method is necessary as we sometimes have derived types that we want to return (SolidBrush, TextureBrush, etc.).

Proposed API

namespace System.Drawing
{
    public class Graphics
    {
+      public IntPtr Handle { get; }
+      public static Graphics FromHandle(IntPtr handle);
    }

    public abstract class Brush
    {
+      public IntPtr Handle { get; }
+      public static Brush FromHandle(IntPtr handle);
    }

    public class Pen
    {
+      public IntPtr Handle { get; }
+      public static Pen FromHandle(IntPtr handle);
    }

    public class Image
    {
+      public IntPtr Handle { get; }
+      public static Image FromHandle(IntPtr handle);
    }

    public class Region
    {
+      public IntPtr Handle { get; }
+      public static Region FromHandle(IntPtr handle);
    }
}

namespace System.Drawing.Drawing2D
{
    public class Matrix
    {
+      public IntPtr Handle { get; }
+      public static Matrix FromHandle(IntPtr handle);
    }

    public class GraphicsPath
    {
+      public IntPtr Handle { get; }
+      public static GraphicsPath FromHandle(IntPtr handle);
    }
}

Notes

• Like Icon.FromHandle, these FromHandle APIs will not take ownership and closing the handle is the caller's responsibility.

Risks

No specific risks outside of what comes normally with accessing backing handles. You can delete the backing object or try to use the handle after the Graphics closes the native handle. Not much different than what you'd be running into using a disposed Graphics.

Note that we already expose IDeviceContext on Graphics which is much more risky as everything on Graphics throws when you're in-between GetHDC() and ReleaseHDC(). The throw there comes from GDI+ as it doesn't want to conflict with the usage of the backing HDC. GDI+ maintains and restores the state of the HDC outside of those calls.

[ghost]

Tagging subscribers to this area: @safern, @tannergooding Notify danmosemsft if you want to be subscribed.

[safern]

@JeremyKuhne given the timeframe I think this would need to be added in 6.0, right?

[JeremyKuhne]

given the timeframe I think this would need to be added in 6.0, right?

Probably. Fyi, I'm investigating the painting loops so I'll likely open more API proposals on System.Drawing.

[JeremyKuhne]

@safern The other thing to consider here is having a public constructor overload that takes the GpGraphics pointer.

[safern]

Sounds reasonable. Feel free to update the proposal to include the new constructor overload if needed.

[tannergooding]

Is NativeGraphics better than Handle which is how (IIRC) WinForms and other similar scenarios expose the "same"?

[tannergooding]

Likewise, are there any other types where it may be beneficial to expose the underlying "handle" and/or support constructing from them?

[JeremyKuhne]

Is NativeGraphics better than Handle which is how (IIRC) WinForms and other similar scenarios expose the "same"?

Handle might be better. I just picked what it is currently called in the proposal.

Likewise, are there any other types where it may be beneficial to expose the underlying "handle" and/or support constructing from them?

Would probably be useful to put on all of the Gp items. Exchanging the drawing surface (Graphics) was the most important.

Here are the other types. Constructing requires a little bit of investigation as we have derived classes. We'd probably have to have static factory methods (which we do have internally in at least some cases).

• Font
• Brush
• Pen
• Image
• Region
• GraphicsPath
• Matrix

Icon is actually a GDI wrapper, not a GDI+ object. It has Handle and FromHandle().

I'll update the proposal above.

[JeremyKuhne]

@tannergooding / @safern Updated the proposal to be more thorough.

[terrajobst]

Video

• Looks good as proposed

namespace System.Drawing
{
    public partial class Graphics
    {
        public IntPtr Handle { get; }
        public static Graphics FromHandle(IntPtr handle);
    }
    public partial class Brush
    {
        public IntPtr Handle { get; }
        public static Brush FromHandle(IntPtr handle);
    }
    public partial class Pen
    {
        public IntPtr Handle { get; }
        public static Pen FromHandle(IntPtr handle);
    }
    public partial class Image
    {
        public IntPtr Handle { get; }
        public static Image FromHandle(IntPtr handle);
    }

    public partial class Region
    {
        public IntPtr Handle { get; }
        public static Region FromHandle(IntPtr handle);
    }
}
namespace System.Drawing.Drawing2D
{
    public partial class Matrix
    {
        public IntPtr Handle { get; }
        public static Matrix FromHandle(IntPtr handle);
    }

    public partial class GraphicsPath
    {
        public IntPtr Handle { get; }
        public static GraphicsPath FromHandle(IntPtr handle);
    }
}

[reflectronic]

We should expose these for CachedBitmap (https://github.com/dotnet/winforms/issues/8822) too.

namespace System.Drawing.Imaging
{
    public partial class CachedBitmap
    {
        public IntPtr Handle { get; }
        public static CachedBitmap FromHandle(IntPtr handle);
    }
}

[safern]

@JeremyKuhne are you planning on working on this one or should we move it to 7.0.0?

[teo-tsirpanis]

How about making the Handle properties return SafeHandles?

[JeremyKuhne]

How about making the Handle properties return SafeHandles?

Good question. Doing so would have performance implications (finalizable objects are expensive to create) for the scenarios this is intended for (advanced interop). It would also be a bit difficult to express ownership in these cases as the originating class actually owns the handle. We could ref count, but that also adds weight.

When I implement this I'll make sure to be very explicit about expectations with the handle ownership.

[JeremyKuhne]

@safern sorry about not responding here, I was distracted with Jury Duty. 7 is fine. :)

[deeprobin]

You're welcome to assign me :smile:

[danmoseley]

@deeprobin assigned, thanks. We should try to reduce your backlog of open PR's first though. Are all except the draft ones waiting on us?

[deeprobin]

@danmoseley Thank you. Thats right. Most prs waiting :/

[danmoseley]

OK. Yeah, this is the slowest time of the year so folks mostly won't be back until Jan. I reviewed your 2nd one, if you address feedback we can merge that. The others I should wait for area experts.

I don't have a problem with you throwing up more PR's meantime, when you become entirely blocked on us 😄

[deeprobin]

OK. Yeah, this is the slowest time of the year so folks mostly won't be back until Jan. I reviewed your 2nd one, if you address feedback we can merge that. The others I should wait for area experts.

I don't have a problem with you throwing up more PR's meantime, when you become entirely blocked on us 😄

Perfect. I should just not get bogged down when then comes the mass of reviews in the new year 😄

[teo-tsirpanis]

Is it decided that we won't expose them as SafeHandles? I am interested in moving the interop code away from HandleRefs to help with the DllImport source generator and to address @JeremyKuhne's concerns about handle ownership (it will be SafeHandles all the way in, like with FileStream). If we go that way, we would also need to add public SafeHandle derivatives such as SafeGraphicsHandle or SafeBrushHandle.

[jkoritzinsky]

I would be in support of @teo-tsirpanis’s proposal to move to SafeHandle-based interop.

[danmoseley]

@deeprobin going through old issues. do you plan to continue with this one?

[deeprobin]

@danmoseley System.Drawing will probably die in the long run anyway.

In the PR it is only about testfailures that fail due to a double-dispose. Since they are 2 objects (the initial object and the by-handle object), the finalizer will always implicitly call Dispose, so an exception will be thrown in any case. GdipDeleteRegion is for ex. not safe to call twice 😞.

Basically the implementation of the PR is ready. Either we leave out the tests or we find an easy way to test this anyway.

Very happy to work on the PR some more, unless it will be such a big effort that it won't be worth it in the long run.

[teo-tsirpanis]

I am working on porting System.Drawing.Common's interop layer to SafeHandles. Here's an alternative proposal based on that:

namespace System.Drawing
{
    public class Graphics
    {
      public SafeGraphicsHandle SafeGraphicsHandle { get; }
      public Graphics(SafeGraphicsHandle handle);
    }

    public abstract class Brush
    {
      public SafeBrushHandle SafeBrushHandle { get; }
      public static Brush FromHandle(SafeBrushHandle handle); // Creates an internal concrete descendant of Brush.
    }

    public class Pen
    {
      public SafePenHandle SafePenHandle { get; }
      public Pen(SafePenHandle handle);
    }

    public class Image
    {
      public SafeImageHandle SafeImageHandle { get; }
      public Image(SafeImageHandle handle);
    }

    public class Region
    {
      public SafeRegionHandle SafeRegionHandle { get; }
      public Region(SafeRegionHandle handle);
    }
}

namespace System.Drawing.Drawing2D
{
    public class Matrix
    {
      public SafeMatrixHandle SafeMatrixHandle { get; }
      public Matrix(SafeMatrixHandle handle);
    }

    public class GraphicsPath
    {
      public SafeGraphicsPathHandle SafeGraphicsPathHandle { get; }
      public GraphicsPath(SafeGraphicsPathHandle handle);
    }
}

namespace Microsoft.Win32.SafeHandles
{
    public abstract class SafeGdiPlusHandle : SafeHandle { }
    public abstract class SafeGraphicsHandle : SafeGdiPlusHandle { }
    public abstract class SafeBrushHandle : SafeGdiPlusHandle { }
    public abstract class SafePenHandle : SafeGdiPlusHandle { }
    public abstract class SafeImageHandle : SafeGdiPlusHandle { }
    public abstract class SafeRegionHandle : SafeGdiPlusHandle { }
    public abstract class SafeMatrixHandle : SafeGdiPlusHandle { }
    public abstract class SafeGraphicsPathHandle : SafeGdiPlusHandle { }
}

@JeremyKuhne what do you think?

[danmoseley]

System.Drawing will probably die in the long run anyway.

I would rather think of it as in maintenance mode. It has not had new features for a long time. For someone that has existing code that uses it, or wants to do something simple and perhaps already depend on the Windows compatibility pack, it is a reasonable choice.

[JeremyKuhne]

I would rather think of it as in maintenance mode.

It won't go anywhere for a long time as Windows Forms is still a heavy user (there are many, many active WinForms developers). This particular API I still plan to implement when I have bandwidth to consume the changes in the Windows Forms runtime.

@teo-tsirpanis There is measurable overhead to SafeHandle and the intent for this API is allow for inner-loop optimizations in WinForms rendering (skipping straight to native GDI+ exports without additional managed heap allocations).

Doing this internally would add overhead I'm uncomfortable with.

[deeprobin]

I think this issue should be taken over by someone else with more experience in System.Drawing.

@teo-tsirpanis Would you like to take over the issue?

[teo-tsirpanis]

OK, it looks relatively simple, I will do it at some point in the 8.0.0 timeframe, assigning myself to not forget it.

I have also abandoned my experiments with using SafeHandles. There are performance concerns and SDC is not that of a high-activity part of the BCL either way.