From 7a891dcbe1f74a3295d57d546a15c424f5ac3f6e Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 15 Sep 2022 22:38:08 +0200 Subject: [PATCH] AtlasEngine: Fix bugs around bitmap font rendering --- src/renderer/atlas/AtlasEngine.api.cpp | 11 +++++++---- src/renderer/atlas/AtlasEngine.cpp | 14 +++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 8861704b5f6..81c8e7bf349 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -620,7 +620,6 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo const auto designUnitsPerPx = fontSizeInPx / static_cast(metrics.designUnitsPerEm); const auto ascent = static_cast(metrics.ascent) * designUnitsPerPx; const auto descent = static_cast(metrics.descent) * designUnitsPerPx; - const auto lineGap = static_cast(metrics.lineGap) * designUnitsPerPx; const auto underlinePosition = static_cast(-metrics.underlinePosition) * designUnitsPerPx; const auto underlineThickness = static_cast(metrics.underlineThickness) * designUnitsPerPx; const auto strikethroughPosition = static_cast(-metrics.strikethroughPosition) * designUnitsPerPx; @@ -628,9 +627,13 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo const auto advanceWidth = static_cast(glyphMetrics.advanceWidth) * designUnitsPerPx; - const auto halfGap = lineGap / 2.0f; - const auto baseline = std::roundf(ascent + halfGap); - const auto lineHeight = std::roundf(baseline + descent + halfGap); + // NOTE: Line-gaps shouldn't be taken into account for lineHeight calculations. + // Terminals don't really have "gaps" between lines and instead the expectation + // is that two full block characters above each other don't leave any gaps + // between the lines. "Terminus TTF" for instance sets a line-gap of 90 units + // even though its font bitmap only covers the ascend/descend height. + const auto baseline = std::roundf(ascent); + const auto lineHeight = std::roundf(baseline + descent); const auto underlinePos = std::roundf(baseline + underlinePosition); const auto underlineWidth = std::max(1.0f, std::roundf(underlineThickness)); const auto strikethroughPos = std::roundf(baseline + strikethroughPosition); diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp index 4bdde3da691..ae4b1f8e2ed 100644 --- a/src/renderer/atlas/AtlasEngine.cpp +++ b/src/renderer/atlas/AtlasEngine.cpp @@ -1102,7 +1102,6 @@ void AtlasEngine::_recreateFontDependentResources() auto& textFormat = _r.textFormats[italic][bold]; THROW_IF_FAILED(_sr.dwriteFactory->CreateTextFormat(_api.fontMetrics.fontName.c_str(), _api.fontMetrics.fontCollection.get(), fontWeight, fontStyle, DWRITE_FONT_STRETCH_NORMAL, _api.fontMetrics.fontSizeInDIP, L"", textFormat.put())); - THROW_IF_FAILED(textFormat->SetTextAlignment(DWRITE_TEXT_ALIGNMENT_CENTER)); THROW_IF_FAILED(textFormat->SetWordWrapping(DWRITE_WORD_WRAPPING_NO_WRAP)); // DWRITE_LINE_SPACING_METHOD_UNIFORM: @@ -1111,11 +1110,16 @@ void AtlasEngine::_recreateFontDependentResources() // We want that. Otherwise fallback fonts might be rendered with an incorrect baseline and get cut off vertically. THROW_IF_FAILED(textFormat->SetLineSpacing(DWRITE_LINE_SPACING_METHOD_UNIFORM, _r.cellSizeDIP.y, _api.fontMetrics.baselineInDIP)); - if (const auto textFormat3 = textFormat.try_query()) - { - THROW_IF_FAILED(textFormat3->SetAutomaticFontAxes(DWRITE_AUTOMATIC_FONT_AXES_OPTICAL_SIZE)); + // NOTE: SetTextAlignment(DWRITE_TEXT_ALIGNMENT_CENTER) breaks certain + // bitmap fonts which expect glyphs to be laid out left-aligned. + + // NOTE: SetAutomaticFontAxes(DWRITE_AUTOMATIC_FONT_AXES_OPTICAL_SIZE) breaks certain + // fonts making them look fairly unslightly. With no option to easily disable this + // feature in Windows Terminal, it's better left disabled by default. - if (!_api.fontAxisValues.empty()) + if (!_api.fontAxisValues.empty()) + { + if (const auto textFormat3 = textFormat.try_query()) { // The wght axis defaults to the font weight. _api.fontAxisValues[0].value = bold || standardAxes[0].value == -1.0f ? static_cast(fontWeight) : standardAxes[0].value;