[win32] Use operations for transform manipulation

This commit refactors Transform in the win32 implementation to better
support multiple handles for different zoom settings of a Transform when
monitor-specific scaling is enabled. The previous implementation
only applied adaptions to the initial handle of the transform and relied
on the transform not to be changed afterwards. This doesn't cover all
scenarios and can lead to unexpected behavior when re-using Transform
objects over different zoom settings.

Author: akoch-yatta

bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/TransformWin32Tests.java

@@ -40,15 +40,21 @@ public void testShouldHaveDifferentHandlesAtDifferentZoomLevels() {
 	@Test
 	public void testScaledTrasformMustHaveScaledValues() {
 		Display display = Display.getDefault();
-		int zoom = DPIUtil.getDeviceZoom();
 		Transform transform = new Transform(display, 0, 0, 0, 0, 4, 2);
 		float[] elements = new float[6];
 		transform.getElements(elements);
-		long scaledHandle = transform.getHandle(zoom * 2);
-		float[] scaledElements = new float[6];
-		Gdip.Matrix_GetElements(scaledHandle, scaledElements);
-		assertEquals(elements[4] * 2, scaledElements[4], 0);
-		assertEquals(elements[5] * 2, scaledElements[5], 0);
+
+		long handle200 = transform.getHandle(200);
+		float[] scaledElements200 = new float[6];
+		Gdip.Matrix_GetElements(handle200, scaledElements200);
+		assertEquals(elements[4] * 2, scaledElements200[4], 0);
+		assertEquals(elements[5] * 2, scaledElements200[5], 0);
+
+		long handle300 = transform.getHandle(300);
+		float[] scaledElements300 = new float[6];
+		Gdip.Matrix_GetElements(handle300, scaledElements300);
+		assertEquals(elements[4] * 3, scaledElements300[4], 0);
+		assertEquals(elements[5] * 3, scaledElements300[5], 0);
 	}
 
 }

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

