[Win32] Represent GC#getClipping(Region) operation in Region #2346

The GC#getClipping(Region) call sets the passed region to the current
clipping region of the GC. Currently, this is not represented as an
operation inside the passed Region, such that when calculating a handle
for a different zoom of that region or when copying that region the
according application of GC#getClipping(Region) to that region is
missing.

This change adds the missing operation to the Region. Since the clipping
needs to be calculated at the of executing the GC#getClipping(Region)
method and not at the time of applying the region operation when
retrieving a handle, the GC#getClipping(Region) stores an operation
inside GC that maintains handles for the according clipping region at
the time of executing the GC#getClipping(Region) method for every zoom
at which the GC is requested. This ensures that the proper clipping
state is available to the Region.

Fixes #2346

Author: HeikoKlare

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java

@@ -3790,6 +3790,44 @@ public void getClipping (Region region) {
 	checkNonDisposed();
 	if (region == null) SWT.error (SWT.ERROR_NULL_ARGUMENT);
 	if (region.isDisposed()) SWT.error (SWT.ERROR_INVALID_ARGUMENT);
+	storeAndApplyOperationForExistingHandle(new GetClippingOperation(region));
+}
+
+private class GetClippingOperation extends Operation {
+	private final Map<Integer, Long> zoomToRegionHandle = new HashMap<>();
+
+	public GetClippingOperation(Region region) {
+		region.set(zoom -> {
+			if (!zoomToRegionHandle.containsKey(zoom)) {
+				System.err.println("No clipping handle for zoom " + zoom + " has been created on this GC");
+				return zoomToRegionHandle.values().iterator().next();
+			}
+			return zoomToRegionHandle.get(zoom);
+		}, getZoom());
+	}
+
+	// Whenever the GC handle is recalculated for a new zoom, we compute and store the clipping
+	// at the times when getClipping(Region) was originally called, such that the region to which
+	// that clipping is set can retrieve it from the storage when required.
+	@Override
+	void apply() {
+		zoomToRegionHandle.computeIfAbsent(getZoom(), __ -> getClippingRegion());
+	}
+
+	@Override
+	void disposeAll() {
+		for (long handle : zoomToRegionHandle.values()) {
+			OS.DeleteObject(handle);
+		}
+		super.disposeAll();
+	}
+}
+
+/**
+ * @return a region handle with the current clipping region of this GC
+ */
+private long getClippingRegion () {
+	long regionHandle = OS.CreateRectRgn(0, 0, 0, 0);
 	long gdipGraphics = data.gdipGraphics;
 	if (gdipGraphics != 0) {
 		long rgn = Gdip.Region_new();
@@ -3799,7 +3837,7 @@ public void getClipping (Region region) {
 			Gdip.Graphics_SetPixelOffsetMode(gdipGraphics, Gdip.PixelOffsetModeNone);
 			Gdip.Graphics_GetVisibleClipBounds(gdipGraphics, rect);
 			Gdip.Graphics_SetPixelOffsetMode(gdipGraphics, Gdip.PixelOffsetModeHalf);
-			OS.SetRectRgn(Region.win32_getHandle(region, getZoom()), rect.X, rect.Y, rect.X + rect.Width, rect.Y + rect.Height);
+			OS.SetRectRgn(regionHandle, rect.X, rect.Y, rect.X + rect.Width, rect.Y + rect.Height);
 		} else {
 			long matrix = Gdip.Matrix_new(1, 0, 0, 1, 0, 0);
 			long identity = Gdip.Matrix_new(1, 0, 0, 1, 0, 0);
@@ -3812,26 +3850,26 @@ public void getClipping (Region region) {
 			POINT pt = new POINT ();
 			OS.GetWindowOrgEx (handle, pt);
 			OS.OffsetRgn (hRgn, pt.x, pt.y);
-			OS.CombineRgn(Region.win32_getHandle(region, getZoom()), hRgn, 0, OS.RGN_COPY);
+			OS.CombineRgn(regionHandle, hRgn, 0, OS.RGN_COPY);
 			OS.DeleteObject(hRgn);
 		}
 		Gdip.Region_delete(rgn);
-		return;
+		return regionHandle;
 	}
 	POINT pt = new POINT ();
 	OS.GetWindowOrgEx (handle, pt);
-	int result = OS.GetClipRgn (handle, Region.win32_getHandle(region, getZoom()));
+	int result = OS.GetClipRgn (handle, regionHandle);
 	if (result != 1) {
 		RECT rect = new RECT();
 		OS.GetClipBox(handle, rect);
-		OS.SetRectRgn(Region.win32_getHandle(region, getZoom()), rect.left, rect.top, rect.right, rect.bottom);
+		OS.SetRectRgn(regionHandle, rect.left, rect.top, rect.right, rect.bottom);
 	} else {
-		OS.OffsetRgn (Region.win32_getHandle(region, getZoom()), pt.x, pt.y);
+		OS.OffsetRgn (regionHandle, pt.x, pt.y);
 	}
 	long metaRgn = OS.CreateRectRgn (0, 0, 0, 0);
 	if (OS.GetMetaRgn (handle, metaRgn) != 0) {
 		OS.OffsetRgn (metaRgn, pt.x, pt.y);
-		OS.CombineRgn (Region.win32_getHandle(region, getZoom()), metaRgn, Region.win32_getHandle(region, getZoom()), OS.RGN_AND);
+		OS.CombineRgn (regionHandle, metaRgn, regionHandle, OS.RGN_AND);
 	}
 	OS.DeleteObject(metaRgn);
 	long hwnd = data.hwnd;
@@ -3848,10 +3886,11 @@ public void getClipping (Region region) {
 			}
 			OS.MapWindowPoints (0, hwnd, pt, 1);
 			OS.OffsetRgn (sysRgn, pt.x, pt.y);
-			OS.CombineRgn (Region.win32_getHandle(region, getZoom()), sysRgn, Region.win32_getHandle(region, getZoom()), OS.RGN_AND);
+			OS.CombineRgn (regionHandle, sysRgn, regionHandle, OS.RGN_AND);
 		}
 		OS.DeleteObject(sysRgn);
 	}
