[Win32] Fix tab stop width in GDI+ text rendering - #3518
Open
HeikoKlare wants to merge 1 commit into
Open
Conversation
Contributor
HeikoKlare
force-pushed
the
tab-stop-gdip-width
branch
from
August 15, 2026 11:39
64a0ba0 to
a11aa17
Compare
There was a problem hiding this comment.
Pull request overview
This pull request fixes an inconsistency in Win32 GC.drawText() when tabs are expanded under the GDI+ text-layout path, aligning tab stop width with Win32/GDI behavior (8× average character width) and adding Win32 tests plus a manual snippet to validate/compare rendering.
Changes:
- Win32: make GDI+ text-layout (
drawTextGDIP) useTEXTMETRIC.tmAveCharWidth * 8forSWT.DRAW_TABtab stops (and remove the now-unused space-measurement helper). - Win32 tests: add parameterized coverage to pin down tab stop width and ensure consistency across GDI vs GDI+ rendering.
- Examples: add
Snippet395and register it in the snippet index (plus exclude it from GTK/Cocoa snippet classpaths).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet395.java | Adds a Windows-focused manual/visual comparison snippet for GDI vs GDI+ text rendering behaviors (tabs, mnemonics, scripts, mirroring, etc.). |
| examples/org.eclipse.swt.snippets/Snippets.md | Registers Snippet395 in the snippet index with a preview link. |
| examples/org.eclipse.swt.snippets/.classpath_gtk | Excludes Snippet395 from the GTK snippet classpath (platform-specific snippet). |
| examples/org.eclipse.swt.snippets/.classpath_cocoa | Excludes Snippet395 from the Cocoa snippet classpath (platform-specific snippet). |
| bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java | Fixes GDI+ tab stop width to match Win32 convention and removes unused helper. |
| bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java | Adds parameterized tests to lock in correct tab stop semantics across rendering paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
HeikoKlare
force-pushed
the
tab-stop-gdip-width
branch
from
August 15, 2026 12:12
a11aa17 to
df7a0b1
Compare
The GDI+ based text rendering path in GC.drawText() derived the DRAW_TAB
tab stop width from the width of a single space glyph, while the GDI
based path and plain (non-advanced) GDI both use eight times the font's
average character width, which is the convention Win32's own DrawText()
and TabbedTextOut() follow.
Those are two different metrics, not two ways of computing the same one:
in proportional fonts a space is roughly half the average character
width, so tab stops came out about half as wide whenever the GDI+ path
was taken. In monospace fonts the two nearly coincide, which is why the
discrepancy was easy to miss.
The GDI+ path is changed to reuse the GDI text metric already computed
by its caller, so that tab stops are sized consistently no matter which
path renders the text. This restores the behaviour established for the
GDI path in bug 289244 ("GDI+ drawText is not consistent with GDI"),
which the GDI+ fallback path reintroduced in bug 305815 had missed.
Add tests that pin down the tab stop width itself (eight average
character widths), its consistency across both rendering paths, and two
properties that must hold for either engine: consecutive tabs advance by
whole tab stops, and text following a tab starts at the same tab stop
regardless of what precedes the tab. They are measured from a leading
tab, which expands to exactly one tab stop and so is not diluted by the
slightly different glyph advances of the two engines, and cover both
proportional and monospace fonts.
Add Snippet395 as a manual/visual companion to the automated tests: it
renders a series of text properties, one row per property, and offers a
checkbox to switch between plain GDI and GDI+ (advanced) rendering at
runtime, without restarting, to visually compare the resulting output.
Assisted-by: Claude Opus 5 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
tab-stop-gdip-width
branch
from
August 15, 2026 12:25
df7a0b1 to
b222ad5
Compare
HeikoKlare
marked this pull request as ready for review
August 15, 2026 12:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an extract of #3100, which removes the GDI fallback in
GC.drawText()entirely. That change makes the GDI+ text rendering path the default for all text, which turns the tab stop defect described below from a rarely reachable corner case into something every consumer ofDRAW_TABwould hit. It is therefore worth fixing separately and up front.What is wrong
GC.drawText()on Win32 renders text in one of three ways, and all three must agree on how wide aSWT.DRAW_TABtab stop is:OS.DrawText()withDT_EXPANDTABSdrawTextGDIP()Paths 1 and 2 use eight times the font's average character width. Path 2 does so explicitly:
Path 3, however, derived its tab stop from the width of a single space glyph:
Those are two different metrics, not two ways of computing the same one. In a proportional font a space is roughly half the average character width, so tab stops in path 3 came out about half as wide. The original commit message for this change blamed "different engines computing the same metric differently" — the measurements below disproved that, and the description was corrected: GDI+ measures the space glyph almost identically to GDI (e.g. Segoe UI 9pt: 3.3 px vs. 3 px). The bug is the choice of metric.
Eight average character widths is not an arbitrary SWT convention, it is what Win32 itself does:
TabbedTextOut(): "If the nTabPositions parameter is zero and lpnTabStopPositions is NULL, tabs are expanded to eight times the average character width."DRAWTEXTPARAMS.iTabLength: "The size of each tab stop, in units equal to the average character width."x-relative computation in path 2.DrawTextExW()astabwidth = tm.tmAveCharWidth * tabstop(defaulttabstop = 8) withplen = ((plen/tabwidth)+1)*tabwidth— character for character the formula used by path 2.How the change fixes it
drawText()already computes aTEXTMETRICfor the current font before deciding which path to take. That metric is now passed on todrawTextGDIP()and used for the tab stops, so path 3 uses the same value as paths 1 and 2:The now-unused
measureSpace()helper is removed. The metric is coherent with the font GDI+ actually uses: in the common case theTEXTMETRICcomes from the sameHFONTthe GDI+ font was created from, and in the font-substitution case (!Font_IsAvailable)data.hGDIFontis round-tripped from the GDI+ font viaFont_GetLogFontW(), so it still describes the substituted font.GCWin32Testsgains four parameterized tests, run over both proportional and monospace fonts. They pin down the tab stop width itself (eight average character widths), its consistency across the rendering paths, and two properties that must hold for either engine: consecutive tabs advance by whole tab stops, and text following a tab starts at the same tab stop regardless of what precedes it. They were verified to fail on the unfixed code for every font, including the monospace ones.Users drawing tabbed text through an advanced
GCwill see tab stops become wider — that is, correct, and matching what the same text renders as withoutsetAdvanced(true).What the history of this code tells us
This is not a new judgment call — it restores a decision the project already made, and repairs a regression:
63863d93c2, Silenio Quarti). This commit deleted themeasureSpace(...) * 8line and replaced it withint tabWidth = lptm.tmAveCharWidth * 8;. In other words, the value this PR switches to is SWT's own answer to precisely the question "how do we make GDI+ tabs consistent with GDI".7e842a36af). To fix Chinese text rendering, this re-introduced the pre-289244 GDI+ code verbatim as a fallback path for strings GDI cannot handle — resurrectingmeasureSpace()and its tab stop line along with it. The 2009 fix was silently undone for that path.So the defect has been latent for ~16 years. Two things kept it from being noticed:
drawTextGDIP(), so most applications never see it — until [Win32] Remove obsolete fallback for drawing text without GDI+ #3100 makes it the only path.Worth noting for reviewers: the fallback path from bug 305815 is a verbatim copy of decade-old code. It is worth assuming that other fixes made to the GDI path between 2009 and 2010 were lost in the same way, and that this may not be the only such discrepancy.
Measurements
Measured on Windows 11, 96 DPI, by reproducing
drawTextGDIP()'s tab stop handling standalone (directGdipcalls against a shipped SWT build), so that the old and the new value could be compared against what Win32DrawText(DT_EXPANDTABS)actually produces. All values are the width in pixels that a single leading tab expands to.8 * tmAveCharWidthDrawTextAcross all 21 font/size combinations tested:
DrawText's tab width equals8 * tmAveCharWidthexactly — the documented convention is also the observed behaviour.DrawTextexactly, not merely within a tolerance.Two further properties were confirmed empirically and are covered by the added tests:
StringFormat::SetTabStops(0, 1, {w})places stops atw, 2w, 3w, …, i.e. identical semantics to path 2's((position / w) + 1) * w. The value swap does not change the geometry model.One risk was explicitly ruled out:
Graphics_MeasureString()is not affected by the GDI+ world transform, so the previous space-based value was equally transform-independent and replacing it with a device-pixel metric introduces no regression underGC.setTransform(). This is consistent withcreateGdipFont(), which already neutralises the world transform when deriving theLOGFONTforGetTextMetrics.Snippet
Snippet395is added as a manual/visual companion to the automated tests. It renders a series of text properties one row per property, and offers a checkbox that switches between plain GDI and GDI+ (advanced) rendering at runtime without restarting, so the two engines can be compared directly. Besides the two tab rows it also covers underline/strikeout, mnemonics, kerning, mirroring and a range of scripts, which makes it useful well beyond this fix — in particular for reviewing #3100.🤖 Generated with Claude Code