Allow drawing the selected text background earlier #163
Conversation
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
martincapello
force-pushed
the
draw-text-shaper-improvement
branch
from
f374506 to
6db938c
Compare
dacap
requested changes
Nov 7, 2025
Member
dacap
left a comment
dacap
left a comment
There was a problem hiding this comment.
I didn't test this yet, so here are some minor changes.
The DrawTextDelegate is deprecated and we'd like to remove it in the future, but it's still used in ui::Entry so it's fine to fix bugs on it 👍
martincapello
force-pushed
the
draw-text-shaper-improvement
branch
from
6db938c to
08332c8
Compare
dacap
requested changes
Nov 17, 2025
Member
dacap
left a comment
dacap
left a comment
There was a problem hiding this comment.
A minor change, I've tested on Linux and macOS with "Apple Chancery" and it works great 👍
text/draw_text_shaper.cpp
Outdated
Comment on lines
54
to
57
| std::vector<gfx::RectF> glyphsBounds; | ||
| for (int i = 0; i < info.glyphCount; ++i) { | ||
| auto bounds = info.getGlyphBounds(i); | ||
| glyphsBounds.push_back(bounds); |
Member
There was a problem hiding this comment.
We can prealloc the expected size of glyphsBounds:
Suggested change
| std::vector<gfx::RectF> glyphsBounds; | |
| for (int i = 0; i < info.glyphCount; ++i) { | |
| auto bounds = info.getGlyphBounds(i); | |
| glyphsBounds.push_back(bounds); | |
| std::vector<gfx::RectF> glyphsBounds(info.glyphCount); | |
| for (int i = 0; i < info.glyphCount; ++i) { | |
| auto bounds = info.getGlyphBounds(i); | |
| glyphsBounds[i] = bounds; |
martincapello
force-pushed
the
draw-text-shaper-improvement
branch
from
08332c8 to
822afab
Compare
Add new methods to DrawTextDelegate to allow drawing the background of the selected text before drawing each character. This fixes an issue when the glyphs used by a TTF were overlapped.
martincapello
force-pushed
the
draw-text-shaper-improvement
branch
from
822afab to
866cf6e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 PR modifies the order in which some things are done when each run of text is processed when the shaping function is executed:
Both of the above things are possible because now we first calculate the union of all glyphs bounds.