fix(textbox): subpixel gaps between adjacent selected characters#22962
Open
ramezgerges wants to merge 4 commits intounoplatform:masterfrom
Open
fix(textbox): subpixel gaps between adjacent selected characters#22962ramezgerges wants to merge 4 commits intounoplatform:masterfrom
ramezgerges wants to merge 4 commits intounoplatform:masterfrom
Conversation
github-actions
bot
added
the
area/skia ✏️
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets Skia text rendering for UnicodeText to reduce visible seams/gaps between adjacent highlighted/selected character clusters by changing how background rectangles are pixel-aligned.
Changes:
- Adjusts highlight background rectangle X rounding logic (switching from
RoundtoFloorand extending right edge). - Adds
Console.WriteLinecalls at the start/end ofDraw(these should not ship in rendering code).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-22962/wasm-skia-net9/index.html |
Contributor
|
The build 205461 found UI Test snapshots differences: Details
|
ramezgerges
force-pushed
the
textbox_selection_gaps
branch
from
f1502dc to
02b39e8
Compare
Contributor
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-22962/wasm-skia-net9/index.html |
Contributor
|
The build 205476 found UI Test snapshots differences: Details
|
Contributor
|
|
Xiaoy312
approved these changes
Apr 4, 2026
Contributor
Xiaoy312
left a comment
Xiaoy312
left a comment
There was a problem hiding this comment.
<3, that was so annoying!
Contributor
|
The build 205476 found UI Test snapshots differences: Details
|
MartinZikmund
approved these changes
Apr 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5112
to
+5118
| // Text with spaces that can produce cluster boundaries at exact | ||
| // integer coordinates, which previously caused 1-pixel gaps in | ||
| // the selection background due to rounding/antialiasing issues. | ||
| var SUT = new TextBox | ||
| { | ||
| Text = "____", | ||
| FontSize = 14, |
There was a problem hiding this comment.
The comment above this test says it relies on “Text with spaces”, but the actual test text is "____" (underscores). Please either update the text to match the intended reproduction case, or adjust the comment to reflect why underscores are the right input for triggering cluster-boundary rounding issues.
Comment on lines
+5131
to
+5135
| for (int i = 0; i < 20; i++) | ||
| { | ||
| screenshot.GetPixel(i, screenshot.Height / 2).Should().Be(Colors.Red, $"Selection background should have no gaps at x={i}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
This assertion checks only the first 20 pixels of the screenshot, so it doesn’t actually validate “no gaps between characters” across the whole selected text (a gap could exist later and the test would still pass). It’s also sensitive to template padding/offsets because it assumes the selection background starts at x=0. Consider deriving the expected highlight span from the screenshot (e.g., find the highlight bounds first) and then asserting there are no non-highlight pixels within that span.
Suggested change
| for (int i = 0; i < 20; i++) | |
| { | |
| screenshot.GetPixel(i, screenshot.Height / 2).Should().Be(Colors.Red, $"Selection background should have no gaps at x={i}"); | |
| } | |
| } | |
| var y = screenshot.Height / 2; | |
| var startX = Enumerable.Range(0, screenshot.Width) | |
| .FirstOrDefault(x => screenshot.GetPixel(x, y) == Colors.Red); | |
| var endX = Enumerable.Range(0, screenshot.Width) | |
| .LastOrDefault(x => screenshot.GetPixel(x, y) == Colors.Red); | |
| (startX <= endX).Should().BeTrue("Selection background should be visible on the sampled scanline."); | |
| for (int x = startX; x <= endX; x++) | |
| { | |
| screenshot.GetPixel(x, y).Should().Be(Colors.Red, $"Selection background should have no gaps between x={startX} and x={endX}; found a gap at x={x}"); | |
| } | |
| } |
Comment on lines
+5131
to
+5133
| for (int i = 0; i < 20; i++) | ||
| { | ||
| screenshot.GetPixel(i, screenshot.Height / 2).Should().Be(Colors.Red, $"Selection background should have no gaps at x={i}"); |
There was a problem hiding this comment.
The test compares pixel colors with exact equality (Should().Be(Colors.Red)), which is brittle with antialiasing and subpixel blending at highlight edges. To reduce flakiness, prefer an assertion that allows a small tolerance (or sample pixels away from edges / validate a rectangle region using ImageAssert’s tolerance helpers).
Suggested change
| for (int i = 0; i < 20; i++) | |
| { | |
| screenshot.GetPixel(i, screenshot.Height / 2).Should().Be(Colors.Red, $"Selection background should have no gaps at x={i}"); | |
| const byte colorTolerance = 8; | |
| for (int i = 0; i < 20; i++) | |
| { | |
| var pixel = screenshot.GetPixel(i, screenshot.Height / 2); | |
| pixel.A.Should().BeInRange((byte)Math.Max(0, Colors.Red.A - colorTolerance), (byte)Math.Min(255, Colors.Red.A + colorTolerance), $"Selection background alpha should have no gaps at x={i}"); | |
| pixel.R.Should().BeInRange((byte)Math.Max(0, Colors.Red.R - colorTolerance), (byte)Math.Min(255, Colors.Red.R + colorTolerance), $"Selection background red channel should have no gaps at x={i}"); | |
| pixel.G.Should().BeInRange((byte)Math.Max(0, Colors.Red.G - colorTolerance), (byte)Math.Min(255, Colors.Red.G + colorTolerance), $"Selection background green channel should have no gaps at x={i}"); | |
| pixel.B.Should().BeInRange((byte)Math.Max(0, Colors.Red.B - colorTolerance), (byte)Math.Min(255, Colors.Red.B + colorTolerance), $"Selection background blue channel should have no gaps at x={i}"); |
| { | ||
| screenshot.GetPixel(i, screenshot.Height / 2).Should().Be(Colors.Red, $"Selection background should have no gaps at x={i}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Indentation for the closing brace is inconsistent here (spaces instead of the surrounding tabs). Please align it with the file’s existing indentation to avoid noisy diffs and keep formatting consistent.
Suggested change
| } | |
| } |
Comment on lines
+5107
to
+5109
| [TestMethod] | ||
| public async Task When_Selection_Background_Has_No_Gaps_Between_Characters() | ||
| { |
There was a problem hiding this comment.
The PR description currently includes closes # without an issue number/link. Please update it to use a fully-qualified issue URL (e.g., Closes https://github.com/unoplatform/uno/issues/<id>) or note explicitly that there is no related issue and who approved it, per the repo PR requirements.
Contributor
|
The build 206090 found UI Test snapshots differences: Details
|
Contributor
|
The build 206090 found UI Test snapshots differences: Details
|
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
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.
GitHub Issue: closes #
PR Type:
What is the current behavior? 🤔
What is the new behavior? 🚀
PR Checklist ✅
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information ℹ️