[win32] Resolve tab stops entirely in points in TextLayout.computeRuns() - #3495
Draft
vogella wants to merge 2 commits into
Draft
[win32] Resolve tab stops entirely in points in TextLayout.computeRuns()#3495vogella wants to merge 2 commits into
vogella wants to merge 2 commits into
Conversation
TextLayout.setTabs() takes the tab stops in points, but computeRuns() converted them to pixels and compared them against a pen position accumulated from raw glyph advances. At zoom levels that are not a multiple of 100 the point/pixel round trip can place a stop one pixel past a pen position that actually sits exactly on that stop, so the tab advanced by a single pixel instead of moving to the next stop. StyledText hits this because it measures its tab width as the width of N spaces in points and passes that back as the only tab stop, so a tab following N spaces lands precisely on the stop. It became visible in 4.36 when monitor-specific scaling turned the effective auto-scale from "integer" into "quarter", exposing zoom 125, 150 and 175. Only the selection of the stop was ever wrong, so both comparisons that ask whether a stop is still ahead of the pen now happen in points, the unit the stops were defined in. Once a stop has been picked, its pixel position is still used unchanged for the run width, and the block that adjusts merged consecutive tabs is untouched. Fixes eclipse-platform/eclipse.platform.ui#3052
The tab stop handling mixed units: the stop was selected in points while the resulting position and the adjustment for merged consecutive tabs were accumulated from the pixel-converted stops. That works, but every step past the last stop rounds separately, so the positions can drift by a pixel from the stop the caller defined, and reasoning about the block requires tracking two units at once. Keep the whole computation in points and convert once, when the final position is known. This removes the pixel copy of the stops and makes the merged-tab adjustment operate on the same values as the lookup. The degenerate case of a non-increasing tabs array now leaves the run at its measured glyph advance rather than adding a non-positive stop delta to it, which was meaningless anyway. No behavior change for well-formed tab stops.
Contributor
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
1 participant
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.
Stacked on #3493, which contains the actual fix. Only the second commit belongs to this PR; it will show on its own once #3493 is merged.
The tab stop handling in
TextLayout.computeRuns()mixes units: the stop is selected in points while the resulting position and the adjustment for merged consecutive tabs are accumulated from the pixel-converted stops. This keeps the whole computation in points and converts once, when the final position is known, so the pixel copy of the stops disappears and the merged-tab adjustment works on the same values as the lookup.Besides being easier to follow, it removes a rounding drift: each step past the last tab stop currently rounds separately, so the positions can end up a pixel away from the stop the caller defined. No behavior change for well-formed tab stops.