From dec1c1131cd5a490fb27c062ad4ebc69acdae2d5 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Jul 2022 17:29:26 +0200 Subject: [PATCH 1/5] AtlasEngine: Implement remaining grid lines --- src/renderer/atlas/AtlasEngine.api.cpp | 85 +++++++++++++++++--------- src/renderer/atlas/AtlasEngine.cpp | 17 ++---- src/renderer/atlas/AtlasEngine.h | 20 +++--- src/renderer/atlas/AtlasEngine.r.cpp | 20 +++--- src/renderer/atlas/shader_ps.hlsl | 52 +++++++++------- 5 files changed, 117 insertions(+), 77 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index be5630d9fd4..2f28e3ca811 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -272,7 +272,7 @@ CATCH_RETURN() DWRITE_TEXT_METRICS metrics; RETURN_IF_FAILED(textLayout->GetMetrics(&metrics)); - *pResult = static_cast(std::ceil(metrics.width)) > _api.fontMetrics.cellSize.x; + *pResult = static_cast(std::ceilf(metrics.width)) > _api.fontMetrics.cellSize.x; return S_OK; } @@ -605,18 +605,43 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo // Point sizes are commonly treated at a 72 DPI scale // (including by OpenType), whereas DirectWrite uses 96 DPI. // Since we want the height in px we multiply by the display's DPI. - const auto fontSizeInPx = std::ceil(requestedSize.Y / 72.0 * _api.dpi); - - const auto designUnitsPerPx = fontSizeInPx / static_cast(metrics.designUnitsPerEm); - const auto ascentInPx = static_cast(metrics.ascent) * designUnitsPerPx; - const auto descentInPx = static_cast(metrics.descent) * designUnitsPerPx; - const auto lineGapInPx = static_cast(metrics.lineGap) * designUnitsPerPx; - const auto advanceWidthInPx = static_cast(glyphMetrics.advanceWidth) * designUnitsPerPx; - - const auto halfGapInPx = lineGapInPx / 2.0; - const auto baseline = std::ceil(ascentInPx + halfGapInPx); - const auto cellWidth = gsl::narrow(std::ceil(advanceWidthInPx)); - const auto cellHeight = gsl::narrow(std::ceil(baseline + descentInPx + halfGapInPx)); + const auto fontSize = std::ceilf(requestedSize.Y / 72.0f * _api.dpi); + + const auto designUnitsPerPx = fontSize / 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 advanceWidth = static_cast(glyphMetrics.advanceWidth) * 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; + const auto strikethroughThickness = static_cast(metrics.strikethroughThickness) * designUnitsPerPx; + + const auto halfGap = lineGap / 2.0f; + const auto baseline = std::ceilf(ascent + halfGap); + const auto lineHeight = std::ceilf(baseline + descent + halfGap); + const auto underlinePos = std::roundf(baseline + underlinePosition); + const auto underlineWidth = std::max(1.0f, std::roundf(underlineThickness)); + const auto strikethroughPos = std::roundf(baseline + strikethroughPosition); + const auto strikethroughWidth = std::max(1.0f, std::roundf(strikethroughThickness)); + const auto thinLineWidth = std::max(1.0f, std::roundf(underlineThickness / 2.0f)); + + // For double underlines we loosely follow what Word: + // 1. The lines are half the width of an underline (= thinLineWidth) + // 2. Ideally the bottom line is aligned with the bottom of the underline + // 3. The top underline is vertically in the middle between baseline and bottom underline + // 4. If the top line gets too close to the baseline the underlines are shifted downwards + // 5. The minimum gap between the two lines appears to be similar to Tex (1.2pt) + auto doubleUnderlinePosBottom = underlinePos + underlineWidth - thinLineWidth; // 2. + auto doubleUnderlinePosTop = std::roundf((baseline + doubleUnderlinePosBottom - thinLineWidth) / 2.0f); // 3. + doubleUnderlinePosTop = std::max(doubleUnderlinePosTop, baseline + thinLineWidth); // 4. + const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi)); // 5. + doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + thinLineWidth + doubleUnderlineGap); // 5. + // Our cells can't overlap each other so we additionally clamp the bottom line to be inside the cell boundaries. + doubleUnderlinePosBottom = std::min(doubleUnderlinePosBottom, lineHeight - thinLineWidth); + + const auto cellWidth = gsl::narrow(std::ceilf(advanceWidth)); + const auto cellHeight = gsl::narrow(lineHeight); { til::size coordSize; @@ -637,28 +662,30 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo if (fontMetrics) { - const auto underlineOffsetInPx = static_cast(-metrics.underlinePosition) * designUnitsPerPx; - const auto underlineThicknessInPx = static_cast(metrics.underlineThickness) * designUnitsPerPx; - const auto strikethroughOffsetInPx = static_cast(-metrics.strikethroughPosition) * designUnitsPerPx; - const auto strikethroughThicknessInPx = static_cast(metrics.strikethroughThickness) * designUnitsPerPx; - const auto lineThickness = gsl::narrow(std::round(std::min(underlineThicknessInPx, strikethroughThicknessInPx))); - const auto underlinePos = gsl::narrow(std::ceil(baseline + underlineOffsetInPx - lineThickness / 2.0)); - const auto strikethroughPos = gsl::narrow(std::round(baseline + strikethroughOffsetInPx - lineThickness / 2.0)); - - auto fontName = wil::make_process_heap_string(requestedFaceName); - const auto fontWeight = gsl::narrow(requestedWeight); + std::wstring fontName{ requestedFaceName }; + const auto fontWeightU16 = gsl::narrow(requestedWeight); + const auto underlinePosU16 = gsl::narrow(underlinePos); + const auto underlineWidthU16 = gsl::narrow(underlineWidth); + const auto strikethroughPosU16 = gsl::narrow(strikethroughPos); + const auto strikethroughWidthU16 = gsl::narrow(strikethroughWidth); + const auto doubleUnderlinePosTopU16 = gsl::narrow(doubleUnderlinePosTop); + const auto doubleUnderlinePosBottomU16 = gsl::narrow(doubleUnderlinePosBottom); + const auto thinLineWidthU16 = gsl::narrow(thinLineWidth); // NOTE: From this point onward no early returns or throwing code should exist, // as we might cause _api to be in an inconsistent state otherwise. fontMetrics->fontCollection = std::move(fontCollection); fontMetrics->fontName = std::move(fontName); - fontMetrics->fontSizeInDIP = static_cast(fontSizeInPx / static_cast(_api.dpi) * 96.0); - fontMetrics->baselineInDIP = static_cast(baseline / static_cast(_api.dpi) * 96.0); + fontMetrics->fontSizeInDIP = fontSize / static_cast(_api.dpi) * 96.0f; + fontMetrics->baselineInDIP = baseline / static_cast(_api.dpi) * 96.0f; fontMetrics->cellSize = { cellWidth, cellHeight }; - fontMetrics->fontWeight = fontWeight; - fontMetrics->underlinePos = underlinePos; - fontMetrics->strikethroughPos = strikethroughPos; - fontMetrics->lineThickness = lineThickness; + fontMetrics->fontWeight = fontWeightU16; + fontMetrics->underlinePos = underlinePosU16; + fontMetrics->underlineWidth = underlineWidthU16; + fontMetrics->strikethroughPos = strikethroughPosU16; + fontMetrics->strikethroughWidth = strikethroughWidthU16; + fontMetrics->doubleUnderlinePos = { doubleUnderlinePosTopU16, doubleUnderlinePosBottomU16 }; + fontMetrics->thinLineWidth = thinLineWidthU16; } } diff --git a/src/renderer/atlas/AtlasEngine.cpp b/src/renderer/atlas/AtlasEngine.cpp index e259d9a7082..474eb3105ac 100644 --- a/src/renderer/atlas/AtlasEngine.cpp +++ b/src/renderer/atlas/AtlasEngine.cpp @@ -976,10 +976,11 @@ void AtlasEngine::_recreateFontDependentResources() _r.cellSizeDIP.x = static_cast(_api.fontMetrics.cellSize.x) / scaling; _r.cellSizeDIP.y = static_cast(_api.fontMetrics.cellSize.y) / scaling; - _r.cellSize = _api.fontMetrics.cellSize; _r.cellCount = _api.cellCount; + _r.dpi = _api.dpi; + _r.fontMetrics = _api.fontMetrics; _r.atlasSizeInPixel = { 0, 0 }; - _r.tileAllocator = TileAllocator{ _r.cellSize, _api.sizeInPixel }; + _r.tileAllocator = TileAllocator{ _api.fontMetrics.cellSize, _api.sizeInPixel }; _r.glyphs = {}; _r.glyphQueue = {}; @@ -1000,12 +1001,6 @@ void AtlasEngine::_recreateFontDependentResources() } // D2D - { - _r.underlinePos = _api.fontMetrics.underlinePos; - _r.strikethroughPos = _api.fontMetrics.strikethroughPos; - _r.lineThickness = _api.fontMetrics.lineThickness; - _r.dpi = _api.dpi; - } { // See AtlasEngine::UpdateFont. // It hardcodes indices 0/1/2 in fontAxisValues to the weight/italic/slant axes. @@ -1036,7 +1031,7 @@ void AtlasEngine::_recreateFontDependentResources() const auto fontStyle = italic ? DWRITE_FONT_STYLE_ITALIC : DWRITE_FONT_STYLE_NORMAL; auto& textFormat = _r.textFormats[italic][bold]; - THROW_IF_FAILED(_sr.dwriteFactory->CreateTextFormat(_api.fontMetrics.fontName.get(), _api.fontMetrics.fontCollection.get(), fontWeight, fontStyle, DWRITE_FONT_STRETCH_NORMAL, _api.fontMetrics.fontSizeInDIP, L"", textFormat.put())); + 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); @@ -1204,7 +1199,7 @@ void AtlasEngine::_flushBufferLine() /* textPosition */ idx, /* textLength */ gsl::narrow_cast(_api.bufferLine.size()) - idx, /* baseFontCollection */ fontCollection.get(), - /* baseFamilyName */ _api.fontMetrics.fontName.get(), + /* baseFamilyName */ _api.fontMetrics.fontName.c_str(), /* fontAxisValues */ textFormatAxis.data(), /* fontAxisValueCount */ gsl::narrow_cast(textFormatAxis.size()), /* mappedLength */ &mappedLength, @@ -1223,7 +1218,7 @@ void AtlasEngine::_flushBufferLine() /* textPosition */ idx, /* textLength */ gsl::narrow_cast(_api.bufferLine.size()) - idx, /* baseFontCollection */ fontCollection.get(), - /* baseFamilyName */ _api.fontMetrics.fontName.get(), + /* baseFamilyName */ _api.fontMetrics.fontName.c_str(), /* baseWeight */ baseWeight, /* baseStyle */ baseStyle, /* baseStretch */ DWRITE_FONT_STRETCH_NORMAL, diff --git a/src/renderer/atlas/AtlasEngine.h b/src/renderer/atlas/AtlasEngine.h index ac0d2ece6a1..6f461aa188d 100644 --- a/src/renderer/atlas/AtlasEngine.h +++ b/src/renderer/atlas/AtlasEngine.h @@ -394,14 +394,17 @@ namespace Microsoft::Console::Render struct FontMetrics { wil::com_ptr fontCollection; - wil::unique_process_heap_string fontName; + std::wstring fontName; float baselineInDIP = 0.0f; float fontSizeInDIP = 0.0f; u16x2 cellSize; u16 fontWeight = 0; u16 underlinePos = 0; + u16 underlineWidth = 0; u16 strikethroughPos = 0; - u16 lineThickness = 0; + u16 strikethroughWidth = 0; + u16x2 doubleUnderlinePos; + u16 thinLineWidth = 0; }; // These flags are shared with shader_ps.hlsl. @@ -823,8 +826,12 @@ namespace Microsoft::Console::Render alignas(sizeof(f32)) f32 enhancedContrast = 0; alignas(sizeof(u32)) u32 cellCountX = 0; alignas(sizeof(u32x2)) u32x2 cellSize; - alignas(sizeof(u32x2)) u32x2 underlinePos; - alignas(sizeof(u32x2)) u32x2 strikethroughPos; + alignas(sizeof(u32)) u32 underlinePos = 0; + alignas(sizeof(u32)) u32 underlineWidth = 0; + alignas(sizeof(u32)) u32 strikethroughPos = 0; + alignas(sizeof(u32)) u32 strikethroughWidth = 0; + alignas(sizeof(u32x2)) u32x2 doubleUnderlinePos; + alignas(sizeof(u32)) u32 thinLineWidth = 0; alignas(sizeof(u32)) u32 backgroundColor = 0; alignas(sizeof(u32)) u32 cursorColor = 0; alignas(sizeof(u32)) u32 selectionColor = 0; @@ -943,12 +950,9 @@ namespace Microsoft::Console::Render Buffer cells; // invalidated by ApiInvalidations::Size Buffer cellGlyphMapping; // invalidated by ApiInvalidations::Size f32x2 cellSizeDIP; // invalidated by ApiInvalidations::Font, caches _api.cellSize but in DIP - u16x2 cellSize; // invalidated by ApiInvalidations::Font, caches _api.cellSize u16x2 cellCount; // invalidated by ApiInvalidations::Font|Size, caches _api.cellCount - u16 underlinePos = 0; - u16 strikethroughPos = 0; - u16 lineThickness = 0; u16 dpi = USER_DEFAULT_SCREEN_DPI; // invalidated by ApiInvalidations::Font, caches _api.dpi + FontMetrics fontMetrics; // invalidated by ApiInvalidations::Font, cached _api.fontMetrics u16x2 atlasSizeInPixel; // invalidated by ApiInvalidations::Font TileHashMap glyphs; TileAllocator tileAllocator; diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index f2b5cdfefb9..08c6a291f8a 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -118,17 +118,21 @@ void AtlasEngine::_updateConstantBuffer() const noexcept ConstBuffer data; data.viewport.x = 0; data.viewport.y = 0; - data.viewport.z = static_cast(_r.cellCount.x * _r.cellSize.x); - data.viewport.w = static_cast(_r.cellCount.y * _r.cellSize.y); + data.viewport.z = static_cast(_r.cellCount.x * _r.fontMetrics.cellSize.x); + data.viewport.w = static_cast(_r.cellCount.y * _r.fontMetrics.cellSize.y); DWrite_GetGammaRatios(_r.gamma, data.gammaRatios); data.enhancedContrast = useClearType ? _r.cleartypeEnhancedContrast : _r.grayscaleEnhancedContrast; data.cellCountX = _r.cellCount.x; - data.cellSize.x = _r.cellSize.x; - data.cellSize.y = _r.cellSize.y; - data.underlinePos.x = _r.underlinePos; - data.underlinePos.y = _r.underlinePos + _r.lineThickness; - data.strikethroughPos.x = _r.strikethroughPos; - data.strikethroughPos.y = _r.strikethroughPos + _r.lineThickness; + 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; + data.strikethroughWidth = _r.fontMetrics.strikethroughWidth; + data.doubleUnderlinePos.x = _r.fontMetrics.doubleUnderlinePos.x; + data.doubleUnderlinePos.y = _r.fontMetrics.doubleUnderlinePos.y; + data.thinLineWidth = _r.fontMetrics.thinLineWidth; data.backgroundColor = _r.backgroundColor; data.cursorColor = _r.cursorOptions.cursorColor; data.selectionColor = _r.selectionColor; diff --git a/src/renderer/atlas/shader_ps.hlsl b/src/renderer/atlas/shader_ps.hlsl index 789cd98b00a..a94418068a7 100644 --- a/src/renderer/atlas/shader_ps.hlsl +++ b/src/renderer/atlas/shader_ps.hlsl @@ -43,8 +43,12 @@ cbuffer ConstBuffer : register(b0) float enhancedContrast; uint cellCountX; uint2 cellSize; - uint2 underlinePos; - uint2 strikethroughPos; + uint underlinePos; + uint underlineWidth; + uint strikethroughPos; + uint strikethroughWidth; + uint2 doubleUnderlinePos; + uint thinLineWidth; uint backgroundColor; uint cursorColor; uint selectionColor; @@ -107,22 +111,7 @@ float4 main(float4 pos: SV_Position): SV_Target } // Layer 2: - // Step 1: Underlines - [branch] if (cell.flags & CellFlags_Underline) - { - [flatten] if (cellPos.y >= underlinePos.x && cellPos.y < underlinePos.y) - { - color = alphaBlendPremultiplied(color, fg); - } - } - [branch] if (cell.flags & CellFlags_UnderlineDotted) - { - [flatten] if (cellPos.y >= underlinePos.x && cellPos.y < underlinePos.y && (viewportPos.x / (underlinePos.y - underlinePos.x) & 3) == 0) - { - color = alphaBlendPremultiplied(color, fg); - } - } - // Step 2: The cell's glyph, potentially drawn in the foreground color + // Step 1: The cell's glyph, potentially drawn in the foreground color { float4 glyph = glyphs[decodeU16x2(cell.glyphPos) + cellPos]; @@ -152,10 +141,31 @@ float4 main(float4 pos: SV_Position): SV_Target } } } - // Step 3: Lines, but not "under"lines - [branch] if (cell.flags & CellFlags_Strikethrough) + // Step 2: Lines { - [flatten] if (cellPos.y >= strikethroughPos.x && cellPos.y < strikethroughPos.y) + // What a nice coincidence that we have exactly 8 flags to handle right now! + bool4x2 flags = { + cell.flags & CellFlags_BorderLeft, + cell.flags & CellFlags_BorderTop, + cell.flags & CellFlags_BorderRight, + cell.flags & CellFlags_BorderBottom, + cell.flags & CellFlags_Underline, + cell.flags & CellFlags_UnderlineDotted, + cell.flags & CellFlags_UnderlineDouble, + cell.flags & CellFlags_Strikethrough, + }; + bool4x2 checks = { + cellPos < thinLineWidth, + (cellSize - cellPos) <= thinLineWidth, + // The following = lo && y < hi`. + (cellPos.y - underlinePos) < underlineWidth, + (cellPos.y - underlinePos) < underlineWidth && (viewportPos.x / underlineWidth & 3) == 0, + any((cellPos.y - doubleUnderlinePos) < thinLineWidth), + (cellPos.y - strikethroughPos) < strikethroughWidth, + }; + [flatten] if (any(flags && checks)) { color = alphaBlendPremultiplied(color, fg); } From 336b27558b058d2ab1b0fcb5ecbbddf1cb49db4b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Jul 2022 18:06:01 +0200 Subject: [PATCH 2/5] Add additional comments --- src/renderer/atlas/AtlasEngine.api.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 2f28e3ca811..656049efe98 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -632,11 +632,19 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo // 3. The top underline is vertically in the middle between baseline and bottom underline // 4. If the top line gets too close to the baseline the underlines are shifted downwards // 5. The minimum gap between the two lines appears to be similar to Tex (1.2pt) - auto doubleUnderlinePosBottom = underlinePos + underlineWidth - thinLineWidth; // 2. - auto doubleUnderlinePosTop = std::roundf((baseline + doubleUnderlinePosBottom - thinLineWidth) / 2.0f); // 3. - doubleUnderlinePosTop = std::max(doubleUnderlinePosTop, baseline + thinLineWidth); // 4. - const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi)); // 5. - doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + thinLineWidth + doubleUnderlineGap); // 5. + // (Additional notes below.) + + // 2. + auto doubleUnderlinePosBottom = underlinePos + underlineWidth - thinLineWidth; + // 3. Since we don't align the center of our two lines, but rather the top borders + // we need to subtract half a line width from our center point. + auto doubleUnderlinePosTop = std::roundf((baseline + doubleUnderlinePosBottom - thinLineWidth) / 2.0f); + // 4. + doubleUnderlinePosTop = std::max(doubleUnderlinePosTop, baseline + thinLineWidth); + // 5. The gap is only the distance _between_ the lines, but we need the distance from the + // top border of the top and bottom lines, which includes an additional line width. + const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi)); + doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + thinLineWidth + doubleUnderlineGap); // Our cells can't overlap each other so we additionally clamp the bottom line to be inside the cell boundaries. doubleUnderlinePosBottom = std::min(doubleUnderlinePosBottom, lineHeight - thinLineWidth); From 92a0518535b54e279a349e858965705931da114f Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 26 Jul 2022 20:24:16 +0200 Subject: [PATCH 3/5] Wording --- src/renderer/atlas/AtlasEngine.api.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 656049efe98..849c7970423 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -626,10 +626,10 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo const auto strikethroughWidth = std::max(1.0f, std::roundf(strikethroughThickness)); const auto thinLineWidth = std::max(1.0f, std::roundf(underlineThickness / 2.0f)); - // For double underlines we loosely follow what Word: + // For double underlines we loosely follow what Word does: // 1. The lines are half the width of an underline (= thinLineWidth) // 2. Ideally the bottom line is aligned with the bottom of the underline - // 3. The top underline is vertically in the middle between baseline and bottom underline + // 3. The top underline is vertically in the middle between baseline and ideal bottom underline // 4. If the top line gets too close to the baseline the underlines are shifted downwards // 5. The minimum gap between the two lines appears to be similar to Tex (1.2pt) // (Additional notes below.) @@ -644,7 +644,7 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo // 5. The gap is only the distance _between_ the lines, but we need the distance from the // top border of the top and bottom lines, which includes an additional line width. const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi)); - doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + thinLineWidth + doubleUnderlineGap); + doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + doubleUnderlineGap + thinLineWidth); // Our cells can't overlap each other so we additionally clamp the bottom line to be inside the cell boundaries. doubleUnderlinePosBottom = std::min(doubleUnderlinePosBottom, lineHeight - thinLineWidth); From 060c9bd83c38423bf5c88d29d2e9a038c010d57b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Wed, 27 Jul 2022 01:20:07 +0200 Subject: [PATCH 4/5] Address feedback --- src/renderer/atlas/shader_ps.hlsl | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/renderer/atlas/shader_ps.hlsl b/src/renderer/atlas/shader_ps.hlsl index a94418068a7..87c77f0526a 100644 --- a/src/renderer/atlas/shader_ps.hlsl +++ b/src/renderer/atlas/shader_ps.hlsl @@ -144,7 +144,9 @@ float4 main(float4 pos: SV_Position): SV_Target // Step 2: Lines { // What a nice coincidence that we have exactly 8 flags to handle right now! - bool4x2 flags = { + // `mask` will mask away any positive results from checks we don't want. + // (I.e. even if we're in an underline, it doesn't matter if we don't want an underline.) + bool4x2 mask = { cell.flags & CellFlags_BorderLeft, cell.flags & CellFlags_BorderTop, cell.flags & CellFlags_BorderRight, @@ -154,18 +156,21 @@ float4 main(float4 pos: SV_Position): SV_Target cell.flags & CellFlags_UnderlineDouble, cell.flags & CellFlags_Strikethrough, }; + // The following = lo && y < hi`. bool4x2 checks = { + // These 2 expand to 4 bools, because cellPos is a + // uint2 vector which results in a bool2 result each. cellPos < thinLineWidth, (cellSize - cellPos) <= thinLineWidth, - // The following = lo && y < hi`. + // These 4 are 4 regular bools. (cellPos.y - underlinePos) < underlineWidth, (cellPos.y - underlinePos) < underlineWidth && (viewportPos.x / underlineWidth & 3) == 0, any((cellPos.y - doubleUnderlinePos) < thinLineWidth), (cellPos.y - strikethroughPos) < strikethroughWidth, }; - [flatten] if (any(flags && checks)) + [flatten] if (any(mask && checks)) { color = alphaBlendPremultiplied(color, fg); } From 8450182f8593534e4eb6ddb7d27d122300536241 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 28 Jul 2022 14:57:36 +0200 Subject: [PATCH 5/5] Address feedback --- src/renderer/atlas/AtlasEngine.api.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 849c7970423..30ea5e20c88 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -671,14 +671,14 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo if (fontMetrics) { std::wstring fontName{ requestedFaceName }; - const auto fontWeightU16 = gsl::narrow(requestedWeight); - const auto underlinePosU16 = gsl::narrow(underlinePos); - const auto underlineWidthU16 = gsl::narrow(underlineWidth); - const auto strikethroughPosU16 = gsl::narrow(strikethroughPos); - const auto strikethroughWidthU16 = gsl::narrow(strikethroughWidth); - const auto doubleUnderlinePosTopU16 = gsl::narrow(doubleUnderlinePosTop); - const auto doubleUnderlinePosBottomU16 = gsl::narrow(doubleUnderlinePosBottom); - const auto thinLineWidthU16 = gsl::narrow(thinLineWidth); + const auto fontWeightU16 = gsl::narrow_cast(requestedWeight); + const auto underlinePosU16 = gsl::narrow_cast(underlinePos); + const auto underlineWidthU16 = gsl::narrow_cast(underlineWidth); + const auto strikethroughPosU16 = gsl::narrow_cast(strikethroughPos); + const auto strikethroughWidthU16 = gsl::narrow_cast(strikethroughWidth); + const auto doubleUnderlinePosTopU16 = gsl::narrow_cast(doubleUnderlinePosTop); + const auto doubleUnderlinePosBottomU16 = gsl::narrow_cast(doubleUnderlinePosBottom); + const auto thinLineWidthU16 = gsl::narrow_cast(thinLineWidth); // NOTE: From this point onward no early returns or throwing code should exist, // as we might cause _api to be in an inconsistent state otherwise.