From 7701b89b458cbc85af3e5392c2f3458f1d24882c Mon Sep 17 00:00:00 2001 From: "Klare, Heiko" Date: Wed, 25 Feb 2026 15:38:20 +0100 Subject: [PATCH] [Win32] Disable obsolete fallback for drawing text without GDI+ A check in GC.drawText() makes the implementation fall back to the text rendering based on glyphs and their positions calculated by GDI even when GDI+ is used in all cases except when it contains specific characters that GDI cannot handle. This was necessary more than a decade ago because specific Chinese characters were not properly rendered by GDI+. But this is quite non-intuitive as in case GDI+/advance mode is enabled the consumer will usually expected GDI+ to be used for rendering. In addition, the problematic scenario from back then works fine now. This change thus adapts the GC.drawText() implementation to use plain GDI+ whenever it is enabled. The fallback code is preserved but only executed if a newly introduced system property is enabled. This allows to switch back to previous behavior in case unexpected regressions are found. The property is to be removed in a future release in case no regressions are found. While adding regression tests for this change it was found that the GDI+ based drawText() path computed the DRAW_TAB tab stop width from the width of a single space glyph as measured by GDI+, while the legacy GDI-based path and plain (non-advanced) GDI both use the GDI text metric's average character width. These two metrics can differ substantially, which made tab stops shrink visibly (roughly by half) compared to plain GDI and the legacy fallback whenever GDI+ rendering was used. The GDI+ path is changed to reuse the same GDI text metric already computed by its caller, so tab stops are now sized consistently across all three rendering paths (plain GDI, default GDI+, and legacy GDI+ fallback). Extend GCWin32Tests with additional regression tests covering not only the previously fixed underline/strikeout invisibility issue, but also style-decoration combinations, mnemonic underlining, mirrored/RTL text, complex scripts and charsets (Arabic, Hebrew, CJK, Cyrillic, Greek, combining diacritics), kerning-sensitive text width, and tab stop expansion; all comparing the default and legacy (system property enabled) rendering paths end-to-end. Add Snippet395 as a manual/visual companion to the automated tests: it renders the same set of text properties side-by-side and offers two checkboxes to toggle at runtime, without restarting, between plain GDI and GDI+ (advanced) rendering, and - while GDI+ is active - between the default and legacy text rendering path, to visually compare the resulting output. May fix https://github.com/eclipse-platform/eclipse.platform.swt/issues/3091 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../eclipse/swt/graphics/GCWin32Tests.java | 420 +++++++++++++++++- .../win32/org/eclipse/swt/graphics/GC.java | 35 +- .../org.eclipse.swt.snippets/.classpath_cocoa | 2 +- .../org.eclipse.swt.snippets/.classpath_gtk | 2 +- .../org/eclipse/swt/snippets/Snippet395.java | 254 +++++++++++ 5 files changed, 700 insertions(+), 13 deletions(-) create mode 100644 examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet395.java 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(); + } +}