Correct GC offset calculation #788

The offset calculation in the GC when having an odd line width is
erroneous in case rotation transformations have been applied to the GC.
With a rotation of 45°, the calculation even suffers from singularities
that result in effectively nothing being drawn. With other
transformation properties, off-by-one error occur.

This change inverts the offset calculation: Instead of performing a
forward transformation of a dummy point whose result is applied to the
expected offset, the inverse transformation is applied to the expected
offset. This avoids singularities and imprecision in the calculation and
ensures that when applying rotations to the GC, the drawing figures are
as expected.

Fixes #788

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

@@ -469,23 +469,28 @@ NSAutoreleasePool checkGC (int mask) {
 		path.setLineCapStyle(capStyle);
 	}
 	if ((state & DRAW_OFFSET) != 0) {
-		data.drawXOffset = data.drawYOffset = 0;
-		NSSize size = new NSSize();
-		size.width = size.height = 1;
-		if (data.transform != null) {
-			size = data.transform.transformSize(size);
-		}
-		double scaling = size.width;
-		if (scaling < 0) scaling = -scaling;
-		double strokeWidth = data.lineWidth * scaling;
-		if (strokeWidth == 0 || ((int)strokeWidth % 2) == 1) {
-			data.drawXOffset = 0.5f / scaling;
-		}
-		scaling = size.height;
-		if (scaling < 0) scaling = -scaling;
-		strokeWidth = data.lineWidth * scaling;
-		if (strokeWidth == 0 || ((int)strokeWidth % 2) == 1) {
-			data.drawYOffset = 0.5f / scaling;
+		boolean isAntiAliasingActive = getAntialias() != SWT.OFF;
+		int effectiveLineWidth = data.lineWidth < 1 ? 1 : Math.round(data.lineWidth);
+		if (isAntiAliasingActive || effectiveLineWidth % 2 == 1) {
+			NSSize offset = new NSSize();
+			// In case the effective line width is odd or anti-aliasing is used, add (0.5, 0.5) to coordinates in
+			// the coordinate system in which drawings are performed.
+			// I.e., a line starting at (0,0) will effectively start in pixel right below
+			// that coordinate with its center at (0.5, 0.5).
+			offset.width = offset.height = 0.5f;
+			if (!isAntiAliasingActive) {
+				offset.width /= effectiveLineWidth;
+				offset.height /= effectiveLineWidth;
+			}
+			// The offset will be applied to the coordinate system of the GC; so transform
+			// it from the drawing coordinate system to the coordinate system of the GC by
+			// applying the inverse transformation as the one applied to the GC and correct
+			// it by the line width.
+			if (data.inverseTransform != null) {
+				data.inverseTransform.transformSize(offset);
+			}
+			data.drawXOffset = Math.abs(offset.width);
+			data.drawYOffset = Math.abs(offset.height);
 		}
 	}
 	return pool;

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

@@ -379,23 +379,33 @@ void checkGC (int mask) {
 		Cairo.cairo_set_miter_limit(cairo, data.lineMiterLimit);
 	}
 	if ((state & DRAW_OFFSET) != 0) {
-		data.cairoXoffset = data.cairoYoffset = 0;
-		double[] matrix = new double[6];
-		Cairo.cairo_get_matrix(cairo, matrix);
-		double[] dx = new double[]{1};
-		double[] dy = new double[]{1};
-		Cairo.cairo_user_to_device_distance(cairo, dx, dy);
-		double scaling = dx[0];
-		if (scaling < 0) scaling = -scaling;
-		double strokeWidth = data.lineWidth * scaling;
-		if (strokeWidth == 0 || ((int)strokeWidth % 2) == 1) {
-			data.cairoXoffset = 0.5 / scaling;
-		}
-		scaling = dy[0];
-		if (scaling < 0) scaling = -scaling;
-		strokeWidth = data.lineWidth * scaling;
-		if (strokeWidth == 0 || ((int)strokeWidth % 2) == 1) {
-			data.cairoYoffset = 0.5 / scaling;
+		boolean isAntiAliasingActive = getAntialias() != SWT.OFF;
+		int effectiveLineWidth = data.lineWidth < 1 ? 1 : Math.round(data.lineWidth);
+		if (isAntiAliasingActive || effectiveLineWidth % 2 == 1) {
+			double[] offsetX = new double[]{1};
+			double[] offsetY = new double[]{1};
+			// In case the effective line width is odd or anti-aliasing is used, add (0.5, 0.5) to coordinates in
+			// the coordinate system in which drawings are performed.
+			// I.e., a line starting at (0,0) will effectively start in pixel right below
+			// that coordinate with its center at (0.5, 0.5).
+			offsetX[0] = offsetY[0] = 0.5f;
+			if (!isAntiAliasingActive) {
+				offsetX[0] /= effectiveLineWidth;
+				offsetY[0] /= effectiveLineWidth;
+			}
+			// The offset will be applied to the coordinate system of the GC; so transform
+			// it from the drawing coordinate system to the coordinate system of the GC by
+			// applying the inverse transformation as the one applied to the GC and correct
+			// it by the line width.
+			double[] matrix = new double[6];
+			Cairo.cairo_get_matrix(cairo, matrix);
+			double[] inverseMatrix = Arrays.copyOf(matrix, 6);
+			Cairo.cairo_matrix_invert(inverseMatrix);
+			Cairo.cairo_set_matrix(cairo, inverseMatrix);
+			Cairo.cairo_user_to_device_distance(cairo, offsetX, offsetY);
+			Cairo.cairo_set_matrix(cairo, matrix);
+			data.cairoXoffset = Math.abs(offsetX[0]);
+			data.cairoYoffset = Math.abs(offsetY[0]);
 		}
 	}
 }

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

@@ -323,24 +323,30 @@ void checkGC(int mask) {
 			data.gdipFont = gdipFont;
 		}
 		if ((state & DRAW_OFFSET) != 0) {
-			data.gdipXOffset = data.gdipYOffset = 0;
-			long matrix = Gdip.Matrix_new(1, 0, 0, 1, 0, 0);
-			PointF point = new PointF();
-			point.X = point.Y = 1;
-			Gdip.Graphics_GetTransform(gdipGraphics, matrix);
-			Gdip.Matrix_TransformVectors(matrix, point, 1);
-			Gdip.Matrix_delete(matrix);
-			float scaling = point.X;
-			if (scaling < 0) scaling = -scaling;
-			float penWidth = data.lineWidth * scaling;
-			if (penWidth == 0 || (((int)penWidth) & 1) == 1) {
-				data.gdipXOffset = 0.5f / scaling;
-			}
-			scaling = point.Y;
-			if (scaling < 0) scaling = -scaling;
-			penWidth = data.lineWidth * scaling;
-			if (penWidth == 0 || (((int)penWidth) & 1) == 1) {
-				data.gdipYOffset = 0.5f / scaling;
+			boolean isAntiAliasingActive = getAntialias() != SWT.OFF;
+			int effectiveLineWidth = data.lineWidth < 1 ? 1 : Math.round(data.lineWidth);
+			if (isAntiAliasingActive || effectiveLineWidth % 2 == 1) {
+				PointF offset = new PointF();
+				// In case the effective line width is odd or anti-aliasing is used, add (0.5, 0.5) to coordinates in
+				// the coordinate system in which drawings are performed.
+				// I.e., a line starting at (0,0) will effectively start in pixel right below
+				// that coordinate with its center at (0.5, 0.5).
+				offset.X = offset.Y = 0.5f;
+				if (!isAntiAliasingActive) {
+					offset.X /= effectiveLineWidth;
+					offset.Y /= effectiveLineWidth;
+				}
+				// The offset will be applied to the coordinate system of the GC; so transform
+				// it from the drawing coordinate system to the coordinate system of the GC by
+				// applying the inverse transformation as the one applied to the GC and correct
+				// it by the line width.
+				long newMatrix = Gdip.Matrix_new(1, 0, 0, 1, 0, 0);
+				Gdip.Graphics_GetTransform(gdipGraphics, newMatrix);
+				Gdip.Matrix_Invert(newMatrix);
+				Gdip.Matrix_TransformVectors(newMatrix, offset, 1);
+				Gdip.Matrix_delete(newMatrix);
+				data.gdipXOffset = Math.abs(offset.X);
+				data.gdipYOffset = Math.abs(offset.Y);
 			}
 		}
 		return;

bundles/org.eclipse.swt/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-SymbolicName: org.eclipse.swt; singleton:=true
-Bundle-Version: 3.124.200.qualifier
+Bundle-Version: 3.124.300.qualifier
 Bundle-ManifestVersion: 2
 Bundle-Localization: plugin
 DynamicImport-Package: org.eclipse.swt.accessibility2

bundles/org.eclipse.swt/pom.xml

@@ -21,7 +21,7 @@
     </parent>
     <groupId>org.eclipse.swt</groupId>
     <artifactId>org.eclipse.swt</artifactId>
-    <version>3.124.200-SNAPSHOT</version>
+    <version>3.124.300-SNAPSHOT</version>
     <packaging>eclipse-plugin</packaging>
 
     <properties>

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java

@@ -15,6 +15,9 @@
 package org.eclipse.swt.tests.junit;
 
 import static org.eclipse.swt.tests.junit.SwtTestUtil.assertSWTProblem;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -35,6 +38,7 @@
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.graphics.RGB;
 import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.graphics.Transform;
 import org.eclipse.swt.internal.DPIUtil;
 import org.eclipse.swt.widgets.Canvas;
 import org.eclipse.swt.widgets.Display;
@@ -733,6 +737,35 @@ public void test_bug493455_drawImageAlpha_srcPos() {
 
 }
 
+/**
+ * @see <a href="https://github.com/eclipse-platform/eclipse.platform.swt/issues/788">Issue 788</a>
+ */
+@Test
+public void test_drawLine_noSingularitiesIn45DregreeRotation() {
+	int imageSize = 3;
+	int centerPixel = imageSize / 2;
+	Image image = new Image(Display.getDefault(), imageSize, imageSize);
+	GC gc = new GC(image);
+	Transform rotation = new Transform(gc.getDevice());
+	gc.getTransform(rotation);
+
+	try {
+		// Rotate 45° around image center
+		rotation.translate(centerPixel, centerPixel);
+		rotation.rotate(45);
+		rotation.translate(- centerPixel, - centerPixel);
+		gc.setTransform(rotation);
+		gc.drawLine(centerPixel, centerPixel, centerPixel + 1, centerPixel);
+
+		assertThat("line is not drawn with 45 degree rotation",
+				image.getImageData().getPixel(centerPixel, centerPixel), not(is(-1)));
+	} finally {
+		rotation.dispose();
+		gc.dispose();
+		image.dispose();
+	}
+}
+
 /* custom */
 Display display;
 Shell shell;

tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: Eclipse SWT Tests
 Bundle-SymbolicName: org.eclipse.swt.tests
-Bundle-Version: 3.107.200.qualifier
+Bundle-Version: 3.107.300.qualifier
 Bundle-Vendor: Eclipse.org
 Bundle-Localization: plugin
 Export-Package: org.eclipse.swt.tests.junit,

tests/org.eclipse.swt.tests/pom.xml

@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.swt</groupId>
   <artifactId>org.eclipse.swt.tests</artifactId>
-  <version>3.107.200-SNAPSHOT</version>
+  <version>3.107.2300-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
   <properties>
     <code.ignoredWarnings>${tests.ignoredWarnings}</code.ignoredWarnings>