@@ -41,7 +41,9 @@ public class Transform extends Resource {
 
 	private int initialZoom;
 
-	private HashMap<Integer, Long> zoomLevelToHandle = new HashMap<>();
+	private Map<Integer, TransformHandle> zoomToHandle = new HashMap<>();
+
+	private List<Operation> operations = new ArrayList<>();
 
 /**
  * Constructs a new identity Transform.
@@ -143,7 +145,7 @@ public Transform (Device device, float m11, float m12, float m21, float m22, flo
 	long handle = Gdip.Matrix_new(m11, m12, m21, m22,
 			DPIUtil.scaleUp(this.device, dx, initialZoom), DPIUtil.scaleUp(this.device, dy, initialZoom));
 	if (handle == 0) SWT.error(SWT.ERROR_NO_HANDLES);
-	zoomLevelToHandle.put(initialZoom, handle);
+	zoomToHandle.put(initialZoom, new TransformHandle(handle, initialZoom));
 	init();
 	this.device.registerResourceWithZoomSupport(this);
 }
@@ -157,16 +159,17 @@ static float[] checkTransform(float[] elements) {
 @Override
 void destroy() {
 	device.deregisterResourceWithZoomSupport(this);
-	zoomLevelToHandle.values().forEach(Gdip::Matrix_delete);
-	zoomLevelToHandle.clear();
+	zoomToHandle.values().forEach(TransformHandle::destroy);
+	zoomToHandle.clear();
+	operations.clear();
 }
 
 @Override
 void destroyHandlesExcept(Set<Integer> zoomLevels) {
-	zoomLevelToHandle.entrySet().removeIf(entry -> {
+	zoomToHandle.entrySet().removeIf(entry -> {
 		final Integer zoom = entry.getKey();
 		if (!zoomLevels.contains(zoom) && zoom != initialZoom) {
-			Gdip.Matrix_delete(entry.getValue());
+			entry.getValue().destroy();
 			return true;
 		}
 		return false;
@@ -191,10 +194,12 @@ public void getElements(float[] elements) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
 	if (elements == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (elements.length < 6) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
-	Gdip.Matrix_GetElements(getHandle(initialZoom), elements);
+	TransformHandle transformHandle = getTransformHandle(initialZoom);
+	Gdip.Matrix_GetElements(transformHandle.handle, elements);
 	Drawable drawable = getDevice();
-	elements[4] = DPIUtil.scaleDown(drawable, elements[4], initialZoom);
-	elements[5] = DPIUtil.scaleDown(drawable, elements[5], initialZoom);
+	int zoom = transformHandle.zoom;
+	elements[4] = DPIUtil.scaleDown(drawable, elements[4], zoom);
+	elements[5] = DPIUtil.scaleDown(drawable, elements[5], zoom);
 }
 
 /**
@@ -209,7 +214,9 @@ public void getElements(float[] elements) {
  */
 public void identity() {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Gdip.Matrix_SetElements(getHandle(initialZoom), 1, 0, 0, 1, 0, 0);
+	// identity invalidates all previous operations, so we remove them
+	operations.clear();
+	storeAndApplyOperationForAllHandles(new SetElementsOperation(1, 0, 0, 1, 0, 0));
 }
 
 /**
@@ -223,7 +230,7 @@ public void identity() {
  */
 public void invert() {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	if (Gdip.Matrix_Invert(getHandle(initialZoom)) != 0) SWT.error(SWT.ERROR_CANNOT_INVERT_MATRIX);
+	storeAndApplyOperationForAllHandles(new InvertOperation());
 }
 
 /**
@@ -238,7 +245,7 @@ public void invert() {
  */
 @Override
 public boolean isDisposed() {
-	return zoomLevelToHandle.isEmpty();
+	return zoomToHandle.isEmpty();
 }
 
 /**
@@ -249,7 +256,8 @@ public boolean isDisposed() {
  */
 public boolean isIdentity() {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	return Gdip.Matrix_IsIdentity(getHandle(initialZoom));
+	TransformHandle transformHandle = getTransformHandle(initialZoom);
+	return Gdip.Matrix_IsIdentity(transformHandle.handle);
 }
 
 /**
@@ -271,7 +279,7 @@ public void multiply(Transform matrix) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
 	if (matrix == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
 	if (matrix.isDisposed()) SWT.error(SWT.ERROR_INVALID_ARGUMENT);
-	Gdip.Matrix_Multiply(getHandle(initialZoom), matrix.getHandle(initialZoom), Gdip.MatrixOrderPrepend);
+	storeAndApplyOperationForAllHandles(new MultiplyOperation(matrix));
 }
 
 /**
@@ -289,7 +297,7 @@ public void multiply(Transform matrix) {
  */
 public void rotate(float angle) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Gdip.Matrix_Rotate(getHandle(initialZoom), angle, Gdip.MatrixOrderPrepend);
+	storeAndApplyOperationForAllHandles(new RotateOperation(angle));
 }
 
 /**
@@ -305,7 +313,7 @@ public void rotate(float angle) {
  */
 public void scale(float scaleX, float scaleY) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Gdip.Matrix_Scale(getHandle(initialZoom), scaleX, scaleY, Gdip.MatrixOrderPrepend);
+	storeAndApplyOperationForAllHandles(new ScaleOperation(scaleX, scaleY));
 }
 
 /**
@@ -325,8 +333,9 @@ public void scale(float scaleX, float scaleY) {
  */
 public void setElements(float m11, float m12, float m21, float m22, float dx, float dy) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Drawable drawable = getDevice();
-	Gdip.Matrix_SetElements(getHandle(initialZoom), m11, m12, m21, m22, DPIUtil.scaleUp(drawable, dx, initialZoom), DPIUtil.scaleUp(drawable, dy, initialZoom));
+	// setElements invalidates all previous operations, so we remove them
+	operations.clear();
+	storeAndApplyOperationForAllHandles(new SetElementsOperation(m11, m12, m21, m22, dx, dy));
 }
 
 /**
@@ -344,7 +353,7 @@ public void setElements(float m11, float m12, float m21, float m22, float dx, fl
  */
 public void shear(float shearX, float shearY) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Gdip.Matrix_Shear(getHandle(initialZoom), shearX, shearY, Gdip.MatrixOrderPrepend);
