Fix getting existing region in RegionScope (#3525)

src/System.Windows.Forms.Primitives/src/Interop/Gdi32/Interop.HRGN.cs

@@ -14,6 +14,8 @@ internal static partial class Interop
 
             public HRGN(IntPtr handle) => Handle = handle;
 
+            public bool IsNull => Handle == IntPtr.Zero;
+
             public static implicit operator HGDIOBJ(HRGN hrgn) => new HGDIOBJ(hrgn.Handle);
         }
     }

src/System.Windows.Forms.Primitives/src/Interop/Gdi32/Interop.RegionScope.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Diagnostics;
 using System.Drawing;
 
 internal static partial class Interop
@@ -21,7 +22,7 @@ internal static partial class Interop
             public HRGN Region { get; private set; }
 
             /// <summary>
-            ///  Creates a region with the given rectangle.
+            ///  Creates a region with the given rectangle via <see cref="CreateRectRgn(int, int, int, int)"/>.
             /// </summary>
             public RegionScope(Rectangle rectangle)
             {
@@ -29,7 +30,7 @@ internal static partial class Interop
             }
 
             /// <summary>
-            ///  Creates a region with the given rectangle.
+            ///  Creates a region with the given rectangle via <see cref="CreateRectRgn(int, int, int, int)"/>.
             /// </summary>
             public RegionScope(int x1, int y1, int x2, int y2)
             {
@@ -37,14 +38,25 @@ internal static partial class Interop
             }
 
             /// <summary>
-            ///  Creates a clipping region copy for the given device context.
+            ///  Creates a clipping region copy via <see cref="GetClipRgn(IntPtr, HRGN)"/> for the given device context.
             /// </summary>
             /// <param name="hdc">Handle to a device context to copy the clipping region from.</param>
             public RegionScope(IntPtr hdc)
             {
-                HRGN region = default;
-                GetClipRgn(hdc, region);
-                Region = region;
+                HRGN region = CreateRectRgn(0, 0, 0, 0);
+                int result = GetClipRgn(hdc, region);
+                Debug.Assert(result != -1, "GetClipRgn failed");
+
+                if (result == 1)
+                {
+                    Region = region;
+                }
+                else
+                {
+                    // No region, delete our temporary region
+                    DeleteObject(region);
+                    Region = default;
+                }
             }
 
             /// <summary>

src/System.Windows.Forms.Primitives/tests/UnitTests/Interop/Gdi32/RegionTests.cs

@@ -0,0 +1,115 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Xunit;
+using static Interop.Gdi32;
+using System.Drawing;
+using static Interop;
+
+namespace System.Windows.Forms.Tests.InteropTests
+{
+    public class RegionTests
+    {
+        [Fact]
+        public void GetClipRgn_NoRegion()
+        {
+            // Create a bitmap using the screen's stats
+            IntPtr hdc = CreateCompatibleDC(IntPtr.Zero);
+            Assert.NotEqual(IntPtr.Zero, hdc);
+
+            try
+            {
+                IntPtr hbitmap = CreateCompatibleBitmap(hdc, 20, 20);
+                Assert.NotEqual(IntPtr.Zero, hbitmap);
+
+                try
+                {
+                    SelectObject(hdc, hbitmap);
+                    HRGN hregion = CreateRectRgn(0, 0, 0, 0);
+
+                    Assert.False(hregion.IsNull);
+                    try
+                    {
+                        int result = GetClipRgn(hdc, hregion);
+
+                        // We should have no clipping region
+                        Assert.Equal(0, result);
+                    }
+                    finally
+                    {
+                        DeleteObject(hregion);
+                    }
+                }
+                finally
+                {
+                    DeleteObject(hbitmap);
+                }
+            }
+            finally
+            {
+                DeleteDC(hdc);
+            }
+        }
+
+        [Fact]
+        public void RegionScope_NullWithNoClippingRegion()
+        {
+            // Create a bitmap using the screen's stats
+            IntPtr hdc = CreateCompatibleDC(IntPtr.Zero);
+            Assert.NotEqual(IntPtr.Zero, hdc);
+
+            try
+            {
+                IntPtr hbitmap = CreateCompatibleBitmap(hdc, 20, 20);
+                Assert.NotEqual(IntPtr.Zero, hbitmap);
+                try
+                {
+                    using var hregion = new RegionScope(hdc);
+                    Assert.True(hregion.IsNull);
+                }
+                finally
+                {
+                    DeleteObject(hbitmap);
+                }
+            }
+            finally
+            {
+                DeleteDC(hdc);
+            }
+        }
+
+        [Fact]
+        public void RegionScope_GetRegion()
+        {
+            // Create a bitmap using the screen's stats
+            IntPtr hdc = CreateCompatibleDC(IntPtr.Zero);
+            Assert.NotEqual(IntPtr.Zero, hdc);
+
+            try
+            {
+                IntPtr hbitmap = CreateCompatibleBitmap(hdc, 20, 20);
+                Assert.NotEqual(IntPtr.Zero, hbitmap);
+                try
+                {
+                    Rectangle rectangle = new Rectangle(1, 2, 3, 4);
+                    using var originalRegion = new RegionScope(rectangle);
+                    SelectClipRgn(hdc, originalRegion);
+                    using var retrievedRegion = new RegionScope(hdc);
+                    RECT rect = default;
+                    RegionType type = GetRgnBox(retrievedRegion, ref rect);
+                    Assert.Equal(RegionType.SIMPLEREGION, type);
+                    Assert.Equal(rectangle, (Rectangle)rect);
+                }
+                finally
+                {
+                    DeleteObject(hbitmap);
+                }
+            }
+            finally
+            {
+                DeleteDC(hdc);
+            }
+        }
+    }
+}

