From fcbf1aca5d38427f91ff2d9cd700b1f5151b14ab Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 28 Aug 2023 18:00:24 +0200 Subject: [PATCH] Correct GC offset calculation #788 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 https://github.com/eclipse-platform/eclipse.platform.swt/issues/788 --- .../cocoa/org/eclipse/swt/graphics/GC.java | 39 +++++++++------- .../gtk/org/eclipse/swt/graphics/GC.java | 44 ++++++++++++------- .../win32/org/eclipse/swt/graphics/GC.java | 42 ++++++++++-------- bundles/org.eclipse.swt/META-INF/MANIFEST.MF | 2 +- bundles/org.eclipse.swt/pom.xml | 2 +- .../Test_org_eclipse_swt_graphics_GC.java | 33 ++++++++++++++ .../META-INF/MANIFEST.MF | 2 +- tests/org.eclipse.swt.tests/pom.xml | 2 +- 8 files changed, 110 insertions(+), 56 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/GC.java index 1d13bdbb4fe..401f1eba6f1 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/GC.java +++ b/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; diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java index cc0551c55e8..d2a8d7c7341 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/GC.java +++ b/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]); } } } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java index baa5273ea5f..073fe63c960 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java +++ b/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; diff --git a/bundles/org.eclipse.swt/META-INF/MANIFEST.MF b/bundles/org.eclipse.swt/META-INF/MANIFEST.MF index 2a52eb548da..95bc106c4ef 100644 --- a/bundles/org.eclipse.swt/META-INF/MANIFEST.MF +++ b/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 diff --git a/bundles/org.eclipse.swt/pom.xml b/bundles/org.eclipse.swt/pom.xml index 1b5f53ede77..38b69b76b73 100644 --- a/bundles/org.eclipse.swt/pom.xml +++ b/bundles/org.eclipse.swt/pom.xml @@ -21,7 +21,7 @@ org.eclipse.swt org.eclipse.swt - 3.124.200-SNAPSHOT + 3.124.300-SNAPSHOT eclipse-plugin diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java index 429fdd2aad8..667787c3c7d 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java +++ b/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 Issue 788 + */ +@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; diff --git a/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF index 68dc191c06f..d0d1eca0f6c 100644 --- a/tests/org.eclipse.swt.tests/META-INF/MANIFEST.MF +++ b/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, diff --git a/tests/org.eclipse.swt.tests/pom.xml b/tests/org.eclipse.swt.tests/pom.xml index b1948cf6b0f..c009c02f2ee 100644 --- a/tests/org.eclipse.swt.tests/pom.xml +++ b/tests/org.eclipse.swt.tests/pom.xml @@ -19,7 +19,7 @@ org.eclipse.swt org.eclipse.swt.tests - 3.107.200-SNAPSHOT + 3.107.2300-SNAPSHOT eclipse-test-plugin ${tests.ignoredWarnings}