AtlasEngine: Clip box glyphs to their cells #15343
Conversation
lhecker
force-pushed
the
dev/lhecker/atlas-engine-box-glyphs
branch
2 times, most recently
from
1e4b8dc to
81ac511
Compare
|
Comparison, Cascadia Code 12pt (note the lines in the left half in particular):
|
lhecker
added
the
Issue-Bug
microsoft-github-policy-service
bot
added
the
Needs-Author-Feedback
lhecker
force-pushed
the
dev/lhecker/atlas-engine-box-glyphs
branch
from
81ac511 to
37a63b7
Compare
microsoft-github-policy-service
bot
removed
the
Needs-Author-Feedback
| if (needsTransform) | ||
| { | ||
| transform.dx = (1.0f - transform.m11) * baselineOrigin.x; | ||
| transform.dy = (1.0f - transform.m22) * baselineOrigin.y; | ||
| _d2dRenderTarget->SetTransform(&transform); | ||
| } |
There was a problem hiding this comment.
This code moved down from line 1302, because calculating the PushAxisAlignedClip rectangle is much simpler that way (the transform is still in absolute coordinates, instead of being relative to the baseline origin of the glyph).
| ALLOW_UNINITIALIZED_BEGIN | ||
| std::array<u32, 0x100> codepoints; | ||
| std::array<u16, 0x100> indices; | ||
| ALLOW_UNINITIALIZED_END |
There was a problem hiding this comment.
This avoids zero-initializing 1536 bytes of stack. The cost of that alone is almost as much as the entire cost of the function lol (apart from GetGlyphIndicesW which we don't have much control over).
| codepoints[i] = 0x2500 + i; | ||
| } | ||
|
|
||
| THROW_IF_FAILED(fontFaceEntry.fontFace->GetGlyphIndicesW(codepoints.data(), codepoints.size(), indices.data())); |
There was a problem hiding this comment.
so I understand: this function caches every glyph ID from the font that mapped to a box drawing character?
There was a problem hiding this comment.
Yep exactly. The alternative is to check the codepoints during text shaping, but I felt like that would be just as bad for code complexity. This approach has the additional benefit that it only needs to run the box-glyph specific logic once, when the glyph is rasterized, and not every time the text is shaped. Just like the alternative approach (checking codepoints) it only works for basic cmap mappings though (so no substitutions, etc.).
Yeah, it doesn't. It wouldn't be too difficult to do the same codepoint-caching trick for BackendD2D though, if we ever need the D2D text renderer to be better. |
|
@lhecker I am marking this one up for 1.18 |
Overhangs for box glyphs can produce unsightly effects, where the
antialiased edges of horizontal and vertical lines overlap between
neighboring glyphs and produce "boldened" intersections.
This avoids the issue in most cases by simply clipping the glyph to the
size of a single cell. The downside is that it fails to work well for
custom line heights, etc.
## Validation Steps Performed
* With Cascadia Code, printing ``"`u{2593}`n`u{2593}"`` in pwsh
doesn't produce a brightened overlap anymore ✅
* ``"`e#3`u{2502}`n`e#4`u{2502}"`` produces a fat vertical line ✅
(cherry picked from commit 4628ceb)
Service-Card-Id: 89213363
Service-Version: 1.18
Overhangs for box glyphs can produce unsightly effects, where the
antialiased edges of horizontal and vertical lines overlap between
neighboring glyphs and produce "boldened" intersections.
This avoids the issue in most cases by simply clipping the glyph to the
size of a single cell. The downside is that it fails to work well for
custom line heights, etc.
Validation Steps Performed
"`u{2593}`n`u{2593}"in pwshdoesn't produce a brightened overlap anymore ✅
"`e#3`u{2502}`n`e#4`u{2502}"produces a fat vertical line ✅