+	return regionHandle;
 }
 
 long getFgBrush() {

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Region.java

@@ -43,6 +43,8 @@ public final class Region extends Resource {
 
 	private boolean isDestroyed;
 
+	private int temporaryHandleZoomHint = 0;
+
 /**
  * Constructs a new empty region.
  * <p>
@@ -171,11 +173,18 @@ public void add (Region region) {
 	if (region == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (region.isDisposed()) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
 	if (!region.operations.isEmpty()) {
+		adoptTemporaryHandleZoomHint(region);
 		final Operation operation = new OperationWithRegion(Operation::add, region.operations);
 		storeAndApplyOperationForAllHandles(operation);
 	}
 }
 
+private void adoptTemporaryHandleZoomHint(Region region) {
+	if (temporaryHandleZoomHint == 0 && region.temporaryHandleZoomHint != 0) {
+		this.temporaryHandleZoomHint = region.temporaryHandleZoomHint;
+	}
+}
+
 /**
  * Returns <code>true</code> if the point specified by the
  * arguments is inside the area specified by the receiver,
@@ -373,6 +382,7 @@ public void intersect (Region region) {
 	if (region == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (region.isDisposed()) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
 	if (!region.operations.isEmpty()) {
+		adoptTemporaryHandleZoomHint(region);
 		final Operation operation = new OperationWithRegion(Operation::intersect, region.operations);
 		storeAndApplyOperationForAllHandles(operation);
 	}
@@ -468,6 +478,21 @@ public boolean isEmpty () {
 	});
 }
 
+/**
+ * Specific method for {@link GC#getClipping(Region)} because the current GC
+ * clipping settings at that specific point in time of executing the getClipping
+ * method need to be stored.
+ * <p>
+ * The context zoom is used as a hint for the case of creating temporary
+ * handles, such that they can be created for a zoom for which we know that the
+ * supplier is capable of providing a proper handle.
+ */
+void set(Function<Integer, Long> handleForZoomSupplier, int contextZoom) {
+	this.temporaryHandleZoomHint = contextZoom;
+	final Operation operation = new OperationWithRegionHandle(Operation::set, handleForZoomSupplier);
+	storeAndApplyOperationForAllHandles(operation);
+}
+
 /**
  * Subtracts the given polygon from the collection of polygons
  * the receiver maintains to describe its area.
@@ -559,6 +584,7 @@ public void subtract (Region region) {
 	if (region == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (region.isDisposed()) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
 	if (!region.operations.isEmpty()) {
+		adoptTemporaryHandleZoomHint(region);
 		final Operation operation = new OperationWithRegion(Operation::subtract, region.operations);
 		storeAndApplyOperationForAllHandles(operation);
 	}
@@ -612,7 +638,8 @@ private void storeAndApplyOperationForAllHandles(Operation operation) {
 
 private <T> T applyUsingAnyHandle(Function<RegionHandle, T> function) {
 	if (zoomToHandle.isEmpty()) {
-		return applyUsingTemporaryHandle(device.getDeviceZoom(), operations, function);
+		int temporaryHandleZoom = temporaryHandleZoomHint != 0 ? temporaryHandleZoomHint : device.getDeviceZoom();
+		return applyUsingTemporaryHandle(temporaryHandleZoom, operations, function);
 	}
 	return function.apply(zoomToHandle.values().iterator().next());
 }
@@ -646,6 +673,7 @@ private RegionHandle getRegionHandle(int zoom) {
 
 Region copy() {
 	Region region = new Region();
+	region.temporaryHandleZoomHint = temporaryHandleZoomHint;
 	region.operations.addAll(operations);
 	return region;
 }
@@ -705,6 +733,8 @@ void apply(RegionHandle regionHandle) {
 		operationStrategy.apply(this, regionHandle.handle(), regionHandle.zoom());
 	}
 
+	abstract void set(long handle, int zoom);
+
 	abstract void add(long handle, int zoom);
 
 	abstract void subtract(long handle, int zoom);
@@ -722,6 +752,11 @@ private static class OperationWithRectangle extends Operation {
 		this.data = data;
 	}
 
+	@Override
+	void set(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
 	@Override
 	void add(long handle, int zoom) {
 		Rectangle bounds = getScaledRectangle(zoom);
@@ -780,6 +815,11 @@ public OperationWithArray(OperationStrategy operationStrategy, int[] data) {
 		this.data = data;
 	}
 
+	@Override
+	void set(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
 	@Override
 	void add(long handle, int zoom) {
 		int[] points = getScaledPoints(zoom);
@@ -827,6 +867,11 @@ public OperationWithPoint(OperationStrategy operationStrategy, Point data) {
 		this.data = data;
 	}
 
+	@Override
+	void set(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
 	@Override
 	void add(long handle, int zoom) {
 		throw new UnsupportedOperationException();
@@ -858,6 +903,11 @@ private static class OperationWithRegion extends Operation {
 		this.operations = List.copyOf(operations);
 	}
 
+	@Override
+	void set(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
 	@Override
 	void add(long handle, int zoom) {
 		applyUsingTemporaryHandle(zoom, operations, regionHandle -> {
@@ -883,5 +933,41 @@ void intersect(long handle, int zoom) {
 	void translate(long handle, int zoom) {
 		throw new UnsupportedOperationException();
 	}
+
+}
+
+private static class OperationWithRegionHandle extends Operation {
+	private final Function<Integer, Long> handleForZoomProvider;
+
+	OperationWithRegionHandle(OperationStrategy operationStrategy, Function<Integer, Long> handleForZoomSupplier) {
+		super(operationStrategy);
+		this.handleForZoomProvider = handleForZoomSupplier;
+	}
+
+	@Override
+	void set(long handle, int zoom) {
+		OS.CombineRgn(handle, handleForZoomProvider.apply(zoom), 0, OS.RGN_COPY);
+	}
+
+	@Override
+	void subtract(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	void intersect(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	void translate(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	void add(long handle, int zoom) {
+		throw new UnsupportedOperationException();
+	}
+
 }
 }