diff --git a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java index 6b4cb998c86..8649ae4d63e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java @@ -15,15 +15,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.concurrent.*; +import java.util.stream.*; import org.eclipse.swt.*; import org.eclipse.swt.internal.*; import org.eclipse.swt.widgets.*; import org.junit.jupiter.api.*; import org.junit.jupiter.api.extension.*; +import org.junit.jupiter.params.*; +import org.junit.jupiter.params.provider.*; @ExtendWith(PlatformSpecificExecutionExtension.class) @ExtendWith(WithMonitorSpecificScalingExtension.class) @@ -62,7 +66,84 @@ public void drawnElementsShouldScaleUpToTheRightZoomLevel() { GC gc = GC.win32_new(shell, new GCData()); gc.getGCData().nativeZoom = zoom * scalingFactor; gc.getGCData().lineWidth = 10; - assertEquals("Drawn elements should scale to the right value", gc.getGCData().lineWidth, gc.getLineWidth() * scalingFactor, 0); + assertEquals("Drawn elements should scale to the right value", gc.getGCData().lineWidth, + gc.getLineWidth() * scalingFactor, 0); + } + + /** + * Regression test for https://github.com/eclipse-platform/eclipse.platform.swt/issues/3091 + */ + @Test + public void test_drawTextLjava_lang_StringII_advanced_unterlined() { + Display display = Display.getDefault(); + FontData defaultFontFata = display.getSystemFont().getFontData()[0]; + defaultFontFata.data.lfUnderline = 1; + Font font = new Font(display, defaultFontFata); + Image image = new Image(display, 100, 100); + try { + GC gc = new GC(image); + gc.setFont(font); + gc.setAdvanced(true); + gc.drawText("Hello World", 10, 50); + ImageData imageData = image.getImageData(); + boolean anyPixelUnderTextIsNotEmpty = false; + for (int i = 0; i < imageData.width * imageData.height; i++) { + if (imageData.getPixel(i % imageData.width, i / imageData.width) != -1) { + anyPixelUnderTextIsNotEmpty = true; + break; + } + } + assertTrue(anyPixelUnderTextIsNotEmpty); + } finally { + image.dispose(); + font.dispose(); + } + } + + private static final String USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY = + "org.eclipse.swt.internal.win32.useGDITextRenderingWithGDIP"; + + /** + * End-to-end counterpart to + * {@link #test_drawTextLjava_lang_StringII_advanced_unterlined()}: it + * compares the actual rendered output of the default (fixed) behavior with + * the legacy behavior restored via the + * {@code org.eclipse.swt.internal.win32.useGDITextRenderingWithGDIP} system + * property, for the exact scenario reported in + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/3091 (an + * underlined font drawn with {@code GC.setAdvanced(true)}). + *

+ * This both confirms that the default behavior is fixed (text is visible) + * and that the escape-hatch property still reproduces the historical, + * broken behavior (no text drawn at all), so that any unintended change to + * either path is caught. + */ + @Test + public void drawTextAdvancedUnderlinePropertyTogglesLegacyFallback() { + Display display = Display.getDefault(); + FontData underlineFontData = display.getSystemFont().getFontData()[0]; + underlineFontData.data.lfUnderline = 1; + Font underlineFont = new Font(display, underlineFontData); + Image image = new Image(display, 100, 100); + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + int pixelsWithGdipDefault = renderTextAndCountNonWhitePixels(image, underlineFont, "Hello World"); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + int pixelsWithLegacyFallback = renderTextAndCountNonWhitePixels(image, underlineFont, "Hello World"); + + assertAll( + () -> assertTrue(pixelsWithGdipDefault > 0, + "Default (GDI+) rendering must draw the underlined text (regression for #3091)"), + () -> assertEquals("Legacy GDI fallback (property=true) historically drew nothing at all for " + + "underlined fonts in advanced mode; the property must still reproduce that behavior", + 0, pixelsWithLegacyFallback) + ); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + underlineFont.dispose(); + image.dispose(); + } } /** @@ -142,4 +223,341 @@ private static int renderTextAndCountNonWhitePixels(Image target, Font font, Str } return count; } + + // ------------------------------------------------------------------ + // Broader coverage: the fix for #3091 changes drawText() from computing + // glyph positions with GDI (GetCharacterPlacement) and drawing them with + // Gdip.Graphics_DrawDriverString, to fully delegating layout and drawing + // to Gdip.Graphics_DrawString. Both are still GDI+ calls, but DrawString + // additionally honours font decorations (underline/strikeout) and uses + // GDI+'s own text layout engine (kerning, complex script shaping, + // mnemonic hotkey prefix, bidi/mirroring), whereas DrawDriverString only + // draws glyphs at explicit, GDI-computed positions. The tests below probe + // several of those text properties, comparing the default (GDI+ + // DrawString) path with the legacy fallback (GDI DrawDriverString, + // restored via the escape-hatch system property) to catch regressions in + // either direction. + // ------------------------------------------------------------------ + + private static Stream styleDecorationCombinations() { + return Stream.of( + Arguments.of(SWT.NORMAL, true, false, "normal weight + underline"), + Arguments.of(SWT.BOLD, true, false, "bold + underline"), + Arguments.of(SWT.ITALIC, false, true, "italic + strikeout"), + Arguments.of(SWT.BOLD | SWT.ITALIC, true, true, "bold + italic + underline + strikeout") + ); + } + + /** + * Extends {@link #drawTextAdvancedUnderlinePropertyTogglesLegacyFallback()} + * to combinations of weight/style (bold, italic) with underline and/or + * strikeout, to ensure the fix is not accidentally narrow to the single + * plain+underline scenario reported in + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/3091. + */ + @ParameterizedTest(name = "{3}") + @MethodSource("styleDecorationCombinations") + public void drawTextStyleCombinationsWithDecorationDifferBetweenRenderingPaths(int styleBits, boolean underline, + boolean strikeout, String description) { + Display display = Display.getDefault(); + FontData fontData = display.getSystemFont().getFontData()[0]; + fontData.setStyle(styleBits); + if (underline) fontData.data.lfUnderline = 1; + if (strikeout) fontData.data.lfStrikeOut = 1; + Font font = new Font(display, fontData); + Image image = new Image(display, 150, 100); + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + int pixelsWithGdipDefault = renderTextAndCountNonWhitePixels(image, font, "Hello"); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + int pixelsWithLegacyFallback = renderTextAndCountNonWhitePixels(image, font, "Hello"); + + assertAll(description, + () -> assertTrue(pixelsWithGdipDefault > 0, + "Default (GDI+) rendering must draw text decorated with " + description), + () -> assertEquals("Legacy GDI fallback must reproduce the historical bug of drawing nothing for " + + description, 0, pixelsWithLegacyFallback) + ); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + font.dispose(); + image.dispose(); + } + } + + /** + * Verifies that mnemonic (accelerator) underline drawing, which is + * implemented completely differently in the two rendering paths (GDI+'s + * built-in hotkey-prefix support in the default path vs. a manually drawn + * line in the legacy fallback), still produces visibly more ink than the + * same text without a mnemonic, on both paths. Unlike font-level + * underline/strikeout, mnemonic underlining does not rely on advanced-mode + * font decoration and historically worked in both rendering paths, so this + * guards against the switch introducing a regression there. + */ + @Test + public void drawTextMnemonicUnderlineIsPreservedAcrossRenderingPaths() { + Display display = Display.getDefault(); + Font font = display.getSystemFont(); + Image image = new Image(display, 150, 60); + try { + for (boolean legacyFallback : new boolean[] { false, true }) { + if (legacyFallback) { + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + } else { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + } + int pixelsWithMnemonic = renderTextAndCountNonWhitePixels(image, font, "&File", + SWT.DRAW_MNEMONIC | SWT.DRAW_TRANSPARENT, SWT.NONE); + int pixelsWithoutMnemonic = renderTextAndCountNonWhitePixels(image, font, "File", + SWT.DRAW_TRANSPARENT, SWT.NONE); + String pathDescription = legacyFallback ? "legacy GDI fallback" : "default GDI+"; + assertTrue(pixelsWithMnemonic > pixelsWithoutMnemonic, + "Mnemonic underline should add visible ink under " + pathDescription + " rendering " + + "(with mnemonic: " + pixelsWithMnemonic + ", without: " + pixelsWithoutMnemonic + ")"); + } + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + image.dispose(); + } + } + + /** + * Verifies that mirrored (right-to-left) rendering, which the two paths + * implement differently (manual coordinate/brush transforms for + * DrawDriverString vs. {@code StringFormatFlagsDirectionRightToLeft} for + * DrawString), still produces a visible, similarly sized block of ink on + * both the default and the legacy-fallback rendering path. + */ + @Test + public void drawTextMirroredStyleRendersConsistentInkAreaAcrossRenderingPaths() { + Display display = Display.getDefault(); + Font font = display.getSystemFont(); + Image image = new Image(display, 200, 60); + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + Rectangle gdipInkBounds = renderTextAndGetInkBounds(image, font, "Hello World", + SWT.DRAW_TRANSPARENT, SWT.RIGHT_TO_LEFT); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + Rectangle legacyInkBounds = renderTextAndGetInkBounds(image, font, "Hello World", + SWT.DRAW_TRANSPARENT, SWT.RIGHT_TO_LEFT); + + assertAll( + () -> assertNotNull(gdipInkBounds, "Default (GDI+) mirrored rendering must draw visible text"), + () -> assertNotNull(legacyInkBounds, "Legacy fallback mirrored rendering must draw visible text") + ); + // Widths may legitimately differ a bit (different layout engines), but a + // gross regression (e.g. text collapsed to a sliver, or drawn far wider + // because mirroring was applied twice) would fall well outside this range. + assertWithinTolerance("mirrored text ink width", legacyInkBounds.width, gdipInkBounds.width, 0.4); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + image.dispose(); + } + } + + private static Stream complexScriptsAndCharsets() { + return Stream.of( + Arguments.of("Arabic", "\u0645\u0631\u062d\u0628\u0627"), + Arguments.of("Hebrew", "\u05e9\u05dc\u05d5\u05dd"), + Arguments.of("CJK (Chinese)", "\u4f60\u597d\u4e16\u754c"), + Arguments.of("Cyrillic", "\u041f\u0440\u0438\u0432\u0435\u0442"), + Arguments.of("Greek", "\u0393\u03b5\u03b9\u03ac \u03c3\u03bf\u03c5"), + Arguments.of("Combining diacritics", "e\u0301clat") + ); + } + + /** + * Verifies that plain (non-decorated) text in a variety of scripts and + * charsets still renders visible ink on both the default (GDI+ + * DrawString) and legacy fallback (GDI+ DrawDriverString) rendering + * paths. This is a smoke test only: exact glyph shaping/positioning is + * expected to differ slightly between the two layout engines, but neither + * path should silently drop the text, which would be a much bigger + * regression than the underline/strikeout issue this change fixes. + */ + @ParameterizedTest(name = "{0}") + @MethodSource("complexScriptsAndCharsets") + public void drawTextComplexScriptsAndCharsetsRenderVisibleInkOnBothPaths(String description, String text) { + Display display = Display.getDefault(); + Font font = display.getSystemFont(); + Image image = new Image(display, 200, 60); + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + int pixelsWithGdipDefault = renderTextAndCountNonWhitePixels(image, font, text); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + int pixelsWithLegacyFallback = renderTextAndCountNonWhitePixels(image, font, text); + + assertAll(description, + () -> assertTrue(pixelsWithGdipDefault > 0, + "Default (GDI+) rendering must draw visible ink for " + description), + () -> assertTrue(pixelsWithLegacyFallback > 0, + "Legacy fallback rendering must draw visible ink for " + description) + ); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + image.dispose(); + } + } + + /** + * Compares the ink width of a kerning-sensitive string between the two + * rendering paths. GDI+'s own typographic layout (default path) and GDI's + * character-placement-based layout (legacy fallback) are expected to + * compute slightly different advances/kerning, but a gross regression + * (e.g. glyphs overlapping to near-zero width, or width roughly doubling) + * would indicate a real layout bug rather than an expected, minor + * kerning difference. + */ + @Test + public void drawTextKerningSensitiveTextWidthRemainsPlausibleBetweenRenderingPaths() { + Display display = Display.getDefault(); + Font font = display.getSystemFont(); + Image image = new Image(display, 300, 60); + String kerningSensitiveText = "AVATAR WAVE To Yes"; + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + Rectangle gdipInkBounds = renderTextAndGetInkBounds(image, font, kerningSensitiveText, + SWT.DRAW_TRANSPARENT, SWT.NONE); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + Rectangle legacyInkBounds = renderTextAndGetInkBounds(image, font, kerningSensitiveText, + SWT.DRAW_TRANSPARENT, SWT.NONE); + + assertAll( + () -> assertNotNull(gdipInkBounds, "Default (GDI+) rendering must draw visible text"), + () -> assertNotNull(legacyInkBounds, "Legacy fallback rendering must draw visible text") + ); + assertWithinTolerance("kerning-sensitive text ink width", legacyInkBounds.width, gdipInkBounds.width, 0.3); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + image.dispose(); + } + } + + /** + * Verifies that tab expansion is computed identically by both rendering + * paths. Both use {@code lptm.tmAveCharWidth * 8} pixels (the Win32 + * DrawText()/TabbedTextOut() convention) for the tab stop width: the + * legacy fallback via {@code OS.GetTextMetrics}, and the default GDI+ + * path via the {@code TEXTMETRIC} computed by the caller before dispatch. + * (An earlier version of the GDI+ path used 8x the GDI+-measured space + * glyph width instead, which produced noticeably narrower/tab stops than + * plain GDI and the legacy fallback - see the regression test below.) + */ + @Test + public void drawTextTabStopsExpandToComparablePositionsBetweenRenderingPaths() { + Display display = Display.getDefault(); + Font font = display.getSystemFont(); + Image image = new Image(display, 300, 60); + try { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + Rectangle gdipInkBounds = renderTextAndGetInkBounds(image, font, "A\tB", SWT.DRAW_TAB, SWT.NONE); + + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, "true"); + Rectangle legacyInkBounds = renderTextAndGetInkBounds(image, font, "A\tB", SWT.DRAW_TAB, SWT.NONE); + + assertAll( + () -> assertNotNull(gdipInkBounds, "Default (GDI+) rendering must draw text after a tab"), + () -> assertNotNull(legacyInkBounds, "Legacy fallback rendering must draw text after a tab") + ); + // The tab should push "B" noticeably to the right of "A" in both paths. + int minimumExpectedTabAdvance = 20; + assertAll( + () -> assertTrue(gdipInkBounds.width > minimumExpectedTabAdvance, + "Default (GDI+) rendering should expand the tab to a visible column, got width " + + gdipInkBounds.width), + () -> assertTrue(legacyInkBounds.width > minimumExpectedTabAdvance, + "Legacy fallback rendering should expand the tab to a visible column, got width " + + legacyInkBounds.width) + ); + // Both paths now use the same tab stop metric, so the ink width should + // be nearly identical; allow a small tolerance for minor per-glyph + // rendering/rounding differences between DrawString and DrawDriverString. + assertWithinTolerance("tab-expanded text ink width", legacyInkBounds.width, gdipInkBounds.width, 0.15); + } finally { + System.clearProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY); + image.dispose(); + } + } + + /** + * Asserts that {@code actual} is within {@code (1 +/- tolerance)} times + * {@code expected}, i.e. flags gross regressions (roughly halved/doubled + * or worse) while tolerating the minor layout differences that are + * expected between GDI+'s own text layout and GDI-computed glyph + * placement. + */ + private static void assertWithinTolerance(String description, int actual, int expected, double tolerance) { + int lowerBound = (int) Math.floor(expected * (1 - tolerance)); + int upperBound = (int) Math.ceil(expected * (1 + tolerance)); + assertTrue(actual >= lowerBound && actual <= upperBound, + "Expected " + description + " (" + actual + ") to be within " + (int) (tolerance * 100) + + "% of the reference value (" + expected + "), i.e. in [" + lowerBound + ", " + upperBound + "]"); + } + + private static int renderTextAndCountNonWhitePixels(Image target, Font font, String text, int drawFlags, + int gcStyle) { + renderText(target, font, text, drawFlags, gcStyle); + return countNonWhitePixels(target); + } + + private static Rectangle renderTextAndGetInkBounds(Image target, Font font, String text, int drawFlags, + int gcStyle) { + renderText(target, font, text, drawFlags, gcStyle); + return inkBounds(target); + } + + private static void renderText(Image target, Font font, String text, int drawFlags, int gcStyle) { + GC testGC = new GC(target, gcStyle); + try { + testGC.setAdvanced(true); + testGC.setBackground(new Color(255, 255, 255)); + testGC.fillRectangle(target.getBounds()); + testGC.setForeground(new Color(0, 0, 0)); + testGC.setFont(font); + testGC.drawText(text, 5, 5, drawFlags); + } finally { + testGC.dispose(); + } + } + + private static int countNonWhitePixels(Image target) { + ImageData imageData = target.getImageData(DPIUtil.getDeviceZoom()); + int count = 0; + for (int y = 0; y < imageData.height; y++) { + for (int x = 0; x < imageData.width; x++) { + RGB rgb = imageData.palette.getRGB(imageData.getPixel(x, y)); + if (rgb.red != 255 || rgb.green != 255 || rgb.blue != 255) { + count++; + } + } + } + return count; + } + + /** + * Returns the bounding box of all non-white pixels in the given image, or + * {@code null} if the image is entirely white (i.e. nothing was drawn). + */ + private static Rectangle inkBounds(Image target) { + ImageData imageData = target.getImageData(DPIUtil.getDeviceZoom()); + int minX = Integer.MAX_VALUE, minY = Integer.MAX_VALUE, maxX = -1, maxY = -1; + for (int y = 0; y < imageData.height; y++) { + for (int x = 0; x < imageData.width; x++) { + RGB rgb = imageData.palette.getRGB(imageData.getPixel(x, y)); + if (rgb.red != 255 || rgb.green != 255 || rgb.blue != 255) { + minX = Math.min(minX, x); + minY = Math.min(minY, y); + maxX = Math.max(maxX, x); + maxY = Math.max(maxY, y); + } + } + } + if (maxX < 0) return null; + return new Rectangle(minX, minY, maxX - minX + 1, maxY - minY + 1); + } } 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 e6b99131a7b..1ed2bbc2dc9 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 @@ -110,6 +110,8 @@ public final class GC extends Resource { static final float[] LINE_DASHDOT_ZERO = new float[]{9, 6, 3, 6}; static final float[] LINE_DASHDOTDOT_ZERO = new float[]{9, 3, 3, 3, 3, 3}; + private static final String USE_GDI_TEXT_RENDERING_WITH_GDPI = "org.eclipse.swt.internal.win32.useGDITextRenderingWithGDIP"; + /** * Prevents uninitialized instances from being created outside the package. */ @@ -2850,7 +2852,18 @@ private void drawTextInPixels (String string, int x, int y, int flags) { OS.SetBkMode(handle, oldBkMode); } -private boolean useGDIP (long hdc, char[] buffer) { +private boolean useGDIP(long hdc, char[] buffer) { + // If the system property is set to true, then GDI text rendering may be used + // even when GDI+/advanced mode is enabled. + // The property exists to allow backward compatibility with long standing + // behavior of using GDI text rendering even when GDI+ was enabled was enforced, + // introduced because of issues with the GDI+ text rendering a long time ago. In + // case no regressions occur because of using GDI+ text rendering, this property + // should be removed and GDI+ text rendering should be used by default when + // GDI+/advanced mode is enabled. + if (!Boolean.getBoolean(USE_GDI_TEXT_RENDERING_WITH_GDPI)) { + return true; + } short[] glyphs = new short[buffer.length]; OS.GetGlyphIndices(hdc, buffer, buffer.length, glyphs, OS.GGI_MARK_NONEXISTING_GLYPHS); for (int i = 0; i < glyphs.length; i++) { @@ -2882,7 +2895,7 @@ void drawText(long gdipGraphics, String string, int x, int y, int flags, Point s if (hFont != 0) OS.SelectObject(hdc, oldFont); Gdip.Graphics_ReleaseHDC(gdipGraphics, hdc); if (gdip) { - drawTextGDIP(gdipGraphics, string, x, y, flags, size == null, size); + drawTextGDIP(gdipGraphics, string, x, y, flags, size == null, size, lptm); return; } int i = 0, start = 0, end = 0, drawX = x, drawY = y, width = 0, mnemonicIndex = -1; @@ -3052,7 +3065,7 @@ private RectF drawText(long gdipGraphics, char[] buffer, int start, int length, return bounds; } -private void drawTextGDIP(long gdipGraphics, String string, int x, int y, int flags, boolean draw, Point size) { +private void drawTextGDIP(long gdipGraphics, String string, int x, int y, int flags, boolean draw, Point size, TEXTMETRIC lptm) { boolean needsBounds = !draw || (flags & SWT.DRAW_TRANSPARENT) == 0; char[] buffer; if ((flags & SWT.DRAW_DELIMITER) == 0) { @@ -3074,7 +3087,14 @@ private void drawTextGDIP(long gdipGraphics, String string, int x, int y, int fl int formatFlags = Gdip.StringFormat_GetFormatFlags(format) | Gdip.StringFormatFlagsMeasureTrailingSpaces; if ((data.style & SWT.MIRRORED) != 0) formatFlags |= Gdip.StringFormatFlagsDirectionRightToLeft; Gdip.StringFormat_SetFormatFlags(format, formatFlags); - float[] tabs = (flags & SWT.DRAW_TAB) != 0 ? new float[]{measureSpace(data.gdipFont, format) * 8} : new float[1]; + // Use the same tab stop width as the legacy GDI/GDI-glyph-position path + // (8 * the font's GDI average character width, matching Win32's own + // DrawText()/TabbedTextOut() convention), rather than 8 * the width of a + // single space glyph as measured by GDI+. The two metrics are computed by + // different engines and can differ substantially (the GDI+ space-glyph + // metric tends to be noticeably narrower), which previously made tab + // stops shrink visibly compared to plain GDI and the legacy fallback. + float[] tabs = (flags & SWT.DRAW_TAB) != 0 ? new float[]{lptm.tmAveCharWidth * 8} : new float[1]; Gdip.StringFormat_SetTabStops(format, 0, tabs.length, tabs); int hotkeyPrefix = (flags & SWT.DRAW_MNEMONIC) != 0 ? Gdip.HotkeyPrefixShow : Gdip.HotkeyPrefixNone; if ((flags & SWT.DRAW_MNEMONIC) != 0 && (data.uiState & OS.UISF_HIDEACCEL) != 0) hotkeyPrefix = Gdip.HotkeyPrefixHide; @@ -4654,12 +4674,7 @@ public boolean isDisposed() { return handle == 0; } -private float measureSpace(long font, long format) { - PointF pt = new PointF(); - RectF bounds = new RectF(); - Gdip.Graphics_MeasureString(data.gdipGraphics, new char[]{' '}, 1, font, pt, format, bounds); - return bounds.Width; -} + /** * Sets the receiver to always use the operating system's advanced graphics diff --git a/examples/org.eclipse.swt.snippets/.classpath_cocoa b/examples/org.eclipse.swt.snippets/.classpath_cocoa index bcad73af751..6f6d5d56841 100644 --- a/examples/org.eclipse.swt.snippets/.classpath_cocoa +++ b/examples/org.eclipse.swt.snippets/.classpath_cocoa @@ -2,6 +2,6 @@ - + diff --git a/examples/org.eclipse.swt.snippets/.classpath_gtk b/examples/org.eclipse.swt.snippets/.classpath_gtk index bcad73af751..7c1971fa7fc 100644 --- a/examples/org.eclipse.swt.snippets/.classpath_gtk +++ b/examples/org.eclipse.swt.snippets/.classpath_gtk @@ -2,6 +2,6 @@ - + diff --git a/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet395.java b/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet395.java new file mode 100644 index 00000000000..0aad3202138 --- /dev/null +++ b/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet395.java @@ -0,0 +1,254 @@ +/******************************************************************************* + * Copyright (c) 2026 Eclipse Foundation and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.swt.snippets; + +import java.util.*; +import java.util.concurrent.atomic.*; + +import org.eclipse.swt.*; +import org.eclipse.swt.custom.*; +import org.eclipse.swt.graphics.*; +import org.eclipse.swt.layout.*; +import org.eclipse.swt.widgets.*; + +/* + * [Win32] GDI+ vs. legacy GDI text rendering snippet, related to + * https://github.com/eclipse-platform/eclipse.platform.swt/issues/3091 : + * GC.drawText() was changed to always use GDI+'s own DrawString/text-layout + * engine while GC.setAdvanced(true) is active, instead of only falling back + * to it for characters GDI+ cannot shape itself (and otherwise using GDI to + * compute glyph positions, then drawing them with GDI+'s DrawDriverString). + * That change affects more than just underline/strikeout: kerning, tab stop + * width, mnemonic underlining and bidi/mirroring are all computed by a + * different engine depending on which path is used. + * + * This snippet renders a series of text properties, one row per property, so + * that the different rendering paths can be visually compared without + * needing to restart the process: + * - The "Use GDI+ (advanced) rendering" checkbox calls GC.setAdvanced() at + * runtime, switching every row between plain GDI (OS.DrawText, used when + * advanced is off) and GDI+ (used when advanced is on). The system + * property - and therefore the second checkbox - only ever matters while + * this one is checked; it is disabled otherwise. + * - The "Use legacy GDI text rendering" checkbox toggles the + * org.eclipse.swt.internal.win32.useGDITextRenderingWithGDIP system + * property at runtime (via System.setProperty) and immediately re-renders + * all rows, since GC.useGDIP() re-reads the property on every drawText() + * call. + * + * How to use this snippet (on Windows): + * 1. Run it. Both checkboxes start as: advanced = checked, legacy = unchecked, + * i.e. the default/new behavior is shown (GDI+'s DrawString). + * 2. Check "Use legacy GDI text rendering" to switch to the legacy fallback + * (GDI+'s DrawDriverString with GDI-computed glyph positions) and watch + * which rows change. Uncheck it to switch back. Keep toggling to compare. + * 3. Uncheck "Use GDI+ (advanced) rendering" to compare against plain GDI + * text rendering (OS.DrawText) altogether - this is the code path used + * whenever a GC is not advanced, both before and after the #3091 fix, and + * is unaffected by the system property (the legacy checkbox is disabled + * while unchecked). + * 4. What to expect while toggling: + * - "Underlined"/"Strikeout"/"Bold + underlined" rows are expected to go + * blank only when GDI+/advanced is on AND legacy GDI text rendering is + * checked (the historical bug from #3091); they should render visibly + * with plain GDI (advanced off) and with the default GDI+ path. + * - "Mnemonic" should show an underlined "F" in all combinations (it does + * not depend on font-level decoration). + * - "Kerning pair", "Tab-separated columns" and the "Mirrored / RTL" row + * may show differences in spacing/positioning between plain GDI, GDI+ + * default and GDI+ legacy fallback (expected, since each combination + * uses a different layout engine), but should never render blank, + * wildly stretched/compressed, or with + * overlapping glyphs. + * - The script/charset rows (Arabic, Hebrew, CJK, Cyrillic, Greek, + * combining diacritics) should render recognisable, visible glyphs + * regardless of the checkbox state. + * + * On platforms other than Windows the system property has no effect and all + * rows are expected to render identically regardless of the checkbox state. + * + * For a list of all SWT example snippets see + * http://www.eclipse.org/swt/snippets/ + */ +public class Snippet395 { + + static final String USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY = + "org.eclipse.swt.internal.win32.useGDITextRenderingWithGDIP"; + + /** One row of the comparison: a label, the text properties to apply, and how to draw it. */ + record TextRow(String label, int fontStyle, boolean underline, boolean strikeout, String text, int drawFlags, + int gcStyle) { + TextRow(String label, String text) { + this(label, SWT.NORMAL, false, false, text, SWT.DRAW_TRANSPARENT, SWT.NONE); + } + } + + /** + * Renders one fresh sample {@link Image} per row, reflecting whatever the + * {@link #USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY} system property is set + * to right now, and whether GDI+ (advanced) rendering is requested at all. + * When {@code advanced} is {@code false}, GC always uses plain GDI + * ({@code OS.DrawText}) regardless of the system property - the property + * (and the fix/regression it is about) only ever applies to the advanced/ + * GDI+ code path. Callers are responsible for disposing the previous set + * of images returned by an earlier call. + */ + private static Map renderSamples(Display display, java.util.List rows, + Map fonts, int sampleWidth, int sampleHeight, boolean advanced) { + Map samples = new HashMap<>(); + for (TextRow row : rows) { + Image sample = new Image(display, sampleWidth, sampleHeight); + GC gc = new GC(sample, row.gcStyle()); + gc.setAdvanced(advanced); + gc.setBackground(new Color(255, 255, 255)); + gc.fillRectangle(sample.getBounds()); + gc.setForeground(new Color(0, 0, 0)); + gc.setFont(fonts.get(row)); + gc.drawText(row.text(), 5, 5, row.drawFlags()); + gc.dispose(); + samples.put(row, sample); + } + return samples; + } + + public static void main(String[] args) { + // U+FFFE has no glyph in any standard font: even in the legacy fallback, + // SWT must still use GDI+'s DrawString/DrawDriverString for it. + String unsupportedGlyph = String.valueOf((char) 0xFFFE); + + java.util.List rows = new ArrayList<>(); + rows.add(new TextRow("Plain text", "Hello World")); + rows.add(new TextRow("Bold", SWT.BOLD, false, false, "Hello World", SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Italic", SWT.ITALIC, false, false, "Hello World", SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Underlined", SWT.NORMAL, true, false, "Hello World", SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Strikeout", SWT.NORMAL, false, true, "Hello World", SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Bold + underlined", SWT.BOLD, true, false, "Hello World", SWT.DRAW_TRANSPARENT, + SWT.NONE)); + rows.add(new TextRow("Underlined + unsupported glyph (U+FFFE)", SWT.NORMAL, true, false, + "Hi" + unsupportedGlyph, SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Mnemonic (accelerator underline)", SWT.NORMAL, false, false, "&File", + SWT.DRAW_MNEMONIC | SWT.DRAW_TRANSPARENT, SWT.NONE)); + rows.add(new TextRow("Tab-separated columns", SWT.NORMAL, false, false, "A\tB\tC", SWT.DRAW_TAB, + SWT.NONE)); + rows.add(new TextRow("Kerning pair", "AVATAR WAVE To Yes")); + rows.add(new TextRow("Mirrored / RTL", SWT.NORMAL, false, false, "Hello World", SWT.DRAW_TRANSPARENT, + SWT.RIGHT_TO_LEFT)); + rows.add(new TextRow("Arabic", "\u0645\u0631\u062d\u0628\u0627 \u0628\u0627\u0644\u0639\u0627\u0644\u0645")); + rows.add(new TextRow("Hebrew", "\u05e9\u05dc\u05d5\u05dd \u05e2\u05d5\u05dc\u05dd")); + rows.add(new TextRow("CJK (Chinese)", "\u4f60\u597d\u4e16\u754c")); + rows.add(new TextRow("Cyrillic", "\u041f\u0440\u0438\u0432\u0435\u0442 \u043c\u0438\u0440")); + rows.add(new TextRow("Greek", "\u0393\u03b5\u03b9\u03ac \u03c3\u03bf\u03c5 \u039a\u03cc\u03c3\u03bc\u03b5")); + rows.add(new TextRow("Combining diacritics", "e\u0301clat na\u0308\u0131ve")); + + Display display = new Display(); + Shell shell = new Shell(display); + shell.setLayout(new GridLayout()); + shell.setText("GDI+ text rendering comparison"); + + Label info = new Label(shell, SWT.WRAP); + info.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + + Button advancedCheckbox = new Button(shell, SWT.CHECK); + advancedCheckbox.setText("Use GDI+ (advanced) rendering - GC.setAdvanced(true); " + + "uncheck to compare against plain GDI (GC.setAdvanced(false))"); + advancedCheckbox.setSelection(true); + advancedCheckbox.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + + Button legacyCheckbox = new Button(shell, SWT.CHECK); + legacyCheckbox.setText("Use legacy GDI text rendering (org.eclipse.swt.internal.win32." + + "useGDITextRenderingWithGDIP = true) - historical, pre-#3091-fix behavior"); + legacyCheckbox.setSelection(Boolean.getBoolean(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY)); + legacyCheckbox.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + // The property only ever affects the advanced/GDI+ code path, so the + // checkbox is meaningless (and disabled) while advanced rendering is off. + legacyCheckbox.setEnabled(advancedCheckbox.getSelection()); + + ScrolledComposite scroller = new ScrolledComposite(shell, SWT.V_SCROLL | SWT.BORDER); + scroller.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + scroller.setExpandHorizontal(true); + scroller.setExpandVertical(true); + + int rowHeight = 42; + int labelWidth = 260; + int sampleWidth = 320; + int sampleHeight = rowHeight - 4; + + Canvas canvas = new Canvas(scroller, SWT.NONE); + canvas.setSize(labelWidth + sampleWidth + 20, rows.size() * rowHeight + 10); + scroller.setContent(canvas); + scroller.setMinSize(canvas.getSize()); + + Font systemFont = display.getSystemFont(); + Map fonts = new HashMap<>(); + for (TextRow row : rows) { + FontData fontData = systemFont.getFontData()[0]; + fontData.setStyle(row.fontStyle()); + if (row.underline()) fontData.data.lfUnderline = 1; + if (row.strikeout()) fontData.data.lfStrikeOut = 1; + fonts.put(row, new Font(display, fontData)); + } + + // Mutable holder so the checkbox listeners can swap in a freshly rendered + // set of samples (and dispose the previous ones) whenever the system + // property or the advanced/GDI+ toggle changes. + AtomicReference> samplesHolder = new AtomicReference<>( + renderSamples(display, rows, fonts, sampleWidth, sampleHeight, advancedCheckbox.getSelection())); + + Runnable refresh = () -> { + boolean advanced = advancedCheckbox.getSelection(); + legacyCheckbox.setEnabled(advanced); + boolean legacyFallback = advanced && legacyCheckbox.getSelection(); + System.setProperty(USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY, Boolean.toString(legacyFallback)); + + Map old = samplesHolder.getAndSet( + renderSamples(display, rows, fonts, sampleWidth, sampleHeight, advanced)); + old.values().forEach(Image::dispose); + + shell.setText("GDI+ text rendering comparison (advanced = " + advanced + + ", legacy GDI fallback property = " + legacyFallback + ")"); + info.setText("GC.setAdvanced(" + advanced + "); system property " + + USE_GDI_TEXT_RENDERING_WITH_GDIP_PROPERTY + " = " + legacyFallback + + "\nToggle the checkboxes above to compare plain GDI vs. GDI+ rendering, and - while GDI+ is" + + " enabled - the default (GDI+ DrawString) vs. legacy (GDI + DrawDriverString) text" + + " rendering path. See the source comment for what to expect per row."); + shell.layout(true, true); + canvas.redraw(); + }; + + advancedCheckbox.addListener(SWT.Selection, e -> refresh.run()); + legacyCheckbox.addListener(SWT.Selection, e -> refresh.run()); + refresh.run(); + + canvas.addPaintListener(e -> { + GC gc = e.gc; + int y = 5; + for (TextRow row : rows) { + gc.drawText(row.label(), 5, y + (rowHeight - 4) / 4, SWT.DRAW_TRANSPARENT); + gc.drawImage(samplesHolder.get().get(row), labelWidth, y); + y += rowHeight; + } + }); + + canvas.addDisposeListener(e -> { + samplesHolder.get().values().forEach(Image::dispose); + fonts.values().forEach(Font::dispose); + }); + + shell.setSize(700, 500); + shell.open(); + while (!shell.isDisposed()) { + if (!display.readAndDispatch()) { + display.sleep(); + } + } + display.dispose(); + } +}