From 6ee8099a2c92b99b87d5062e06835a9b3d75f183 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Thu, 30 Jul 2020 23:43:37 +0100 Subject: [PATCH] Refactor grid line renderers with support for more line types (#7107) This is a refactoring of the grid line renderers, adjusting the line widths to scale with the font size, and optimising the implementation to cut down on the number of draw calls. It also extends the supported grid line types to include true underlines and strike-through lines in the style of the active font. The main gist of the optimisation was to render the horizontal lines with a single draw call, instead of a loop with lots of little strokes joined together. In the case of the vertical lines, which still needed to be handled in a loop, I've tried to move the majority of static calculations outside the loop, so there is bit of optimisation there too. At the same time this code was updated to support a variable stroke width for the lines, instead of having them hardcoded to 1 pixel. The width is now calculated as a fraction of the font size (0.025 "em"), which is still going to be 1 pixel wide in most typical usage, but will scale up appropriately if you zoom in far enough. And in preparation for supporting the SGR strike-through attribute, and true underlines, I've extended the grid line renders with options for handling those line types as well. The offset and thickness of the lines is obtained from the font metrics (rounded to a pixel width, with a minimum of one pixel), so they match the style of the font. VALIDATION For now we're still only rendering grid lines, and only the top and bottom lines in the case of the DirectX renderer in Windows Terminal. So to test, I hacked in some code to force the renderer to use all the different options, confirming that they were working in both the GDI and DirectX renderers. I've tested the output with a number of different fonts, comparing it with the same text rendered in WordPad. For the most part they match exactly, but there can be slight differences when we adjust the font size for grid alignment. And in the case of the GDI renderer, where we're working with pixel heights rather than points, it's difficult to match the sizes exactly. This is a first step towards supporting the strike-through attribute (#6205) and true underlines (#2915). Closes #6911 --- .../actions/spell-check/dictionary/apis.txt | 3 + src/renderer/dx/DxRenderer.cpp | 135 ++++++++++++------ src/renderer/dx/DxRenderer.hpp | 13 +- src/renderer/gdi/gdirenderer.hpp | 10 ++ src/renderer/gdi/paint.cpp | 64 ++++++--- src/renderer/gdi/state.cpp | 37 +++++ src/renderer/inc/IRenderEngine.hpp | 4 +- 7 files changed, 200 insertions(+), 66 deletions(-) diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt index 298b869c186..1e33da10d7e 100644 --- a/.github/actions/spell-check/dictionary/apis.txt +++ b/.github/actions/spell-check/dictionary/apis.txt @@ -23,6 +23,7 @@ IObject IStorage llabs LCID +lround LSHIFT NCHITTEST NCLBUTTONDBLCLK @@ -31,6 +32,8 @@ NOAGGREGATION NOREDIRECTIONBITMAP oaidl ocidl +otms +OUTLINETEXTMETRICW PAGESCROLL RETURNCMD rfind diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index e5d3d848e18..cdd1bc72e58 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1484,63 +1484,84 @@ try _d2dBrushForeground->SetColor(_ColorFFromColorRef(color)); - const auto font = _glyphCell; - D2D_POINT_2F target = til::point{ coordTarget } * font; + const D2D1_SIZE_F font = _glyphCell; + const D2D_POINT_2F target = { coordTarget.X * font.width, coordTarget.Y * font.height }; + const auto fullRunWidth = font.width * gsl::narrow_cast(cchLine); - D2D_POINT_2F start = { 0 }; - D2D_POINT_2F end = { 0 }; - - for (size_t i = 0; i < cchLine; i++) + const auto DrawLine = [=](const auto x0, const auto y0, const auto x1, const auto y1, const auto strokeWidth) noexcept { - // 0.5 pixel offset for crisp lines - start = { target.x + 0.5f, target.y + 0.5f }; + _d2dDeviceContext->DrawLine({ x0, y0 }, { x1, y1 }, _d2dBrushForeground.Get(), strokeWidth, _strokeStyle.Get()); + }; - if (lines & GridLines::Top) - { - end = start; - end.x += font.width(); + // NOTE: Line coordinates are centered within the line, so they need to be + // offset by half the stroke width. For the start coordinate we add half + // the stroke width, and for the end coordinate we subtract half the width. - _d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); - } + if (lines & (GridLines::Left | GridLines::Right)) + { + const auto halfGridlineWidth = _lineMetrics.gridlineWidth / 2.0f; + const auto startY = target.y + halfGridlineWidth; + const auto endY = target.y + font.height - halfGridlineWidth; if (lines & GridLines::Left) { - end = start; - end.y += font.height(); + auto x = target.x + halfGridlineWidth; + for (size_t i = 0; i < cchLine; i++, x += font.width) + { + DrawLine(x, startY, x, endY, _lineMetrics.gridlineWidth); + } + } - _d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); + if (lines & GridLines::Right) + { + auto x = target.x + font.width - halfGridlineWidth; + for (size_t i = 0; i < cchLine; i++, x += font.width) + { + DrawLine(x, startY, x, endY, _lineMetrics.gridlineWidth); + } } + } - // NOTE: Watch out for inclusive/exclusive rectangles here. - // We have to remove 1 from the font size for the bottom and right lines to ensure that the - // starting point remains within the clipping rectangle. - // For example, if we're drawing a letter at 0,0 and the font size is 8x16.... - // The bottom left corner inclusive is at 0,15 which is Y (0) + Font Height (16) - 1 = 15. - // The top right corner inclusive is at 7,0 which is X (0) + Font Height (8) - 1 = 7. + if (lines & (GridLines::Top | GridLines::Bottom)) + { + const auto halfGridlineWidth = _lineMetrics.gridlineWidth / 2.0f; + const auto startX = target.x + halfGridlineWidth; + const auto endX = target.x + fullRunWidth - halfGridlineWidth; - // 0.5 pixel offset for crisp lines; -0.5 on the Y to fit _in_ the cell, not outside it. - start = { target.x + 0.5f, target.y + font.height() - 0.5f }; + if (lines & GridLines::Top) + { + const auto y = target.y + halfGridlineWidth; + DrawLine(startX, y, endX, y, _lineMetrics.gridlineWidth); + } if (lines & GridLines::Bottom) { - end = start; - end.x += font.width() - 1.f; - - _d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); + const auto y = target.y + font.height - halfGridlineWidth; + DrawLine(startX, y, endX, y, _lineMetrics.gridlineWidth); } + } - start = { target.x + font.width() - 0.5f, target.y + 0.5f }; + // In the case of the underline and strikethrough offsets, the stroke width + // is already accounted for, so they don't require further adjustments. - if (lines & GridLines::Right) - { - end = start; - end.y += font.height() - 1.f; + if (lines & GridLines::Underline) + { + const auto halfUnderlineWidth = _lineMetrics.underlineWidth / 2.0f; + const auto startX = target.x + halfUnderlineWidth; + const auto endX = target.x + fullRunWidth - halfUnderlineWidth; + const auto y = target.y + _lineMetrics.underlineOffset; - _d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); - } + DrawLine(startX, y, endX, y, _lineMetrics.underlineWidth); + } - // Move to the next character in this run. - target.x += font.width(); + if (lines & GridLines::Strikethrough) + { + const auto halfStrikethroughWidth = _lineMetrics.strikethroughWidth / 2.0f; + const auto startX = target.x + halfStrikethroughWidth; + const auto endX = target.x + fullRunWidth - halfStrikethroughWidth; + const auto y = target.y + _lineMetrics.strikethroughOffset; + + DrawLine(startX, y, endX, y, _lineMetrics.strikethroughWidth); } return S_OK; @@ -1707,7 +1728,8 @@ try _dpi, _dwriteTextFormat, _dwriteTextAnalyzer, - _dwriteFontFace)); + _dwriteFontFace, + _lineMetrics)); _glyphCell = fiFontInfo.GetSize(); @@ -1796,13 +1818,15 @@ float DxEngine::GetScaling() const noexcept Microsoft::WRL::ComPtr format; Microsoft::WRL::ComPtr analyzer; Microsoft::WRL::ComPtr face; + LineMetrics lineMetrics; return _GetProposedFont(pfiFontInfoDesired, pfiFontInfo, iDpi, format, analyzer, - face); + face, + lineMetrics); } // Routine Description: @@ -2071,7 +2095,8 @@ CATCH_RETURN(); const int dpi, Microsoft::WRL::ComPtr& textFormat, Microsoft::WRL::ComPtr& textAnalyzer, - Microsoft::WRL::ComPtr& fontFace) const noexcept + Microsoft::WRL::ComPtr& fontFace, + LineMetrics& lineMetrics) const noexcept { try { @@ -2234,6 +2259,34 @@ CATCH_RETURN(); false, scaled, unscaled); + + // There is no font metric for the grid line width, so we use a small + // multiple of the font size, which typically rounds to a pixel. + lineMetrics.gridlineWidth = std::round(fontSize * 0.025f); + + // All other line metrics are in design units, so to get a pixel value, + // we scale by the font size divided by the design-units-per-em. + const auto scale = fontSize / fontMetrics.designUnitsPerEm; + lineMetrics.underlineOffset = std::round(fontMetrics.underlinePosition * scale); + lineMetrics.underlineWidth = std::round(fontMetrics.underlineThickness * scale); + lineMetrics.strikethroughOffset = std::round(fontMetrics.strikethroughPosition * scale); + lineMetrics.strikethroughWidth = std::round(fontMetrics.strikethroughThickness * scale); + + // We always want the lines to be visible, so if a stroke width ends up + // at zero after rounding, we need to make it at least 1 pixel. + lineMetrics.gridlineWidth = std::max(lineMetrics.gridlineWidth, 1.0f); + lineMetrics.underlineWidth = std::max(lineMetrics.underlineWidth, 1.0f); + lineMetrics.strikethroughWidth = std::max(lineMetrics.strikethroughWidth, 1.0f); + + // Offsets are relative to the base line of the font, so we subtract + // from the ascent to get an offset relative to the top of the cell. + lineMetrics.underlineOffset = fullPixelAscent - lineMetrics.underlineOffset; + lineMetrics.strikethroughOffset = fullPixelAscent - lineMetrics.strikethroughOffset; + + // We also add half the stroke width to the offset, since the line + // coordinates designate the center of the line. + lineMetrics.underlineOffset += lineMetrics.underlineWidth / 2.0f; + lineMetrics.strikethroughOffset += lineMetrics.strikethroughWidth / 2.0f; } CATCH_RETURN(); diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index c56262d7d28..5eeb781a57f 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -142,6 +142,16 @@ namespace Microsoft::Console::Render bool _isEnabled; bool _isPainting; + struct LineMetrics + { + float gridlineWidth; + float underlineOffset; + float underlineWidth; + float strikethroughOffset; + float strikethroughWidth; + }; + + LineMetrics _lineMetrics; til::size _displaySizePixels; til::size _glyphCell; ::Microsoft::WRL::ComPtr _boxDrawingEffect; @@ -267,7 +277,8 @@ namespace Microsoft::Console::Render const int dpi, ::Microsoft::WRL::ComPtr& textFormat, ::Microsoft::WRL::ComPtr& textAnalyzer, - ::Microsoft::WRL::ComPtr& fontFace) const noexcept; + ::Microsoft::WRL::ComPtr& fontFace, + LineMetrics& lineMetrics) const noexcept; [[nodiscard]] til::size _GetClientSize() const; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index c375f8b764a..ad3b7089f09 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -96,6 +96,16 @@ namespace Microsoft::Console::Render std::vector cursorInvertRects; + struct LineMetrics + { + int gridlineWidth; + int underlineOffset; + int underlineWidth; + int strikethroughOffset; + int strikethroughWidth; + }; + + LineMetrics _lineMetrics; COORD _coordFontLast; int _iCurrentDpi; diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index b416510f99e..2008b220399 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -459,40 +459,58 @@ using namespace Microsoft::Console::Render; auto restoreBrushOnExit = wil::scope_exit([&] { hbr.reset(SelectBrush(_hdcMemoryContext, hbrPrev.get())); }); // Get the font size so we know the size of the rectangle lines we'll be inscribing. - COORD const coordFontSize = _GetFontSize(); + const auto fontWidth = _GetFontSize().X; + const auto fontHeight = _GetFontSize().Y; + const auto widthOfAllCells = fontWidth * gsl::narrow_cast(cchLine); + + const auto DrawLine = [=](const auto x, const auto y, const auto w, const auto h) { + return PatBlt(_hdcMemoryContext, x, y, w, h, PATCOPY); + }; - // For each length of the line, inscribe the various lines as specified by the enum - for (size_t i = 0; i < cchLine; i++) + if (lines & GridLines::Left) { - if (lines & GridLines::Top) + auto x = ptTarget.x; + for (size_t i = 0; i < cchLine; i++, x += fontWidth) { - RETURN_HR_IF(E_FAIL, !(PatBlt(_hdcMemoryContext, ptTarget.x, ptTarget.y, coordFontSize.X, 1, PATCOPY))); + RETURN_HR_IF(E_FAIL, !DrawLine(x, ptTarget.y, _lineMetrics.gridlineWidth, fontHeight)); } + } - if (lines & GridLines::Left) + if (lines & GridLines::Right) + { + // NOTE: We have to subtract the stroke width from the cell width + // to ensure the x coordinate remains inside the clipping rectangle. + auto x = ptTarget.x + fontWidth - _lineMetrics.gridlineWidth; + for (size_t i = 0; i < cchLine; i++, x += fontWidth) { - RETURN_HR_IF(E_FAIL, !(PatBlt(_hdcMemoryContext, ptTarget.x, ptTarget.y, 1, coordFontSize.Y, PATCOPY))); + RETURN_HR_IF(E_FAIL, !DrawLine(x, ptTarget.y, _lineMetrics.gridlineWidth, fontHeight)); } + } - // NOTE: Watch out for inclusive/exclusive rectangles here. - // We have to remove 1 from the font size for the bottom and right lines to ensure that the - // starting point remains within the clipping rectangle. - // For example, if we're drawing a letter at 0,0 and the font size is 8x16.... - // The bottom left corner inclusive is at 0,15 which is Y (0) + Font Height (16) - 1 = 15. - // The top right corner inclusive is at 7,0 which is X (0) + Font Height (8) - 1 = 7. + if (lines & GridLines::Top) + { + const auto y = ptTarget.y; + RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.gridlineWidth)); + } - if (lines & GridLines::Bottom) - { - RETURN_HR_IF(E_FAIL, !(PatBlt(_hdcMemoryContext, ptTarget.x, ptTarget.y + coordFontSize.Y - 1, coordFontSize.X, 1, PATCOPY))); - } + if (lines & GridLines::Bottom) + { + // NOTE: We have to subtract the stroke width from the cell height + // to ensure the y coordinate remains inside the clipping rectangle. + const auto y = ptTarget.y + fontHeight - _lineMetrics.gridlineWidth; + RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.gridlineWidth)); + } - if (lines & GridLines::Right) - { - RETURN_HR_IF(E_FAIL, !(PatBlt(_hdcMemoryContext, ptTarget.x + coordFontSize.X - 1, ptTarget.y, 1, coordFontSize.Y, PATCOPY))); - } + if (lines & GridLines::Underline) + { + const auto y = ptTarget.y + _lineMetrics.underlineOffset; + RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.underlineWidth)); + } - // Move to the next character in this run. - ptTarget.x += coordFontSize.X; + if (lines & GridLines::Strikethrough) + { + const auto y = ptTarget.y + _lineMetrics.strikethroughOffset; + RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.strikethroughWidth)); } return S_OK; diff --git a/src/renderer/gdi/state.cpp b/src/renderer/gdi/state.cpp index 65eec1aaa4e..e7ad9e2f6ef 100644 --- a/src/renderer/gdi/state.cpp +++ b/src/renderer/gdi/state.cpp @@ -232,6 +232,43 @@ GdiEngine::~GdiEngine() // Save off the font metrics for various other calculations RETURN_HR_IF(E_FAIL, !(GetTextMetricsW(_hdcMemoryContext, &_tmFontMetrics))); + // There is no font metric for the grid line width, so we use a small + // multiple of the font size, which typically rounds to a pixel. + const auto fontSize = _tmFontMetrics.tmHeight - _tmFontMetrics.tmInternalLeading; + _lineMetrics.gridlineWidth = std::lround(fontSize * 0.025); + + OUTLINETEXTMETRICW outlineMetrics; + if (GetOutlineTextMetricsW(_hdcMemoryContext, sizeof(outlineMetrics), &outlineMetrics)) + { + // For TrueType fonts, the other line metrics can be obtained from + // the font's outline text metric structure. + _lineMetrics.underlineOffset = outlineMetrics.otmsUnderscorePosition; + _lineMetrics.underlineWidth = outlineMetrics.otmsUnderscoreSize; + _lineMetrics.strikethroughOffset = outlineMetrics.otmsStrikeoutPosition; + _lineMetrics.strikethroughWidth = outlineMetrics.otmsStrikeoutSize; + } + else + { + // If we can't obtain the outline metrics for the font, we just pick + // some reasonable values for the offsets and widths. + _lineMetrics.underlineOffset = -std::lround(fontSize * 0.05); + _lineMetrics.underlineWidth = _lineMetrics.gridlineWidth; + _lineMetrics.strikethroughOffset = std::lround(_tmFontMetrics.tmAscent / 3.0); + _lineMetrics.strikethroughWidth = _lineMetrics.gridlineWidth; + } + + // We always want the lines to be visible, so if a stroke width ends + // up being zero, we need to make it at least 1 pixel. + _lineMetrics.gridlineWidth = std::max(_lineMetrics.gridlineWidth, 1); + _lineMetrics.underlineWidth = std::max(_lineMetrics.underlineWidth, 1); + _lineMetrics.strikethroughWidth = std::max(_lineMetrics.strikethroughWidth, 1); + + // Offsets are relative to the base line of the font, so we subtract + // from the ascent to get an offset relative to the top of the cell. + const auto ascent = _tmFontMetrics.tmAscent; + _lineMetrics.underlineOffset = ascent - _lineMetrics.underlineOffset; + _lineMetrics.strikethroughOffset = ascent - _lineMetrics.strikethroughOffset; + // Now find the size of a 0 in this current font and save it for conversions done later. _coordFontLast = Font.GetSize(); diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index a70960754c0..388ba308960 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -35,7 +35,9 @@ namespace Microsoft::Console::Render Top = 0x1, Bottom = 0x2, Left = 0x4, - Right = 0x8 + Right = 0x8, + Underline = 0x10, + Strikethrough = 0x20 }; virtual ~IRenderEngine() = 0;