From 2cd9523f047c04384d8f62e6dae19c7080eea9ff Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 28 Jul 2022 15:59:20 +0200 Subject: [PATCH 1/3] AtlasEngine: Scale glyphs to better fit the cell size --- src/renderer/atlas/AtlasEngine.api.cpp | 1 + src/renderer/atlas/AtlasEngine.cpp | 150 ++------------- src/renderer/atlas/AtlasEngine.h | 10 +- src/renderer/atlas/AtlasEngine.r.cpp | 214 +++++++++++++++++++++- src/renderer/atlas/DWriteTextAnalysis.cpp | 172 +++++++++++++++++ src/renderer/atlas/DWriteTextAnalysis.h | 66 +++++++ src/renderer/atlas/atlas.vcxproj | 2 + 7 files changed, 468 insertions(+), 147 deletions(-) create mode 100644 src/renderer/atlas/DWriteTextAnalysis.cpp create mode 100644 src/renderer/atlas/DWriteTextAnalysis.h diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 18a61adc2c9..a1d1559f0e0 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -688,6 +688,7 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo fontMetrics->fontName = std::move(fontName); fontMetrics->fontSizeInDIP = fontSize / static_cast(_api.dpi) * 96.0f; fontMetrics->baselineInDIP = baseline / static_cast(_api.dpi) * 96.0f; + fontMetrics->advanceScale = cellWidth / advanceWidth; fontMetrics->cellSize = { cellWidth, cellHeight }; fontMetrics->fontWeight = fontWeightU16; fontMetrics->underlinePos = underlinePosU16; diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp index ad92dbb1f2d..4000073890d 100644 --- a/src/renderer/atlas/AtlasEngine.cpp +++ b/src/renderer/atlas/AtlasEngine.cpp @@ -26,134 +26,6 @@ using namespace Microsoft::Console::Render; -struct TextAnalyzer final : IDWriteTextAnalysisSource, IDWriteTextAnalysisSink -{ - constexpr TextAnalyzer(const std::vector& text, std::vector& results) noexcept : - _text{ text }, _results{ results } - { - Ensures(_text.size() <= UINT32_MAX); - } - - // TextAnalyzer will be allocated on the stack and reference counting is pointless because of that. - // The debug version will assert that we don't leak any references though. -#ifdef NDEBUG - ULONG __stdcall AddRef() noexcept override - { - return 1; - } - - ULONG __stdcall Release() noexcept override - { - return 1; - } -#else - ULONG _refCount = 1; - - ~TextAnalyzer() - { - assert(_refCount == 1); - } - - ULONG __stdcall AddRef() noexcept override - { - return ++_refCount; - } - - ULONG __stdcall Release() noexcept override - { - return --_refCount; - } -#endif - - HRESULT __stdcall QueryInterface(const IID& riid, void** ppvObject) noexcept override - { - __assume(ppvObject != nullptr); - - if (IsEqualGUID(riid, __uuidof(IDWriteTextAnalysisSource)) || IsEqualGUID(riid, __uuidof(IDWriteTextAnalysisSink))) - { - *ppvObject = this; - return S_OK; - } - - *ppvObject = nullptr; - return E_NOINTERFACE; - } - - HRESULT __stdcall GetTextAtPosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept override - { - // Writing to address 0 is a crash in practice. Just what we want. - __assume(textString != nullptr); - __assume(textLength != nullptr); - - const auto size = gsl::narrow_cast(_text.size()); - textPosition = std::min(textPosition, size); - *textString = _text.data() + textPosition; - *textLength = size - textPosition; - return S_OK; - } - - HRESULT __stdcall GetTextBeforePosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept override - { - // Writing to address 0 is a crash in practice. Just what we want. - __assume(textString != nullptr); - __assume(textLength != nullptr); - - const auto size = gsl::narrow_cast(_text.size()); - textPosition = std::min(textPosition, size); - *textString = _text.data(); - *textLength = textPosition; - return S_OK; - } - - DWRITE_READING_DIRECTION __stdcall GetParagraphReadingDirection() noexcept override - { - return DWRITE_READING_DIRECTION_LEFT_TO_RIGHT; - } - - HRESULT __stdcall GetLocaleName(UINT32 textPosition, UINT32* textLength, const WCHAR** localeName) noexcept override - { - // Writing to address 0 is a crash in practice. Just what we want. - __assume(textLength != nullptr); - __assume(localeName != nullptr); - - *textLength = gsl::narrow_cast(_text.size()) - textPosition; - *localeName = nullptr; - return S_OK; - } - - HRESULT __stdcall GetNumberSubstitution(UINT32 textPosition, UINT32* textLength, IDWriteNumberSubstitution** numberSubstitution) noexcept override - { - return E_NOTIMPL; - } - - HRESULT __stdcall SetScriptAnalysis(UINT32 textPosition, UINT32 textLength, const DWRITE_SCRIPT_ANALYSIS* scriptAnalysis) noexcept override - try - { - _results.emplace_back(AtlasEngine::TextAnalyzerResult{ textPosition, textLength, scriptAnalysis->script, static_cast(scriptAnalysis->shapes), 0 }); - return S_OK; - } - CATCH_RETURN() - - HRESULT __stdcall SetLineBreakpoints(UINT32 textPosition, UINT32 textLength, const DWRITE_LINE_BREAKPOINT* lineBreakpoints) noexcept override - { - return E_NOTIMPL; - } - - HRESULT __stdcall SetBidiLevel(UINT32 textPosition, UINT32 textLength, UINT8 explicitLevel, UINT8 resolvedLevel) noexcept override - { - return E_NOTIMPL; - } - - HRESULT __stdcall SetNumberSubstitution(UINT32 textPosition, UINT32 textLength, IDWriteNumberSubstitution* numberSubstitution) noexcept override - { - return E_NOTIMPL; - } - -private: - const std::vector& _text; - std::vector& _results; -}; - #pragma warning(suppress : 26455) // Default constructor may not throw. Declare it 'noexcept' (f.6). AtlasEngine::AtlasEngine() { @@ -966,9 +838,12 @@ void AtlasEngine::_recreateFontDependentResources() _r.cellSizeDIP.x = static_cast(_api.fontMetrics.cellSize.x) / scaling; _r.cellSizeDIP.y = static_cast(_api.fontMetrics.cellSize.y) / scaling; + _r.fontMetrics = _api.fontMetrics; _r.cellCount = _api.cellCount; _r.dpi = _api.dpi; _r.fontMetrics = _api.fontMetrics; + _r.dipPerPixel = static_cast(USER_DEFAULT_SCREEN_DPI) / static_cast(_r.dpi); + _r.pixelPerDIP = static_cast(_r.dpi) / static_cast(USER_DEFAULT_SCREEN_DPI); _r.atlasSizeInPixel = { 0, 0 }; _r.tileAllocator = TileAllocator{ _api.fontMetrics.cellSize, _api.sizeInPixel }; @@ -1022,8 +897,8 @@ 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())); - textFormat->SetTextAlignment(DWRITE_TEXT_ALIGNMENT_CENTER); - textFormat->SetWordWrapping(DWRITE_WORD_WRAPPING_NO_WRAP); + THROW_IF_FAILED(textFormat->SetTextAlignment(DWRITE_TEXT_ALIGNMENT_CENTER)); + THROW_IF_FAILED(textFormat->SetWordWrapping(DWRITE_WORD_WRAPPING_NO_WRAP)); // DWRITE_LINE_SPACING_METHOD_UNIFORM: // > Lines are explicitly set to uniform spacing, regardless of contained font sizes. @@ -1031,9 +906,11 @@ 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 (!_api.fontAxisValues.empty()) + if (const auto textFormat3 = textFormat.try_query()) { - if (const auto textFormat3 = textFormat.try_query()) + THROW_IF_FAILED(textFormat3->SetAutomaticFontAxes(DWRITE_AUTOMATIC_FONT_AXES_OPTICAL_SIZE)); + + if (!_api.fontAxisValues.empty()) { // The wght axis defaults to the font weight. _api.fontAxisValues[0].value = bold || standardAxes[0].value == -1.0f ? static_cast(fontWeight) : standardAxes[0].value; @@ -1166,7 +1043,8 @@ void AtlasEngine::_flushBufferLine() const auto textFormat = _getTextFormat(_api.attributes.bold, _api.attributes.italic); const auto& textFormatAxis = _getTextFormatAxis(_api.attributes.bold, _api.attributes.italic); - TextAnalyzer atlasAnalyzer{ _api.bufferLine, _api.analysisResults }; + TextAnalysisSource analysisSource{ _api.bufferLine.data(), gsl::narrow(_api.bufferLine.size()) }; + TextAnalysisSink analysisSink{ _api.analysisResults }; wil::com_ptr fontCollection; THROW_IF_FAILED(textFormat->GetFontCollection(fontCollection.addressof())); @@ -1185,7 +1063,7 @@ void AtlasEngine::_flushBufferLine() { wil::com_ptr fontFace5; THROW_IF_FAILED(_sr.systemFontFallback.query()->MapCharacters( - /* analysisSource */ &atlasAnalyzer, + /* analysisSource */ &analysisSource, /* textPosition */ idx, /* textLength */ gsl::narrow_cast(_api.bufferLine.size()) - idx, /* baseFontCollection */ fontCollection.get(), @@ -1204,7 +1082,7 @@ void AtlasEngine::_flushBufferLine() wil::com_ptr font; THROW_IF_FAILED(_sr.systemFontFallback->MapCharacters( - /* analysisSource */ &atlasAnalyzer, + /* analysisSource */ &analysisSource, /* textPosition */ idx, /* textLength */ gsl::narrow_cast(_api.bufferLine.size()) - idx, /* baseFontCollection */ fontCollection.get(), @@ -1290,7 +1168,7 @@ void AtlasEngine::_flushBufferLine() else { _api.analysisResults.clear(); - THROW_IF_FAILED(_sr.textAnalyzer->AnalyzeScript(&atlasAnalyzer, idx, complexityLength, &atlasAnalyzer)); + THROW_IF_FAILED(_sr.textAnalyzer->AnalyzeScript(&analysisSource, idx, complexityLength, &analysisSink)); //_sr.textAnalyzer->AnalyzeBidi(&atlasAnalyzer, idx, complexityLength, &atlasAnalyzer); for (const auto& a : _api.analysisResults) diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index 6f461aa188d..c2321a5e017 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -8,9 +8,12 @@ #include #include "../../renderer/inc/IRenderEngine.hpp" +#include "DWriteTextAnalysis.h" namespace Microsoft::Console::Render { + struct TextAnalysisSinkResult; + class AtlasEngine final : public IRenderEngine { public: @@ -397,6 +400,7 @@ namespace Microsoft::Console::Render std::wstring fontName; float baselineInDIP = 0.0f; float fontSizeInDIP = 0.0f; + f32 advanceScale = 0; u16x2 cellSize; u16 fontWeight = 0; u16 underlinePos = 0; @@ -953,6 +957,8 @@ namespace Microsoft::Console::Render u16x2 cellCount; // invalidated by ApiInvalidations::Font|Size, caches _api.cellCount u16 dpi = USER_DEFAULT_SCREEN_DPI; // invalidated by ApiInvalidations::Font, caches _api.dpi FontMetrics fontMetrics; // invalidated by ApiInvalidations::Font, cached _api.fontMetrics + f32 dipPerPixel = 1.0f; // invalidated by ApiInvalidations::Font, caches USER_DEFAULT_SCREEN_DPI / _api.dpi + f32 pixelPerDIP = 1.0f; // invalidated by ApiInvalidations::Font, caches _api.dpi / USER_DEFAULT_SCREEN_DPI u16x2 atlasSizeInPixel; // invalidated by ApiInvalidations::Font TileHashMap glyphs; TileAllocator tileAllocator; @@ -983,7 +989,7 @@ namespace Microsoft::Console::Render std::vector bufferLine; std::vector bufferLineColumn; Buffer bufferLineMetadata; - std::vector analysisResults; + std::vector analysisResults; Buffer clusterMap; Buffer textProps; Buffer glyphIndices; @@ -992,7 +998,7 @@ namespace Microsoft::Console::Render Buffer glyphOffsets; std::vector fontFeatures; // changes are flagged as ApiInvalidations::Font|Size std::vector fontAxisValues; // changes are flagged as ApiInvalidations::Font|Size - FontMetrics fontMetrics; // changes are flagged as ApiInvalidations::Font|Size + FontMetrics fontMetrics; // changes are flagged as ApiInvalidations::Font u16x2 cellCount; // caches `sizeInPixel / cellSize` u16x2 sizeInPixel; // changes are flagged as ApiInvalidations::Size diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index ea53cdfeb09..ce729aaf667 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -126,7 +126,6 @@ void AtlasEngine::_updateConstantBuffer() const noexcept data.cellCountX = _r.cellCount.x; data.cellSize.x = _r.fontMetrics.cellSize.x; data.cellSize.y = _r.fontMetrics.cellSize.y; - data.underlinePos = _r.fontMetrics.underlinePos; data.underlineWidth = _r.fontMetrics.underlineWidth; data.strikethroughPos = _r.fontMetrics.strikethroughPos; @@ -245,19 +244,20 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const const auto value = item.value->data(); const auto coords = &value->coords[0]; const auto charsLength = key->charCount; - const auto cells = static_cast(key->attributes.cellCount); + const auto cellCount = static_cast(key->attributes.cellCount); const auto textFormat = _getTextFormat(key->attributes.bold, key->attributes.italic); const auto coloredGlyph = WI_IsFlagSet(value->flags, CellFlags::ColoredGlyph); + const f32x2 layoutBox{ cellCount * _r.cellSizeDIP.x, _r.cellSizeDIP.y }; // See D2DFactory::DrawText wil::com_ptr textLayout; - THROW_IF_FAILED(_sr.dwriteFactory->CreateTextLayout(&key->chars[0], charsLength, textFormat, cells * _r.cellSizeDIP.x, _r.cellSizeDIP.y, textLayout.addressof())); + THROW_IF_FAILED(_sr.dwriteFactory->CreateTextLayout(&key->chars[0], charsLength, textFormat, layoutBox.x, layoutBox.y, textLayout.addressof())); if (_r.typography) { textLayout->SetTypography(_r.typography.get(), { 0, charsLength }); } - auto options = D2D1_DRAW_TEXT_OPTIONS_CLIP; + auto options = D2D1_DRAW_TEXT_OPTIONS_NONE; // D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT enables a bunch of internal machinery // which doesn't have to run if we know we can't use it anyways in the shader. WI_SetFlagIf(options, D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT, coloredGlyph); @@ -271,23 +271,219 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const _r.d2dRenderTarget->SetTextAntialiasMode(coloredGlyph ? D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE : D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE); } - for (u32 i = 0; i < cells; ++i) + bool scalingRequired = false; + f32x2 offset{ 0, 0 }; + f32x2 scale{ 1, 1 }; + + // Block Element and Box Drawing characters need to be handled separately from + // others, because unlike them they're supposed to fill the entire layout box. + // Or in other words: If they overflow the layout box, that's actually great! + // To be on the safe side measure this code however will always adjust + // their scale to result in an exact size match of the layoutBox. + if (charsLength == 1 && key->chars[0] >= 0x2500 && key->chars[0] <= 0x259F) + { + wil::com_ptr fontCollection; + THROW_IF_FAILED(textFormat->GetFontCollection(fontCollection.addressof())); + const auto baseWeight = textFormat->GetFontWeight(); + const auto baseStyle = textFormat->GetFontStyle(); + + TextAnalysisSource analysisSource{ &key->chars[0], 1 }; + UINT32 mappedLength = 0; + wil::com_ptr mappedFont; + FLOAT mappedScale = 0; + THROW_IF_FAILED(_sr.systemFontFallback->MapCharacters( + /* analysisSource */ &analysisSource, + /* textPosition */ 0, + /* textLength */ 1, + /* baseFontCollection */ fontCollection.get(), + /* baseFamilyName */ _r.fontMetrics.fontName.data(), + /* baseWeight */ baseWeight, + /* baseStyle */ baseStyle, + /* baseStretch */ DWRITE_FONT_STRETCH_NORMAL, + /* mappedLength */ &mappedLength, + /* mappedFont */ mappedFont.addressof(), + /* scale */ &mappedScale)); + + if (mappedFont) + { + wil::com_ptr fontFace; + THROW_IF_FAILED(mappedFont->CreateFontFace(fontFace.addressof())); + + DWRITE_FONT_METRICS metrics; + fontFace->GetMetrics(&metrics); + + const u32 codePoint = key->chars[0]; + u16 glyphIndex; + THROW_IF_FAILED(fontFace->GetGlyphIndicesW(&codePoint, 1, &glyphIndex)); + + DWRITE_GLYPH_METRICS glyphMetrics; + THROW_IF_FAILED(fontFace->GetDesignGlyphMetrics(&glyphIndex, 1, &glyphMetrics)); + + const f32x2 boxSize{ + static_cast(glyphMetrics.advanceWidth) / static_cast(metrics.designUnitsPerEm) * _r.fontMetrics.fontSizeInDIP, + static_cast(glyphMetrics.advanceHeight) / static_cast(metrics.designUnitsPerEm) * _r.fontMetrics.fontSizeInDIP, + }; + + // 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 + _r.dipPerPixel) / boxSize.x; + scale.y = (layoutBox.y + _r.dipPerPixel) / boxSize.y; + } + } + else + { + 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 targetSize{ + layoutBox.x - _r.dipPerPixel, + layoutBox.y - _r.dipPerPixel, + }; + f32x2 actualSize{ + layoutBox.x + overhang.left + overhang.right, + layoutBox.y + overhang.top + overhang.bottom, + }; + + // 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. + // If we didn't force upscale that ligatures it would seemingly shrink shorter and shorter, as its + // glyph advance is often slightly shorter by a fractional pixel or two compared to our terminal's cells. + // It's a trade off that keeps most glyphs "crisp" while retaining support for things like "===". + // At least I can't think of any better heuristic for this at the moment... + if (cellCount > 2) + { + const auto advanceScale = _r.fontMetrics.advanceScale; + scalingRequired = true; + scale = { advanceScale, advanceScale }; + actualSize.x *= advanceScale; + actualSize.y *= advanceScale; + } + + // We need to offset glyphs that are simply outside of our layout box (layoutBox.x/.y) + // and additionally downsize glyphs that are entirely too large to fit in. + // The DWRITE_OVERHANG_METRICS will tell us how many DIPs the layout box is too large/small. + // It contains a positive number if the glyph is outside and a negative one if it's inside + // the layout box. For example, given a layoutBox.x/.y (and cell size) of 20/30: + // * "M" is the "largest" ASCII character and might be: + // left: -0.6f + // right: -0.6f + // top: -7.6f + // bottom: -7.4f + // "M" doesn't fill the layout box at all! + // This is because we've rounded up the Terminal's cell size to whole pixels in + // _resolveFontMetrics. top/bottom margins are fairly large because we added the + // chosen font's ascender, descender and line gap metrics to get our line height. + // --> offsetX = 0 + // --> offsetY = 0 + // --> scale = 1 + // * The bar diacritic (U+0336 combining long stroke overlay) + // left: -9.0f + // top: -16.3f + // right: 5.6f + // bottom: -11.7f + // right is positive! Our glyph is 5.6 DIPs outside of the layout box and would + // appear cut off during rendering. left is negative at -9, which indicates that + // we can simply shift the glyph by 5.6 DIPs to the left to fit it into our bounds. + // --> offsetX = -5.6f + // --> offsetY = 0 + // --> scale = 1 + // * Any wide emoji in a narrow cell (U+26A0 warning sign) + // left: 6.7f + // top: -4.1f + // right: 6.7f + // bottom: -3.0f + // Our emoji is outside the bounds on both the left and right side and we need to shrink it. + // --> offsetX = 0 + // --> offsetY = 0 + // --> scale = layoutBox.y / (layoutBox.y + left + right) + // = 0.69f + offset.x = clampedOverhang.left - clampedOverhang.right; + offset.y = clampedOverhang.top - clampedOverhang.bottom; + + if (actualSize.x > targetSize.x) + { + scalingRequired = true; + offset.x = (overhang.left - overhang.right) * 0.5f; + scale.x = targetSize.x / actualSize.x; + scale.y = scale.x; + } + if (actualSize.y > targetSize.y) + { + scalingRequired = true; + offset.y = (overhang.top - overhang.bottom) * 0.5f; + scale.x = std::min(scale.x, targetSize.y / actualSize.y); + scale.y = scale.x; + } + + offset.x *= scale.x; + offset.y *= scale.y; + + // Due to D2D1_DRAW_TEXT_OPTIONS_NO_SNAP our baseline won't get + // pixel aligned anymore and so we have to do it ourselves here. + offset.x = roundf(offset.x * _r.pixelPerDIP) * _r.dipPerPixel; + offset.y = roundf(offset.y * _r.pixelPerDIP) * _r.dipPerPixel; + } + + // !!! IMPORTANT !!! + // DirectWrite/2D snaps the baseline to whole pixels, which is something we technically + // want (it makes text look crisp), but fails in weird ways if `scalingRequired` is true. + // As our scaling matrix's dx/dy (center point) is based on the `origin` coordinates + // each cell we draw gets a unique, fractional baseline which gets rounded differently. + // I'm not 100% sure why that happens, since `origin` is always in full pixels... + // But this causes wide glyphs to draw as tiles that are potentially misaligned vertically by a pixel. + // The resulting text rendering looks especially bad for ligatures like "====" in Cascadia Code, + // where every single "=" might be blatantly misaligned vertically (same for any box drawings). + WI_SetFlagIf(options, D2D1_DRAW_TEXT_OPTIONS_NO_SNAP, scalingRequired); + + const f32x2 inverseScale{ 1.0f - scale.x, 1.0f - scale.y }; + const f32x2 halfSize{ layoutBox.x * 0.5f, layoutBox.y * 0.5f }; + + for (u32 i = 0; i < cellCount; ++i) { const auto coord = coords[i]; D2D1_RECT_F rect; - rect.left = static_cast(coord.x) * static_cast(USER_DEFAULT_SCREEN_DPI) / static_cast(_r.dpi); - rect.top = static_cast(coord.y) * static_cast(USER_DEFAULT_SCREEN_DPI) / static_cast(_r.dpi); + rect.left = static_cast(coord.x) * _r.dipPerPixel; + rect.top = static_cast(coord.y) * _r.dipPerPixel; rect.right = rect.left + _r.cellSizeDIP.x; rect.bottom = rect.top + _r.cellSizeDIP.y; D2D1_POINT_2F origin; - origin.x = rect.left - i * _r.cellSizeDIP.x; - origin.y = rect.top; + origin.x = rect.left - i * _r.cellSizeDIP.x + offset.x; + origin.y = rect.top + offset.y; _r.d2dRenderTarget->PushAxisAlignedClip(&rect, D2D1_ANTIALIAS_MODE_ALIASED); _r.d2dRenderTarget->Clear(); + if (scalingRequired) + { + // TODO: dx/dy should technically be pixel snapped + // Due to D2D1_DRAW_TEXT_OPTIONS_NO_SNAP our baseline isn't getting pixel aligned anymore + // and we should align do it here. We'd have to offset dx/dy in the matrix below in + // such a way that our baseline is on a whole pixel _after_ the scaling operation. + const D2D1_MATRIX_3X2_F transform{ + scale.x, + 0, + 0, + scale.y, + (origin.x + halfSize.x) * inverseScale.x, + (origin.y + halfSize.y) * inverseScale.y, + }; + _r.d2dRenderTarget->SetTransform(&transform); + } _r.d2dRenderTarget->DrawTextLayout(origin, textLayout.get(), _r.brush.get(), options); + if (scalingRequired) + { + static constexpr D2D1_MATRIX_3X2_F identity{ 1, 0, 0, 1, 0, 0 }; + _r.d2dRenderTarget->SetTransform(&identity); + } _r.d2dRenderTarget->PopAxisAlignedClip(); } } diff --git a/src/renderer/atlas/DWriteTextAnalysis.cpp b/src/renderer/atlas/DWriteTextAnalysis.cpp new file mode 100644 index 00000000000..8b4ba477349 --- /dev/null +++ b/src/renderer/atlas/DWriteTextAnalysis.cpp @@ -0,0 +1,172 @@ +#include "pch.h" +#include "DWriteTextAnalysis.h" + +#pragma warning(disable : 4100) // '...': unreferenced formal parameter +#pragma warning(disable : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1). + +using namespace Microsoft::Console::Render; + +TextAnalysisSource::TextAnalysisSource(const wchar_t* _text, const UINT32 _textLength) noexcept : + _text{ _text }, + _textLength{ _textLength } +{ +} + +// TextAnalysisSource will be allocated on the stack and reference counting is pointless because of that. +// The debug version will assert that we don't leak any references though. +#ifdef NDEBUG +ULONG __stdcall TextAnalysisSource::AddRef() noexcept +{ + return 1; +} + +ULONG __stdcall TextAnalysisSource::Release() noexcept +{ + return 1; +} +#else +TextAnalysisSource::~TextAnalysisSource() +{ + assert(_refCount == 1); +} + +ULONG __stdcall TextAnalysisSource::AddRef() noexcept +{ + return ++_refCount; +} + +ULONG __stdcall TextAnalysisSource::Release() noexcept +{ + return --_refCount; +} +#endif + +HRESULT TextAnalysisSource::QueryInterface(const IID& riid, void** ppvObject) noexcept +{ + __assume(ppvObject != nullptr); + + if (IsEqualGUID(riid, __uuidof(IDWriteTextAnalysisSource))) + { + *ppvObject = this; + return S_OK; + } + + *ppvObject = nullptr; + return E_NOINTERFACE; +} + +HRESULT TextAnalysisSource::GetTextAtPosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept +{ + // Writing to address 0 is a crash in practice. Just what we want. + __assume(textString != nullptr); + __assume(textLength != nullptr); + + textPosition = std::min(textPosition, _textLength); + *textString = _text + textPosition; + *textLength = _textLength - textPosition; + return S_OK; +} + +HRESULT TextAnalysisSource::GetTextBeforePosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept +{ + // Writing to address 0 is a crash in practice. Just what we want. + __assume(textString != nullptr); + __assume(textLength != nullptr); + + textPosition = std::min(textPosition, _textLength); + *textString = _text; + *textLength = textPosition; + return S_OK; +} + +DWRITE_READING_DIRECTION TextAnalysisSource::GetParagraphReadingDirection() noexcept +{ + return DWRITE_READING_DIRECTION_LEFT_TO_RIGHT; +} + +HRESULT TextAnalysisSource::GetLocaleName(UINT32 textPosition, UINT32* textLength, const WCHAR** localeName) noexcept +{ + // Writing to address 0 is a crash in practice. Just what we want. + __assume(textLength != nullptr); + __assume(localeName != nullptr); + + *textLength = _textLength - textPosition; + *localeName = nullptr; + return S_OK; +} + +HRESULT TextAnalysisSource::GetNumberSubstitution(UINT32 textPosition, UINT32* textLength, IDWriteNumberSubstitution** numberSubstitution) noexcept +{ + return E_NOTIMPL; +} + +TextAnalysisSink::TextAnalysisSink(std::vector& results) noexcept : + _results{ results } +{ +} + +// TextAnalysisSource will be allocated on the stack and reference counting is pointless because of that. +// The debug version will assert that we don't leak any references though. +#ifdef NDEBUG +ULONG __stdcall TextAnalysisSink::AddRef() noexcept +{ + return 1; +} + +ULONG __stdcall TextAnalysisSink::Release() noexcept +{ + return 1; +} +#else +TextAnalysisSink::~TextAnalysisSink() +{ + assert(_refCount == 1); +} + +ULONG __stdcall TextAnalysisSink::AddRef() noexcept +{ + return ++_refCount; +} + +ULONG __stdcall TextAnalysisSink::Release() noexcept +{ + return --_refCount; +} +#endif + +HRESULT TextAnalysisSink::QueryInterface(const IID& riid, void** ppvObject) noexcept +{ + __assume(ppvObject != nullptr); + + if (IsEqualGUID(riid, __uuidof(IDWriteTextAnalysisSink))) + { + *ppvObject = this; + return S_OK; + } + + *ppvObject = nullptr; + return E_NOINTERFACE; +} + +HRESULT __stdcall TextAnalysisSink::SetScriptAnalysis(UINT32 textPosition, UINT32 textLength, const DWRITE_SCRIPT_ANALYSIS* scriptAnalysis) noexcept +try +{ + _results.emplace_back(TextAnalysisSinkResult{ textPosition, textLength, scriptAnalysis->script, static_cast(scriptAnalysis->shapes), 0 }); + return S_OK; +} +CATCH_RETURN() + +HRESULT TextAnalysisSink::SetLineBreakpoints(UINT32 textPosition, UINT32 textLength, const DWRITE_LINE_BREAKPOINT* lineBreakpoints) noexcept +{ + return E_NOTIMPL; +} + +HRESULT TextAnalysisSink::SetBidiLevel(UINT32 textPosition, UINT32 textLength, UINT8 explicitLevel, UINT8 resolvedLevel) noexcept +{ + return E_NOTIMPL; +} + +HRESULT TextAnalysisSink::SetNumberSubstitution(UINT32 textPosition, UINT32 textLength, IDWriteNumberSubstitution* numberSubstitution) noexcept +{ + return E_NOTIMPL; +} diff --git a/src/renderer/atlas/DWriteTextAnalysis.h b/src/renderer/atlas/DWriteTextAnalysis.h new file mode 100644 index 00000000000..15490ca868f --- /dev/null +++ b/src/renderer/atlas/DWriteTextAnalysis.h @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +namespace Microsoft::Console::Render +{ + struct TextAnalysisSinkResult + { + uint32_t textPosition = 0; + uint32_t textLength = 0; + + // These 2 fields represent DWRITE_SCRIPT_ANALYSIS. + // Not using DWRITE_SCRIPT_ANALYSIS drops the struct size from 20 down to 12 bytes. + uint16_t script = 0; + uint8_t shapes = 0; + + uint8_t bidiLevel = 0; + }; + + struct TextAnalysisSource final : IDWriteTextAnalysisSource + { + TextAnalysisSource(const wchar_t* _text, const UINT32 _textLength) noexcept; +#ifndef NDEBUG + ~TextAnalysisSource(); +#endif + + ULONG __stdcall AddRef() noexcept override; + ULONG __stdcall Release() noexcept override; + HRESULT __stdcall QueryInterface(const IID& riid, void** ppvObject) noexcept override; + HRESULT __stdcall GetTextAtPosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept override; + HRESULT __stdcall GetTextBeforePosition(UINT32 textPosition, const WCHAR** textString, UINT32* textLength) noexcept override; + DWRITE_READING_DIRECTION __stdcall GetParagraphReadingDirection() noexcept override; + HRESULT __stdcall GetLocaleName(UINT32 textPosition, UINT32* textLength, const WCHAR** localeName) noexcept override; + HRESULT __stdcall GetNumberSubstitution(UINT32 textPosition, UINT32* textLength, IDWriteNumberSubstitution** numberSubstitution) noexcept override; + + private: + const wchar_t* _text; + const UINT32 _textLength; +#ifndef NDEBUG + ULONG _refCount = 1; +#endif + }; + + struct TextAnalysisSink final : IDWriteTextAnalysisSink + { + TextAnalysisSink(std::vector& results) noexcept; +#ifndef NDEBUG + ~TextAnalysisSink(); +#endif + + ULONG __stdcall AddRef() noexcept override; + ULONG __stdcall Release() noexcept override; + HRESULT __stdcall QueryInterface(const IID& riid, void** ppvObject) noexcept override; + HRESULT __stdcall SetScriptAnalysis(UINT32 textPosition, UINT32 textLength, const DWRITE_SCRIPT_ANALYSIS* scriptAnalysis) noexcept override; + HRESULT __stdcall SetLineBreakpoints(UINT32 textPosition, UINT32 textLength, const DWRITE_LINE_BREAKPOINT* lineBreakpoints) noexcept override; + HRESULT __stdcall SetBidiLevel(UINT32 textPosition, UINT32 textLength, UINT8 explicitLevel, UINT8 resolvedLevel) noexcept override; + HRESULT __stdcall SetNumberSubstitution(UINT32 textPosition, UINT32 textLength, IDWriteNumberSubstitution* numberSubstitution) noexcept override; + + private: + std::vector& _results; +#ifndef NDEBUG + ULONG _refCount = 1; +#endif + }; +} diff --git a/src/renderer/atlas/atlas.vcxproj b/src/renderer/atlas/atlas.vcxproj index 3a4c51bf987..355a9175240 100644 --- a/src/renderer/atlas/atlas.vcxproj +++ b/src/renderer/atlas/atlas.vcxproj @@ -14,6 +14,7 @@ + Create @@ -21,6 +22,7 @@ + From c20e0a007a99cef02ef7892c65fc013222fa6ad1 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Fri, 29 Jul 2022 23:55:44 +0200 Subject: [PATCH 2/3] Address feedback, Fix issues --- src/renderer/atlas/AtlasEngine.cpp | 1 - src/renderer/atlas/AtlasEngine.h | 2 +- src/renderer/atlas/AtlasEngine.r.cpp | 52 ++++++++++++++++------------ 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp index 4000073890d..ffe24158c64 100644 --- a/src/renderer/atlas/AtlasEngine.cpp +++ b/src/renderer/atlas/AtlasEngine.cpp @@ -838,7 +838,6 @@ void AtlasEngine::_recreateFontDependentResources() _r.cellSizeDIP.x = static_cast(_api.fontMetrics.cellSize.x) / scaling; _r.cellSizeDIP.y = static_cast(_api.fontMetrics.cellSize.y) / scaling; - _r.fontMetrics = _api.fontMetrics; _r.cellCount = _api.cellCount; _r.dpi = _api.dpi; _r.fontMetrics = _api.fontMetrics; diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index c2321a5e017..08a388ed1d4 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -998,7 +998,7 @@ namespace Microsoft::Console::Render Buffer glyphOffsets; std::vector fontFeatures; // changes are flagged as ApiInvalidations::Font|Size std::vector fontAxisValues; // changes are flagged as ApiInvalidations::Font|Size - FontMetrics fontMetrics; // changes are flagged as ApiInvalidations::Font + FontMetrics fontMetrics; // changes are flagged as ApiInvalidations::Font|Size u16x2 cellCount; // caches `sizeInPixel / cellSize` u16x2 sizeInPixel; // changes are flagged as ApiInvalidations::Size diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index ce729aaf667..d6ea168dd72 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -271,6 +271,7 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const _r.d2dRenderTarget->SetTextAntialiasMode(coloredGlyph ? D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE : D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE); } + const f32x2 halfSize{ layoutBox.x * 0.5f, layoutBox.y * 0.5f }; bool scalingRequired = false; f32x2 offset{ 0, 0 }; f32x2 scale{ 1, 1 }; @@ -343,10 +344,6 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const std::max(0.0f, overhang.right), std::max(0.0f, overhang.bottom), }; - f32x2 targetSize{ - layoutBox.x - _r.dipPerPixel, - layoutBox.y - _r.dipPerPixel, - }; f32x2 actualSize{ layoutBox.x + overhang.left + overhang.right, layoutBox.y + overhang.top + overhang.bottom, @@ -408,28 +405,34 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const offset.x = clampedOverhang.left - clampedOverhang.right; offset.y = clampedOverhang.top - clampedOverhang.bottom; - if (actualSize.x > targetSize.x) + if (actualSize.x > layoutBox.x) { scalingRequired = true; offset.x = (overhang.left - overhang.right) * 0.5f; - scale.x = targetSize.x / actualSize.x; + scale.x = (layoutBox.x - _r.dipPerPixel) / actualSize.x; scale.y = scale.x; } - if (actualSize.y > targetSize.y) + if (actualSize.y > layoutBox.y) { scalingRequired = true; offset.y = (overhang.top - overhang.bottom) * 0.5f; - scale.x = std::min(scale.x, targetSize.y / actualSize.y); + scale.x = std::min(scale.x, (layoutBox.y - _r.dipPerPixel) / actualSize.y); scale.y = scale.x; } offset.x *= scale.x; offset.y *= scale.y; - // Due to D2D1_DRAW_TEXT_OPTIONS_NO_SNAP our baseline won't get - // pixel aligned anymore and so we have to do it ourselves here. - offset.x = roundf(offset.x * _r.pixelPerDIP) * _r.dipPerPixel; - offset.y = roundf(offset.y * _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? + // 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; } // !!! IMPORTANT !!! @@ -444,7 +447,6 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const WI_SetFlagIf(options, D2D1_DRAW_TEXT_OPTIONS_NO_SNAP, scalingRequired); const f32x2 inverseScale{ 1.0f - scale.x, 1.0f - scale.y }; - const f32x2 halfSize{ layoutBox.x * 0.5f, layoutBox.y * 0.5f }; for (u32 i = 0; i < cellCount; ++i) { @@ -457,17 +459,15 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const rect.bottom = rect.top + _r.cellSizeDIP.y; D2D1_POINT_2F origin; - origin.x = rect.left - i * _r.cellSizeDIP.x + offset.x; - origin.y = rect.top + offset.y; + origin.x = rect.left - i * _r.cellSizeDIP.x; + origin.y = rect.top; - _r.d2dRenderTarget->PushAxisAlignedClip(&rect, D2D1_ANTIALIAS_MODE_ALIASED); - _r.d2dRenderTarget->Clear(); + { + _r.d2dRenderTarget->PushAxisAlignedClip(&rect, D2D1_ANTIALIAS_MODE_ALIASED); + _r.d2dRenderTarget->Clear(); + } if (scalingRequired) { - // TODO: dx/dy should technically be pixel snapped - // Due to D2D1_DRAW_TEXT_OPTIONS_NO_SNAP our baseline isn't getting pixel aligned anymore - // and we should align do it here. We'd have to offset dx/dy in the matrix below in - // such a way that our baseline is on a whole pixel _after_ the scaling operation. const D2D1_MATRIX_3X2_F transform{ scale.x, 0, @@ -478,13 +478,19 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const }; _r.d2dRenderTarget->SetTransform(&transform); } - _r.d2dRenderTarget->DrawTextLayout(origin, textLayout.get(), _r.brush.get(), options); + { + origin.x += offset.x; + origin.y += offset.y; + _r.d2dRenderTarget->DrawTextLayout(origin, textLayout.get(), _r.brush.get(), options); + } if (scalingRequired) { static constexpr D2D1_MATRIX_3X2_F identity{ 1, 0, 0, 1, 0, 0 }; _r.d2dRenderTarget->SetTransform(&identity); } - _r.d2dRenderTarget->PopAxisAlignedClip(); + { + _r.d2dRenderTarget->PopAxisAlignedClip(); + } } } From dfa92559611f5c982f78cba73d123638a5e54227 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Sat, 30 Jul 2022 00:56:35 +0200 Subject: [PATCH 3/3] Add comment, Fix centering --- src/renderer/atlas/AtlasEngine.r.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index d6ea168dd72..62fad5538da 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -420,9 +420,6 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const scale.y = scale.x; } - offset.x *= scale.x; - offset.y *= scale.y; - // 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`. @@ -479,6 +476,8 @@ void AtlasEngine::_drawGlyph(const AtlasQueueItem& item) const _r.d2dRenderTarget->SetTransform(&transform); } { + // Now that we're done using origin to calculate the center point for our transformation + // we can use it for its intended purpose to slightly shift the glyph around. origin.x += offset.x; origin.y += offset.y; _r.d2dRenderTarget->DrawTextLayout(origin, textLayout.get(), _r.brush.get(), options);