AtlasEngine: Fix uneven baselines when scaling glyphs #14039
Conversation
|
|
ghost
added
Area-AtlasEngine
Area-Fonts
|
in "new" the vertical stems of letters (primarily |
|
Ah woops... "Old" is using ClearType and "New" isn't, which makes "Old" appear more crisp. If you open the image in a new tab and zoom in, you can see that the stems are practically identical. |
|
Well, it looks less weird... ! |
|
Yep, agreed. I'll try to make it work properly and as expected without downsizing in a follow-up PR, but that's probably only for 1.17 by then. |
| @@ -627,34 +623,27 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* | |||
| // --> offsetY = 0 | |||
| // --> scale = layoutBox.y / (layoutBox.y + left + right) | |||
| // = 0.69f | |||
| offset.x = clampedOverhang.left - clampedOverhang.right; | |||
| offset.y = clampedOverhang.top - clampedOverhang.bottom; | |||
| offset.x = std::max(0.0f, overhang.left) - std::max(0.0f, overhang.right); | |||
There was a problem hiding this comment.
This changes how overhangs are dealt with; do we need to update the explanatory comment above? Specifically I notice that the "bar diacritic" part has a negative left value...
There was a problem hiding this comment.
No the big comment is still up to date. This particular change (for offset.x) is equivalent to the old code. clampedOverhang used to be computed as:
const DWRITE_OVERHANG_METRICS clampedOverhang{
std::max(0.0f, overhang.left),
std::max(0.0f, overhang.top),
std::max(0.0f, overhang.right),
std::max(0.0f, overhang.bottom),
};| } | ||
| if ((actualSize.y - layoutBox.y) > _r.dipPerPixel) | ||
| if (overhang.top > _r.dipPerPixel || overhang.bottom > _r.dipPerPixel) |
There was a problem hiding this comment.
this is after the overhangs have been sign-adjusted, right?
There was a problem hiding this comment.
You mean clamped? In that case: No, these are the raw overhangs and a positive value indicates that the glyph clips outside of the layout box we've given it.
| // (https://en.wikipedia.org/wiki/ClearType#How_ClearType_works) | ||
| offset.x = roundf(offset.x * _r.pixelPerDIP) * _r.dipPerPixel; | ||
| // As explained below, we use D2D1_DRAW_TEXT_OPTIONS_NO_SNAP to prevent a weird issue with baseline snapping. | ||
| // But we do want it technically, so this re-implements baseline snapping... I think? |
There was a problem hiding this comment.
reviewer note: comment is not new, don't think too hard about the `... I think?"
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
This commit changes the glyph scale algorithm to prefer aligning glyphs to their baseline. This improves the visual appearance of simulated italic glyphs. However wide Emojis in narrow cells now look slightly worse without centering. Closes #13987 ## Validation Steps Performed * Use FiraCode which has no italic variant and instead uses simulated italics * Write italic text * Baseline is consistent ✅ (cherry picked from commit 97dc5c8) Service-Card-Id: 85767343 Service-Version: 1.16
|
🎉 Handy links: |
ghost
mentioned this pull request
This commit changes the glyph scale algorithm to prefer aligning glyphs to
their baseline. This improves the visual appearance of simulated italic glyphs.
However wide Emojis in narrow cells now look slightly worse without centering.
Closes #13987
Validation Steps Performed