+	storeAndApplyOperationForAllHandles(new ShearOperation(shearX, shearY));
 }
 
 /**
@@ -364,14 +373,15 @@ public void shear(float shearX, float shearY) {
 public void transform(float[] pointArray) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
 	if (pointArray == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
-	int length = pointArray.length;
 	Drawable drawable = getDevice();
+	TransformHandle transformHandle = getTransformHandle(initialZoom);
+	int length = pointArray.length;
 	for (int i = 0; i < length; i++) {
-		pointArray[i] = DPIUtil.scaleUp(drawable, pointArray[i], initialZoom);
+		pointArray[i] = DPIUtil.scaleUp(drawable, pointArray[i], transformHandle.zoom);
 	}
-	Gdip.Matrix_TransformPoints(getHandle(initialZoom), pointArray, length / 2);
+	Gdip.Matrix_TransformPoints(transformHandle.handle, pointArray, length / 2);
 	for (int i = 0; i < length; i++) {
-		pointArray[i] = DPIUtil.scaleDown(drawable, pointArray[i], initialZoom);
+		pointArray[i] = DPIUtil.scaleDown(drawable, pointArray[i], transformHandle.zoom);
 	}
 }
 
@@ -388,8 +398,123 @@ public void transform(float[] pointArray) {
  */
 public void translate(float offsetX, float offsetY) {
 	if (isDisposed()) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
-	Drawable drawable = getDevice();
-	Gdip.Matrix_Translate(getHandle(initialZoom), DPIUtil.scaleUp(drawable, offsetX, initialZoom), DPIUtil.scaleUp(drawable, offsetY, initialZoom), Gdip.MatrixOrderPrepend);
+	storeAndApplyOperationForAllHandles(new TranslateOperation(offsetX, offsetY));
+}
+
+private record TransformHandle(long handle, int zoom) {
+	void destroy() {
+		Gdip.Matrix_delete(handle);
+	}
+}
+
+private record InvertOperation() implements Operation {
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		long handle = transformHandle.handle;
+		if (Gdip.Matrix_Invert(handle) != 0) SWT.error(SWT.ERROR_CANNOT_INVERT_MATRIX);
+	}
+}
+
+private class MultiplyOperation implements Operation {
+	private final float[] elements;
+
+	public MultiplyOperation(Transform matrix) {
+		// As operation are executed on demand on new handles the passed matrix could
+		// already be disposed. Therefore, we need to store the elements to restore
+		// a temporary matrix for this operation on demand
+		elements = new float[6];
+		matrix.getElements(elements);
+	}
+
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		long handle = transformHandle.handle;
+		int zoom = transformHandle.zoom;
+		long newHandle = Gdip.Matrix_new(elements[0], elements[1], elements[2], elements[3], DPIUtil.scaleUp(elements[4], zoom), DPIUtil.scaleUp(elements[5], zoom));
+		if (newHandle == 0) SWT.error(SWT.ERROR_NO_HANDLES);
+		try {
+			Gdip.Matrix_Multiply(handle, newHandle, Gdip.MatrixOrderPrepend);
+		} finally {
+			Gdip.Matrix_delete(newHandle);
+		}
+	}
+}
+
+private record RotateOperation(float angle) implements Operation {
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		long handle = transformHandle.handle;
+		Gdip.Matrix_Rotate(handle, angle, Gdip.MatrixOrderPrepend);
+	}
+}
+
+private record ScaleOperation(float scaleX, float scaleY) implements Operation {
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		long handle = transformHandle.handle;
+		Gdip.Matrix_Scale(handle, scaleX, scaleY, Gdip.MatrixOrderPrepend);
+	}
+}
+
+private class SetElementsOperation implements Operation {
+	private final float m11;
+	private final float m12;
+	private final float m21;
+	private final float m22;
+	private final float dx;
+	private final float dy;
+
+	public SetElementsOperation(float m11, float m12, float m21, float m22, float dx, float dy) {
+		this.m11 = m11;
+		this.m12 = m12;
+		this.m21 = m21;
+		this.m22 = m22;
+		this.dx = dx;
+		this.dy = dy;
+	}
+
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		Drawable drawable = getDevice();
+		long handle = transformHandle.handle;
+		int zoom = transformHandle.zoom;
+		Gdip.Matrix_SetElements(handle, m11, m12, m21, m22, DPIUtil.scaleUp(drawable, dx, zoom), DPIUtil.scaleUp(drawable, dy, zoom));
+	}
+}
+
+private record ShearOperation(float shearX, float shearY) implements Operation {
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		long handle = transformHandle.handle;
+		Gdip.Matrix_Shear(handle, shearX, shearY, Gdip.MatrixOrderPrepend);
+	}
+}
+
+private class TranslateOperation implements Operation {
+	private final float offsetX;
+	private final float offsetY;
+
+	public TranslateOperation(float offsetX, float offsetY) {
+		this.offsetX = offsetX;
+		this.offsetY = offsetY;
+	}
+
+	@Override
+	public void apply(TransformHandle transformHandle) {
+		Drawable drawable = getDevice();
+		long handle = transformHandle.handle;
+		int zoom = transformHandle.zoom;
+		Gdip.Matrix_Translate(handle, DPIUtil.scaleUp(drawable, offsetX, zoom), DPIUtil.scaleUp(drawable, offsetY, zoom), Gdip.MatrixOrderPrepend);
+	}
+}
+
+private interface Operation {
+	void apply(TransformHandle transformHandle);
+}
+
+private void storeAndApplyOperationForAllHandles(Operation operation) {
+	operations.add(operation);
+	zoomToHandle.forEach((zoom, handle) -> operation.apply(handle));
 }
 
 /**
@@ -405,17 +530,21 @@ public String toString() {
 	getElements(elements);
 	return "Transform {" + elements [0] + "," + elements [1] + "," +elements [2] + "," +elements [3] + "," +elements [4] + "," +elements [5] + "}";
 }
-
-long getHandle(int zoomLevel) {
-	if(zoomLevelToHandle.get(zoomLevel) == null) {
+private TransformHandle getTransformHandle(int zoom) {
+	if(zoomToHandle.get(zoom) == null) {
 		float[] elements = new float[6];
 		getElements(elements);
-		elements[4] = DPIUtil.scaleUp(device, DPIUtil.scaleDown(device, elements[4], initialZoom), zoomLevel);
-		elements[5] = DPIUtil.scaleUp(device, DPIUtil.scaleDown(device, elements[5], initialZoom), zoomLevel);
-
-		zoomLevelToHandle.put(zoomLevel, Gdip.Matrix_new(elements[0], elements[1], elements[2], elements[3], elements[4], elements[5]));
+		elements[4] = DPIUtil.scaleUp(device, DPIUtil.scaleDown(device, elements[4], initialZoom), zoom);
+		elements[5] = DPIUtil.scaleUp(device, DPIUtil.scaleDown(device, elements[5], initialZoom), zoom);
+		long handle = Gdip.Matrix_new(elements[0], elements[1], elements[2], elements[3], elements[4], elements[5]);
+		TransformHandle transformHandle = new TransformHandle(handle, zoom);
+		zoomToHandle.put(zoom, transformHandle);
+		return transformHandle;
 	}
-	return zoomLevelToHandle.get(zoomLevel);
+	return zoomToHandle.get(zoom);
 }
 
+long getHandle(int zoomLevel) {
+	return getTransformHandle(zoomLevel).handle;
+}
 }

examples/org.eclipse.swt.examples/src/examples_graphics.properties

@@ -104,6 +104,7 @@ Low=Low
 Default=Default
 High=High
 Transform=Transform
+TransformReuseOper=Transform Re-usage
 Cards=Cards
 Solid=Solid
 Dash=Dash
@@ -137,6 +138,9 @@ Text=Text
 Shapes=Shapes
 Next=Next
 Back=Back
+Shear=Shear
+Translate=Translate
+Identity=Identity
 AnimPathClipping=Animated Path Clipping
 ##### ------------------ Tab Descriptions ------------------ #####
 AlphaDescription=This tab demonstrates alpha blending. It draws various shapes and images as alpha values change.
@@ -164,6 +168,7 @@ MazeDescription=This is a miscellaneous demonstration.  Three objects move throu
 AnimPathClippingDesc=This tab demonstrates the use of path clipping.  A clipping is the area onto which a drawing is made visible.  The default clipping is the entire canvas (all the space in the right panel).  However, one can set the clipping to be something different.  In this tab, the clipping is the set of triangles, rectangles and the circle.  When the blue rectangle is drawn, only the portions of the blue rectangle appearing in the clipping are visible.
 PathClippingDesc=This tab demonstrates the use of path clipping.  A clipping is the area onto which a drawing is made visible.  The default clipping is the entire canvas (all the space in the right panel).  However, one can set the clipping to be something different.
 PathOperDescription=This tab demonstrates the use of paths. It allows the user to see the differences between filling, drawing and closing paths.
+TransformReuseOperDescription=This tab demonstrates the re-usage of transform. It allows the user to test with long living Transform instances.
 rgbDescription=Miscellaneous tab that demonstrates emerging colors from layering other colors.
 RegionClippingDescription=This tab demonstrates how to apply a region clipping and the effects of applying one.  It also demonstrates the operations that can be applied between two regions.
 ShapesDescription=This tab draws 3D shapes (in 2D) using various line styles.

examples/org.eclipse.swt.examples/src/org/eclipse/swt/examples/graphics/GraphicsExample.java

@@ -462,6 +462,7 @@ GraphicsTab[] createTabs() {
 		new ImageFlipTab(this),
 		new ImageScaleTab(this),
 		new PathTab(this),
+		new TransformReuseTab(this),
 	};
 }