src/System.Windows.Forms/src/System/Windows/Forms/DCMapping.cs

@@ -19,27 +19,12 @@ namespace System.Windows.Forms
     ///
     ///  Example:
     ///
-    ///  using(DCMapping mapping = new DCMapping(hDC, new Rectangle(10,10, 50, 50) {
-    ///  // inside here the hDC's mapping of (0,0) is inset by (10,10) and
-    ///  // all painting is clipped at (0,0) - (50,50)
+    ///  using(DCMapping mapping = new DCMapping(hDC, new Rectangle(10,10, 50, 50)
+    ///  {
+    ///      // inside here the hDC's mapping of (0,0) is inset by (10,10) and
+    ///      // all painting is clipped at (0,0) - (50,50)
     ///  }
     ///
-    ///  To use with GDI+ you can get the hDC from the Graphics object. You'd want to do this in a situation where
-    ///  you're handing off a graphics object to someone, and you want the world translated some amount X,Y. This
-    ///  works better than g.TranslateTransform(x,y) - as if someone calls g.GetHdc and does a GDI operation - their
-    ///  world is NOT transformed.
-    ///
-    ///  HandleRef hDC = new HandleRef(this, originalGraphics.GetHdc());
-    ///  try {
-    ///  using(DCMapping mapping = new DCMapping(hDC, new Rectangle(10,10, 50, 50) {
-    ///
-    ///  // DO NOT ATTEMPT TO USE originalGraphics here - you'll get an Object Busy error
-    ///  // rather ask the mapping object for a graphics object.
-    ///  mapping.Graphics.DrawRectangle(Pens.Black, rect);
-    ///  }
-    ///  }
-    ///  finally { g.ReleaseHdc(hDC);}
-    ///
     ///  PERF: DCMapping is a structure so that it will allocate on the stack rather than in GC managed
     ///  memory. This way disposing the object does not force a GC. Since DCMapping objects aren't
     ///  likely to be passed between functions rather used and disposed in the same one, this reduces
@@ -48,8 +33,6 @@ namespace System.Windows.Forms
     internal struct DCMapping : IDisposable
     {
         private DeviceContext _dc;
-        private Graphics _graphics;
-        private Rectangle _translatedBounds;
 
         public unsafe DCMapping(IntPtr hDC, Rectangle bounds)
         {
@@ -60,8 +43,6 @@ namespace System.Windows.Forms
 
             bool success;
 
-            _translatedBounds = bounds;
-            _graphics = null;
             _dc = DeviceContext.FromHdc(hDC);
             _dc.SaveHdc();
 
@@ -79,13 +60,7 @@ namespace System.Windows.Forms
 
             try
             {
-                // Create an empty region oriented at 0,0 so we can populate it with the original clipping region of the hDC passed in.
-                var hOriginalClippingRegion = new Gdi32.RegionScope(0, 0, 0, 0);
-                Debug.Assert(!hOriginalClippingRegion.IsNull, "CreateRectRgn() failed.");
-
-                // Get the clipping region from the hDC: result = {-1 = error, 0 = no region, 1 = success} per MSDN
-                int result = Gdi32.GetClipRgn(hDC, hOriginalClippingRegion);
-                Debug.Assert(result != -1, "GetClipRgn() failed.");
+                var hOriginalClippingRegion = new Gdi32.RegionScope(hDC);
 
                 // Shift the viewpoint origint by coordinates specified in "bounds".
                 var lastViewPort = new Point();
@@ -93,7 +68,7 @@ namespace System.Windows.Forms
                 Debug.Assert(success, "SetViewportOrgEx() failed.");
 
                 RegionType originalRegionType;
-                if (result != 0)
+                if (!hOriginalClippingRegion.IsNull)
                 {
                     // Get the origninal clipping region so we can determine its type (we'll check later if we've restored the region back properly.)
                     RECT originalClipRect = new RECT();
@@ -138,16 +113,6 @@ namespace System.Windows.Forms
 
         public void Dispose()
         {
-            if (_graphics != null)
-            {
-                // Reset GDI+ if used.
-                // we need to dispose the graphics object first, as it will do
-                // some restoration to the ViewPort and ClipRectangle to restore the hDC to
-                // the same state it was created in
-                _graphics.Dispose();
-                _graphics = null;
-            }
-
             if (_dc != null)
             {
                 // Now properly reset GDI.
@@ -156,25 +121,5 @@ namespace System.Windows.Forms
                 _dc = null;
             }
         }
-
-        /// <summary>
-        ///  Allows you to get the graphics object based off of the translated HDC.
-        ///  Note this will be disposed when the DCMapping object is disposed.
-        /// </summary>
-        public Graphics Graphics
-        {
-            get
-            {
-                Debug.Assert(_dc != null, "unexpected null dc!");
-
-                if (_graphics == null)
-                {
-                    _graphics = Graphics.FromHdcInternal(_dc.Hdc);
-                    _graphics.SetClip(new Rectangle(Point.Empty, _translatedBounds.Size));
-                }
-
-                return _graphics;
-            }
-        }
     }
 }