From 47f1167af2b30d41d771118b8f5bc73945cca079 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 22 Sep 2022 00:30:15 +0200 Subject: [PATCH] AtlasEngine: Fix uneven baselines when scaling glyphs (#14039) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 97dc5c8d754495d68996d341df961c9515b05804) Service-Card-Id: 85767343 Service-Version: 1.16 --- src/renderer/atlas/AtlasEngine.h | 2 +- src/renderer/atlas/AtlasEngine.r.cpp | 73 +++++++++++++--------------- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index 1c58d351c2e..7e417c21544 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -523,9 +523,9 @@ namespace Microsoft::Console::Render struct CachedGlyphLayout { wil::com_ptr textLayout; - f32x2 halfSize; f32x2 offset; f32x2 scale; + f32x2 scaleCenter; D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE; bool scalingRequired = false; diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index e727570a4a4..381f43de4d6 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -472,10 +472,10 @@ void AtlasEngine::_drawGlyph(const TileHashMap::iterator& it) const AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* chars, u16 charsLength, u16 cellCount, IDWriteTextFormat* textFormat, bool coloredGlyph) const { const f32x2 layoutBox{ cellCount * _r.cellSizeDIP.x, _r.cellSizeDIP.y }; - const f32x2 halfSize{ layoutBox.x * 0.5f, layoutBox.y * 0.5f }; bool scalingRequired = false; f32x2 offset{ 0, 0 }; f32x2 scale{ 1, 1 }; + f32x2 scaleCenter; // See D2DFactory::DrawText wil::com_ptr textLayout; @@ -550,12 +550,18 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* static_cast(glyphMetrics.advanceHeight) / static_cast(metrics.designUnitsPerEm) * _r.fontMetrics.fontSizeInDIP, }; + scalingRequired = true; + // We always want box drawing glyphs to be centered in their cell. + offset.x = (layoutBox.x - boxSize.x) * 0.5f; + offset.y = (layoutBox.y - boxSize.y) * 0.5f; // We always want box drawing glyphs to exactly match the size of a terminal cell. - // So for safe measure we'll always scale them to the exact size. // But add 1px to the destination size, so that we don't end up with fractional pixels. - scalingRequired = true; - scale.x = layoutBox.x / boxSize.x; - scale.y = layoutBox.y / boxSize.y; + scale.x = (layoutBox.x + _r.dipPerPixel) / boxSize.x; + scale.y = (layoutBox.y + _r.dipPerPixel) / boxSize.y; + // Now that the glyph is in the center of the cell thanks + // to the offset, the scaleCenter is center of the cell. + scaleCenter.x = layoutBox.x * 0.5f; + scaleCenter.y = layoutBox.y * 0.5f; } } else @@ -563,16 +569,7 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* DWRITE_OVERHANG_METRICS overhang; THROW_IF_FAILED(textLayout->GetOverhangMetrics(&overhang)); - 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), - }; - f32x2 actualSize{ - layoutBox.x + overhang.left + overhang.right, - layoutBox.y + overhang.top + overhang.bottom, - }; + auto actualSizeX = layoutBox.x + overhang.left + overhang.right; // Long glyphs should be drawn with their proper design size, even if that makes them a bit blurry, // because otherwise we fail to support "pseudo" block characters like the "===" ligature in Cascadia Code. @@ -585,8 +582,7 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* const auto advanceScale = _r.fontMetrics.advanceScale; scalingRequired = true; scale = { advanceScale, advanceScale }; - actualSize.x *= advanceScale; - actualSize.y *= advanceScale; + actualSizeX *= advanceScale; } // We need to offset glyphs that are simply outside of our layout box (layoutBox.x/.y) @@ -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); + scaleCenter.x = offset.x; + scaleCenter.y = _r.fontMetrics.baselineInDIP; - if ((actualSize.x - layoutBox.x) > _r.dipPerPixel) + if ((actualSizeX - layoutBox.x) > _r.dipPerPixel) { scalingRequired = true; offset.x = (overhang.left - overhang.right) * 0.5f; - scale.x = layoutBox.x / actualSize.x; + scale.x = layoutBox.x / actualSizeX; scale.y = scale.x; + scaleCenter.x = layoutBox.x * 0.5f; } - if ((actualSize.y - layoutBox.y) > _r.dipPerPixel) + if (overhang.top > _r.dipPerPixel || overhang.bottom > _r.dipPerPixel) { + const auto descend = _r.cellSizeDIP.y - _r.fontMetrics.baselineInDIP; + const auto scaleTop = _r.fontMetrics.baselineInDIP / (_r.fontMetrics.baselineInDIP + overhang.top); + const auto scaleBottom = descend / (descend + overhang.bottom); scalingRequired = true; - offset.y = (overhang.top - overhang.bottom) * 0.5f; - scale.x = std::min(scale.x, layoutBox.y / actualSize.y); + scale.x = std::min(scale.x, std::min(scaleTop, scaleBottom)); scale.y = scale.x; } - - // 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? - // It calculates the new `baseline` height after transformation by `scale.y` relative to the center point `halfSize.y`. - // - // This works even if `scale.y == 1`, because then `baseline == baselineInDIP + offset.y` and `baselineInDIP` - // is always measured in full pixels. So rounding it will be equivalent to just rounding `offset.y` itself. - const auto baseline = halfSize.y + (_r.fontMetrics.baselineInDIP + offset.y - halfSize.y) * scale.y; - // This rounds to the nearest multiple of _r.dipPerPixel. - const auto baselineFixed = roundf(baseline * _r.pixelPerDIP) * _r.dipPerPixel; - offset.y += (baselineFixed - baseline) / scale.y; } auto options = D2D1_DRAW_TEXT_OPTIONS_NONE; @@ -672,11 +661,19 @@ AtlasEngine::CachedGlyphLayout AtlasEngine::_getCachedGlyphLayout(const wchar_t* // where every single "=" might be blatantly misaligned vertically (same for any box drawings). WI_SetFlagIf(options, D2D1_DRAW_TEXT_OPTIONS_NO_SNAP, scalingRequired); + // ClearType basically has a 3x higher horizontal resolution. To make our glyphs render the same everywhere, + // it's probably for the best to ensure we initially rasterize them on a whole pixel boundary. + // (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? + offset.y = roundf(offset.y * _r.pixelPerDIP) * _r.dipPerPixel; + return CachedGlyphLayout{ .textLayout = textLayout, - .halfSize = halfSize, .offset = offset, .scale = scale, + .scaleCenter = scaleCenter, .options = options, .scalingRequired = scalingRequired, }; @@ -790,8 +787,8 @@ void AtlasEngine::CachedGlyphLayout::applyScaling(ID2D1RenderTarget* d2dRenderTa 0, 0, scale.y, - (origin.x + halfSize.x) * (1.0f - scale.x), - (origin.y + halfSize.y) * (1.0f - scale.y), + (origin.x + scaleCenter.x) * (1.0f - scale.x), + (origin.y + scaleCenter.y) * (1.0f - scale.y), }; d2dRenderTarget->SetTransform(&transform); }