From 9ad9b01d53706a99a8ea85be7d4e32bb156f3c25 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 12 Jun 2020 13:51:30 -0700 Subject: [PATCH 01/14] Recycle CustomTextLayouts between layout/render actions. This saves an awful lot of construction/destruction and memory allocation, especially with text that changes a lot (see: cacafire). --- src/renderer/dx/CustomTextLayout.cpp | 37 +++++++++++++++++++++-- src/renderer/dx/CustomTextLayout.h | 5 +++- src/renderer/dx/DxRenderer.cpp | 45 ++++++++++------------------ src/renderer/dx/DxRenderer.hpp | 2 ++ 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index d19d2d0c338..e62092b8db7 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -20,14 +20,13 @@ using namespace Microsoft::Console::Render; // - analyzer - DirectWrite text analyzer from the factory that has been cached at a level above this layout (expensive to create) // - format - The DirectWrite format object representing the size and other text properties to be applied (by default) to a layout // - font - The DirectWrite font face to use while calculating layout (by default, will fallback if necessary) -// - clusters - From the backing buffer, the text to be displayed clustered by the columns it should consume. + // - width - The count of pixels available per column (the expected pixel width of every column) // - boxEffect - Box drawing scaling effects that are cached for the base font across layouts. CustomTextLayout::CustomTextLayout(gsl::not_null const factory, gsl::not_null const analyzer, gsl::not_null const format, gsl::not_null const font, - std::basic_string_view const clusters, size_t const width, IBoxDrawingEffect* const boxEffect) : _factory{ factory.get() }, @@ -47,8 +46,37 @@ CustomTextLayout::CustomTextLayout(gsl::not_null const factory // Fetch the locale name out once now from the format _localeName.resize(gsl::narrow_cast(format->GetLocaleNameLength()) + 1); // +1 for null THROW_IF_FAILED(format->GetLocaleName(_localeName.data(), gsl::narrow(_localeName.size()))); +} + +//Routine Description: +// - Resets this custom text layout to the freshly allocated state in terms of text analysis. +// Arguments: +// - , modifies internal state +// Return Value: +// - S_OK or suitable memory management issue +[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::Reset() +try +{ + _runs.clear(); + _breakpoints.clear(); + _runIndex = 0; + _isEntireTextSimple = false; + _textClusterColumns.clear(); + _text.clear(); + return S_OK; +} +CATCH_RETURN() - _textClusterColumns.reserve(clusters.size()); +// Routine Description: +// - Appends text to this layout for analysis/processing. +// Arguments: +// - clusters - From the backing buffer, the text to be displayed clustered by the columns it should consume. +// Return Value: +// - S_OK or suitable memory management issue. +[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::AppendClusters(const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters) +try +{ + _textClusterColumns.reserve(_textClusterColumns.size() + clusters.size()); for (const auto& cluster : clusters) { @@ -64,7 +92,10 @@ CustomTextLayout::CustomTextLayout(gsl::not_null const factory _text += text; } + + return S_OK; } +CATCH_RETURN() // Routine Description: // - Figures out how many columns this layout should take. This will use the analyze step only. diff --git a/src/renderer/dx/CustomTextLayout.h b/src/renderer/dx/CustomTextLayout.h index f3916419dcc..42e73c5dfda 100644 --- a/src/renderer/dx/CustomTextLayout.h +++ b/src/renderer/dx/CustomTextLayout.h @@ -24,10 +24,13 @@ namespace Microsoft::Console::Render gsl::not_null const analyzer, gsl::not_null const format, gsl::not_null const font, - const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters, size_t const width, IBoxDrawingEffect* const boxEffect); + [[nodiscard]] HRESULT STDMETHODCALLTYPE AppendClusters(const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters); + + [[nodiscard]] HRESULT STDMETHODCALLTYPE Reset(); + [[nodiscard]] HRESULT STDMETHODCALLTYPE GetColumns(_Out_ UINT32* columns); // IDWriteTextLayout methods (but we don't actually want to implement them all, so just this one matching the existing interface) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 8966ff18e54..cbef6b051f7 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -95,6 +95,7 @@ DxEngine::DxEngine() : _scale{ 1.0f }, _prevScale{ 1.0f }, _chainMode{ SwapChainMode::ForComposition }, + _customLayout{ }, _customRenderer{ ::Microsoft::WRL::Make() } { const auto was = _tracelogCount.fetch_add(1); @@ -443,8 +444,7 @@ try switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { // use the HWND's dimensions for the swap chain dimensions. RECT rect = { 0 }; RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); @@ -473,8 +473,7 @@ try break; } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { // Use the given target size for compositions. SwapChainDesc.Width = _displaySizePixels.width(); SwapChainDesc.Height = _displaySizePixels.height(); @@ -881,15 +880,13 @@ CATCH_RETURN(); { switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { RECT clientRect = { 0 }; LOG_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &clientRect)); return til::rectangle{ clientRect }.size(); } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { return _sizeTarget; } default: @@ -1295,13 +1292,8 @@ try const D2D1_POINT_2F origin = til::point{ coord } * _glyphCell; // Create the text layout - CustomTextLayout layout(_dwriteFactory.Get(), - _dwriteTextAnalyzer.Get(), - _dwriteTextFormat.Get(), - _dwriteFontFace.Get(), - clusters, - _glyphCell.width(), - _boxDrawingEffect.Get()); + RETURN_IF_FAILED(_customLayout->Reset()); + RETURN_IF_FAILED(_customLayout->AppendClusters(clusters)); // Get the baseline for this font as that's where we draw from DWRITE_LINE_SPACING spacing; @@ -1344,7 +1336,7 @@ try D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); // Layout then render the text - RETURN_IF_FAILED(layout.Draw(&context, _customRenderer.Get(), origin.x, origin.y)); + RETURN_IF_FAILED(_customLayout->Draw(&context, _customRenderer.Get(), origin.x, origin.y)); return S_OK; } @@ -1589,6 +1581,9 @@ try // Calculate and cache the box effect for the base font. Scale is 1.0f because the base font is exactly the scale we want already. RETURN_IF_FAILED(CustomTextLayout::s_CalculateBoxEffect(_dwriteTextFormat.Get(), _glyphCell.width(), _dwriteFontFace.Get(), 1.0f, &_boxDrawingEffect)); + // Prepare the text layout + _customLayout = WRL::Make(_dwriteFactory.Get(), _dwriteTextAnalyzer.Get(), _dwriteTextFormat.Get(), _dwriteFontFace.Get(), _glyphCell.width(), _boxDrawingEffect.Get()); + return S_OK; } CATCH_RETURN(); @@ -1716,17 +1711,11 @@ try const Cluster cluster(glyph, 0); // columns don't matter, we're doing analysis not layout. - // Create the text layout - CustomTextLayout layout(_dwriteFactory.Get(), - _dwriteTextAnalyzer.Get(), - _dwriteTextFormat.Get(), - _dwriteFontFace.Get(), - { &cluster, 1 }, - _glyphCell.width(), - _boxDrawingEffect.Get()); + RETURN_IF_FAILED(_customLayout->Reset()); + RETURN_IF_FAILED(_customLayout->AppendClusters({ &cluster, 1 })); UINT32 columns = 0; - RETURN_IF_FAILED(layout.GetColumns(&columns)); + RETURN_IF_FAILED(_customLayout->GetColumns(&columns)); *pResult = columns != 1; @@ -2131,12 +2120,10 @@ CATCH_RETURN(); switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { return D2D1::ColorF(rgb); } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { // Get the A value we've snuck into the highest byte const BYTE a = ((color >> 24) & 0xFF); const float aFloat = a / 255.0f; diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index dc66ae1f668..144eca9eb7a 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -22,6 +22,7 @@ #include #include +#include "CustomTextLayout.h" #include "CustomTextRenderer.h" #include "../../types/inc/Viewport.hpp" @@ -171,6 +172,7 @@ namespace Microsoft::Console::Render ::Microsoft::WRL::ComPtr _dwriteTextFormat; ::Microsoft::WRL::ComPtr _dwriteFontFace; ::Microsoft::WRL::ComPtr _dwriteTextAnalyzer; + ::Microsoft::WRL::ComPtr _customLayout; ::Microsoft::WRL::ComPtr _customRenderer; ::Microsoft::WRL::ComPtr _strokeStyle; From 942e7d53e7b868cbe5dada60e1fae44f1d6b0ddc Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 12 Jun 2020 14:57:16 -0700 Subject: [PATCH 02/14] order runs inplace with std::sort to dump the overhead of vector alloc, memory copy, swap. --- src/renderer/dx/CustomTextLayout.cpp | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index e62092b8db7..677b260eacb 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -1858,21 +1858,13 @@ void CustomTextLayout::_SplitCurrentRun(const UINT32 splitPosition) // - void CustomTextLayout::_OrderRuns() { - const size_t totalRuns = _runs.size(); - std::vector runs; - runs.resize(totalRuns); - - UINT32 nextRunIndex = 0; - for (UINT32 i = 0; i < totalRuns; ++i) + std::sort(_runs.begin(), _runs.end(), [](auto&& a, auto&& b) { return a.textStart < b.textStart; }); + for (UINT32 i = 0; i < _runs.size() - 1; ++i) { - runs.at(i) = _runs.at(nextRunIndex); - runs.at(i).nextRunIndex = i + 1; - nextRunIndex = _runs.at(nextRunIndex).nextRunIndex; + _runs[i].nextRunIndex = i + 1; } - runs.back().nextRunIndex = 0; - - _runs.swap(runs); + _runs.back().nextRunIndex = 0; } #pragma endregion From bcb1ecd8be17881b6b5669f06d8e47fc6d44fa9a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 12 Jun 2020 15:05:35 -0700 Subject: [PATCH 03/14] be fussy about our &s. --- src/renderer/dx/CustomTextLayout.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index 677b260eacb..d94729e9792 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -1858,7 +1858,7 @@ void CustomTextLayout::_SplitCurrentRun(const UINT32 splitPosition) // - void CustomTextLayout::_OrderRuns() { - std::sort(_runs.begin(), _runs.end(), [](auto&& a, auto&& b) { return a.textStart < b.textStart; }); + std::sort(_runs.begin(), _runs.end(), [](auto& a, auto& b) { return a.textStart < b.textStart; }); for (UINT32 i = 0; i < _runs.size() - 1; ++i) { _runs[i].nextRunIndex = i + 1; From 7d0af686619d6109b903cec1c8f90c776be61c14 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 12 Jun 2020 15:08:25 -0700 Subject: [PATCH 04/14] code formatter. --- src/renderer/dx/DxRenderer.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index cbef6b051f7..176033dadc7 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -95,7 +95,7 @@ DxEngine::DxEngine() : _scale{ 1.0f }, _prevScale{ 1.0f }, _chainMode{ SwapChainMode::ForComposition }, - _customLayout{ }, + _customLayout{}, _customRenderer{ ::Microsoft::WRL::Make() } { const auto was = _tracelogCount.fetch_add(1); @@ -444,7 +444,8 @@ try switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { // use the HWND's dimensions for the swap chain dimensions. RECT rect = { 0 }; RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); @@ -473,7 +474,8 @@ try break; } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { // Use the given target size for compositions. SwapChainDesc.Width = _displaySizePixels.width(); SwapChainDesc.Height = _displaySizePixels.height(); @@ -880,13 +882,15 @@ CATCH_RETURN(); { switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { RECT clientRect = { 0 }; LOG_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &clientRect)); return til::rectangle{ clientRect }.size(); } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { return _sizeTarget; } default: @@ -2120,10 +2124,12 @@ CATCH_RETURN(); switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { return D2D1::ColorF(rgb); } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { // Get the A value we've snuck into the highest byte const BYTE a = ((color >> 24) & 0xFF); const float aFloat = a / 255.0f; From bff43ae9f7eb8b2f192e4bf27dd70b91f9e7c8a2 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 12 Jun 2020 16:13:13 -0700 Subject: [PATCH 05/14] consolidate push/pop clip calls to be per-row. --- src/renderer/dx/CustomTextRenderer.cpp | 54 ++++++++++++---- src/renderer/dx/CustomTextRenderer.h | 8 +++ src/renderer/dx/DxRenderer.cpp | 90 ++++++++++++++------------ src/renderer/dx/DxRenderer.hpp | 1 + 4 files changed, 99 insertions(+), 54 deletions(-) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index d4f3118af87..a57e11b4a28 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -409,6 +409,33 @@ CATCH_RETURN() ::Microsoft::WRL::ComPtr d2dContext; RETURN_IF_FAILED(drawingContext->renderTarget->QueryInterface(d2dContext.GetAddressOf())); + // Determine clip rectangle + D2D1_RECT_F clipRect; + clipRect.top = origin.y; + clipRect.bottom = clipRect.top + drawingContext->cellSize.height; + clipRect.left = 0; + clipRect.right = drawingContext->targetSize.width; + + if (_clipRect.top != clipRect.top || _clipRect.bottom != clipRect.bottom || + _clipRect.left != clipRect.left || _clipRect.right != clipRect.right) + { + if (_hasClipPushed) + { + d2dContext->PopAxisAlignedClip(); + _hasClipPushed = false; + } + + // Clip all drawing in this glyph run to where we expect. + // We need the AntialiasMode here to be Aliased to ensure + // that background boxes line up with each other and don't leave behind + // stray colors. + // See GH#3626 for more details. + d2dContext->PushAxisAlignedClip(clipRect, D2D1_ANTIALIAS_MODE_ALIASED); + + _clipRect = clipRect; + _hasClipPushed = true; + } + // Draw the background // The rectangle needs to be deduced based on the origin and the BidiDirection const auto advancesSpan = gsl::make_span(glyphRun->glyphAdvances, glyphRun->glyphCount); @@ -425,18 +452,6 @@ CATCH_RETURN() } rect.right = rect.left + totalSpan; - // Clip all drawing in this glyph run to where we expect. - // We need the AntialiasMode here to be Aliased to ensure - // that background boxes line up with each other and don't leave behind - // stray colors. - // See GH#3626 for more details. - d2dContext->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); - - // Ensure we pop it on the way out - auto popclip = wil::scope_exit([&d2dContext]() noexcept { - d2dContext->PopAxisAlignedClip(); - }); - d2dContext->FillRectangle(rect, drawingContext->backgroundBrush); RETURN_IF_FAILED(_drawCursor(d2dContext.Get(), rect, *drawingContext, true)); @@ -635,6 +650,21 @@ CATCH_RETURN() } #pragma endregion +[[nodiscard]] HRESULT CustomTextRenderer::EndFrame(void* clientDrawingContext) noexcept +try +{ + DrawingContext* drawingContext = static_cast(clientDrawingContext); + + if (_hasClipPushed) + { + drawingContext->renderTarget->PopAxisAlignedClip(); + _hasClipPushed = false; + } + + return S_OK; +} +CATCH_RETURN() + [[nodiscard]] HRESULT CustomTextRenderer::_DrawBasicGlyphRun(DrawingContext* clientDrawingContext, D2D1_POINT_2F baselineOrigin, DWRITE_MEASURING_MODE measuringMode, diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index e99cd864d2e..3114c309d83 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -18,6 +18,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory, const DWRITE_LINE_SPACING spacing, const D2D_SIZE_F cellSize, + const D2D_SIZE_F targetSize, const std::optional& cursorInfo, const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept { @@ -28,6 +29,7 @@ namespace Microsoft::Console::Render this->dwriteFactory = dwriteFactory; this->spacing = spacing; this->cellSize = cellSize; + this->targetSize = targetSize; this->cursorInfo = cursorInfo; this->options = options; } @@ -39,6 +41,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory; DWRITE_LINE_SPACING spacing; D2D_SIZE_F cellSize; + D2D_SIZE_F targetSize; std::optional cursorInfo; D2D1_DRAW_TEXT_OPTIONS options; }; @@ -98,6 +101,8 @@ namespace Microsoft::Console::Render BOOL isRightToLeft, IUnknown* clientDrawingEffect) noexcept override; + [[nodiscard]] HRESULT STDMETHODCALLTYPE EndFrame(void* clientDrawingContext) noexcept; + private: [[nodiscard]] HRESULT _FillRectangle(void* clientDrawingContext, IUnknown* clientDrawingEffect, @@ -128,5 +133,8 @@ namespace Microsoft::Console::Render DWRITE_MEASURING_MODE measuringMode, _In_ const DWRITE_GLYPH_RUN* glyphRun, _In_opt_ const DWRITE_GLYPH_RUN_DESCRIPTION* glyphRunDescription) noexcept; + + D2D1_RECT_F _clipRect; + bool _hasClipPushed = false; }; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 176033dadc7..62547b3b7cb 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -96,7 +96,8 @@ DxEngine::DxEngine() : _prevScale{ 1.0f }, _chainMode{ SwapChainMode::ForComposition }, _customLayout{}, - _customRenderer{ ::Microsoft::WRL::Make() } + _customRenderer{ ::Microsoft::WRL::Make() }, + _drawingContext{} { const auto was = _tracelogCount.fetch_add(1); if (0 == was) @@ -998,6 +999,49 @@ try _d2dRenderTarget->BeginDraw(); _isPainting = true; + + { + // Get the baseline for this font as that's where we draw from + DWRITE_LINE_SPACING spacing; + RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); + + // GH#5098: If we're rendering with cleartype text, we need to always + // render onto an opaque background. If our background's opacity is + // 1.0f, that's great, we can use that. Otherwise, we need to force the + // text renderer to render this text in grayscale. In + // UpdateDrawingBrushes, we'll set the backgroundColor's a channel to + // 1.0 if we're in cleartype mode and the background's opacity is 1.0. + // Otherwise, at this point, the _backgroundColor's alpha is <1.0. + // + // Currently, only text with the default background color uses an alpha + // of 0, every other background uses 1.0 + // + // DANGER: Layers slow us down. Only do this in the specific case where + // someone has chosen the slower ClearType antialiasing (versus the faster + // grayscale antialiasing) + const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE; + const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f; + // Another way of naming "bgIsDefault" is "bgHasTransparency" + const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) && + (_backgroundColor.r == _defaultBackgroundColor.r) && + (_backgroundColor.g == _defaultBackgroundColor.g) && + (_backgroundColor.b == _defaultBackgroundColor.b); + const bool forceGrayscaleAA = usingCleartype && + usingTransparency && + bgIsDefault; + + // Assemble the drawing context information + _drawingContext = std::make_unique(_d2dRenderTarget.Get(), + _d2dBrushForeground.Get(), + _d2dBrushBackground.Get(), + forceGrayscaleAA, + _dwriteFactory.Get(), + spacing, + _glyphCell, + _d2dRenderTarget->GetSize(), + _frameInfo.cursorInfo, + D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); + } } return S_OK; @@ -1021,6 +1065,8 @@ try { _isPainting = false; + LOG_IF_FAILED(_customRenderer->EndFrame(_drawingContext.get())); + hr = _d2dRenderTarget->EndDraw(); if (SUCCEEDED(hr)) @@ -1299,48 +1345,8 @@ try RETURN_IF_FAILED(_customLayout->Reset()); RETURN_IF_FAILED(_customLayout->AppendClusters(clusters)); - // Get the baseline for this font as that's where we draw from - DWRITE_LINE_SPACING spacing; - RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); - - // GH#5098: If we're rendering with cleartype text, we need to always - // render onto an opaque background. If our background's opacity is - // 1.0f, that's great, we can use that. Otherwise, we need to force the - // text renderer to render this text in grayscale. In - // UpdateDrawingBrushes, we'll set the backgroundColor's a channel to - // 1.0 if we're in cleartype mode and the background's opacity is 1.0. - // Otherwise, at this point, the _backgroundColor's alpha is <1.0. - // - // Currently, only text with the default background color uses an alpha - // of 0, every other background uses 1.0 - // - // DANGER: Layers slow us down. Only do this in the specific case where - // someone has chosen the slower ClearType antialiasing (versus the faster - // grayscale antialiasing) - const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE; - const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f; - // Another way of naming "bgIsDefault" is "bgHasTransparency" - const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) && - (_backgroundColor.r == _defaultBackgroundColor.r) && - (_backgroundColor.g == _defaultBackgroundColor.g) && - (_backgroundColor.b == _defaultBackgroundColor.b); - const bool forceGrayscaleAA = usingCleartype && - usingTransparency && - bgIsDefault; - - // Assemble the drawing context information - DrawingContext context(_d2dRenderTarget.Get(), - _d2dBrushForeground.Get(), - _d2dBrushBackground.Get(), - forceGrayscaleAA, - _dwriteFactory.Get(), - spacing, - _glyphCell, - _frameInfo.cursorInfo, - D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); - // Layout then render the text - RETURN_IF_FAILED(_customLayout->Draw(&context, _customRenderer.Get(), origin.x, origin.y)); + RETURN_IF_FAILED(_customLayout->Draw(_drawingContext.get(), _customRenderer.Get(), origin.x, origin.y)); return S_OK; } diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 144eca9eb7a..181b23062ee 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -188,6 +188,7 @@ namespace Microsoft::Console::Render UINT _swapChainFlags; ::Microsoft::WRL::ComPtr _dxgiSwapChain; wil::unique_handle _swapChainFrameLatencyWaitableObject; + std::unique_ptr _drawingContext; // Terminal effects resources. bool _retroTerminalEffects; From 2f44fb97dc41c388591132174bc90915d148f827 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 15 Jun 2020 13:52:01 -0700 Subject: [PATCH 06/14] eliminate cursor turds. --- src/renderer/dx/DxRenderer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 62547b3b7cb..77e1c775706 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -694,6 +694,7 @@ CATCH_RETURN() try { _sizeTarget = Pixels; + _invalidMap.resize(_sizeTarget / _glyphCell, true); return S_OK; } @@ -1345,6 +1346,9 @@ try RETURN_IF_FAILED(_customLayout->Reset()); RETURN_IF_FAILED(_customLayout->AppendClusters(clusters)); + // Copy cursor info into drawing context + _drawingContext->cursorInfo = _frameInfo.cursorInfo; + // Layout then render the text RETURN_IF_FAILED(_customLayout->Draw(_drawingContext.get(), _customRenderer.Get(), origin.x, origin.y)); From 6b34284c61bee1ca7310f5d5fbd92db77a50f589 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 15 Jun 2020 14:34:15 -0700 Subject: [PATCH 07/14] A little cleanup and static analysis fixes. --- src/renderer/dx/CustomTextLayout.cpp | 4 ++-- src/renderer/dx/CustomTextLayout.h | 2 +- src/renderer/dx/CustomTextRenderer.cpp | 1 + src/renderer/dx/CustomTextRenderer.h | 2 +- src/renderer/dx/DxRenderer.cpp | 4 +--- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index d94729e9792..dccba78e2da 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -54,7 +54,7 @@ CustomTextLayout::CustomTextLayout(gsl::not_null const factory // - , modifies internal state // Return Value: // - S_OK or suitable memory management issue -[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::Reset() +[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::Reset() noexcept try { _runs.clear(); @@ -1861,7 +1861,7 @@ void CustomTextLayout::_OrderRuns() std::sort(_runs.begin(), _runs.end(), [](auto& a, auto& b) { return a.textStart < b.textStart; }); for (UINT32 i = 0; i < _runs.size() - 1; ++i) { - _runs[i].nextRunIndex = i + 1; + til::at(_runs, i).nextRunIndex = i + 1; } _runs.back().nextRunIndex = 0; diff --git a/src/renderer/dx/CustomTextLayout.h b/src/renderer/dx/CustomTextLayout.h index 42e73c5dfda..dac30b06c12 100644 --- a/src/renderer/dx/CustomTextLayout.h +++ b/src/renderer/dx/CustomTextLayout.h @@ -29,7 +29,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT STDMETHODCALLTYPE AppendClusters(const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters); - [[nodiscard]] HRESULT STDMETHODCALLTYPE Reset(); + [[nodiscard]] HRESULT STDMETHODCALLTYPE Reset() noexcept; [[nodiscard]] HRESULT STDMETHODCALLTYPE GetColumns(_Out_ UINT32* columns); diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index a57e11b4a28..f58f756f1ef 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -654,6 +654,7 @@ CATCH_RETURN() try { DrawingContext* drawingContext = static_cast(clientDrawingContext); + RETURN_HR_IF(E_INVALIDARG, !drawingContext); if (_hasClipPushed) { diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index 3114c309d83..71bb190a873 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -134,7 +134,7 @@ namespace Microsoft::Console::Render _In_ const DWRITE_GLYPH_RUN* glyphRun, _In_opt_ const DWRITE_GLYPH_RUN_DESCRIPTION* glyphRunDescription) noexcept; - D2D1_RECT_F _clipRect; + D2D1_RECT_F _clipRect = { 0 }; bool _hasClipPushed = false; }; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 77e1c775706..be4ff668726 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1346,9 +1346,6 @@ try RETURN_IF_FAILED(_customLayout->Reset()); RETURN_IF_FAILED(_customLayout->AppendClusters(clusters)); - // Copy cursor info into drawing context - _drawingContext->cursorInfo = _frameInfo.cursorInfo; - // Layout then render the text RETURN_IF_FAILED(_customLayout->Draw(_drawingContext.get(), _customRenderer.Get(), origin.x, origin.y)); @@ -2215,5 +2212,6 @@ CATCH_LOG() [[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept { _frameInfo = info; + _drawingContext->cursorInfo = info.cursorInfo; return S_OK; } From 65c2d93d467991a346af43c9a0280d9f26e78135 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 15 Jun 2020 14:36:34 -0700 Subject: [PATCH 08/14] dump explicit frame info, just stash the cursor info where we need it. --- src/renderer/dx/DxRenderer.cpp | 3 +-- src/renderer/dx/DxRenderer.hpp | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index be4ff668726..b00b5156c25 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1040,7 +1040,7 @@ try spacing, _glyphCell, _d2dRenderTarget->GetSize(), - _frameInfo.cursorInfo, + std::nullopt, D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); } } @@ -2211,7 +2211,6 @@ CATCH_LOG() // - S_OK [[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept { - _frameInfo = info; _drawingContext->cursorInfo = info.cursorInfo; return S_OK; } diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 181b23062ee..c9f36bb2297 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -209,8 +209,6 @@ namespace Microsoft::Console::Render float _defaultTextBackgroundOpacity; - RenderFrameInfo _frameInfo; - // DirectX constant buffers need to be a multiple of 16; align to pad the size. __declspec(align(16)) struct { From 1ab1dec5a3231a575c3ac9ba5a31c1ce37e72100 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 17 Jun 2020 09:27:23 -0700 Subject: [PATCH 09/14] Change DrawingContext constructor to initializer list pattern. --- src/renderer/dx/CustomTextRenderer.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index 71bb190a873..9352eee77c0 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -20,18 +20,18 @@ namespace Microsoft::Console::Render const D2D_SIZE_F cellSize, const D2D_SIZE_F targetSize, const std::optional& cursorInfo, - const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept + const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept : + renderTarget(renderTarget), + foregroundBrush(foregroundBrush), + backgroundBrush(backgroundBrush), + forceGrayscaleAA(forceGrayscaleAA), + dwriteFactory(dwriteFactory), + spacing(spacing), + cellSize(cellSize), + targetSize(targetSize), + cursorInfo(cursorInfo), + options(options) { - this->renderTarget = renderTarget; - this->foregroundBrush = foregroundBrush; - this->backgroundBrush = backgroundBrush; - this->forceGrayscaleAA = forceGrayscaleAA; - this->dwriteFactory = dwriteFactory; - this->spacing = spacing; - this->cellSize = cellSize; - this->targetSize = targetSize; - this->cursorInfo = cursorInfo; - this->options = options; } ID2D1RenderTarget* renderTarget; From 8bd876b3a9aa77484abb7a8466662299e974cf87 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 17 Jun 2020 10:49:17 -0700 Subject: [PATCH 10/14] use optional instead of item and bool to ensure we don't end up in a torn state of remembering the rectangle dimensions when we think we popped the clip. --- src/renderer/dx/CustomTextRenderer.cpp | 31 ++++++++++++++++---------- src/renderer/dx/CustomTextRenderer.h | 3 +-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index f58f756f1ef..019e22b9553 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -416,24 +416,31 @@ CATCH_RETURN() clipRect.left = 0; clipRect.right = drawingContext->targetSize.width; - if (_clipRect.top != clipRect.top || _clipRect.bottom != clipRect.bottom || - _clipRect.left != clipRect.left || _clipRect.right != clipRect.right) + // If we already have a clip rectangle, check if it different than the previous one. + if (_clipRect.has_value()) { - if (_hasClipPushed) + const auto storedVal = _clipRect.value(); + // If it is different, pop off the old one and push the new one on. + if (storedVal.top != clipRect.top || storedVal.bottom != clipRect.bottom || + storedVal.left != clipRect.left || storedVal.right != clipRect.right) { d2dContext->PopAxisAlignedClip(); - _hasClipPushed = false; - } - // Clip all drawing in this glyph run to where we expect. - // We need the AntialiasMode here to be Aliased to ensure - // that background boxes line up with each other and don't leave behind - // stray colors. - // See GH#3626 for more details. + // Clip all drawing in this glyph run to where we expect. + // We need the AntialiasMode here to be Aliased to ensure + // that background boxes line up with each other and don't leave behind + // stray colors. + // See GH#3626 for more details. + d2dContext->PushAxisAlignedClip(clipRect, D2D1_ANTIALIAS_MODE_ALIASED); + _clipRect = clipRect; + } + } + // If we have no clip rectangle, it's easy. Push it on and go. + else + { + // See above for aliased flag explanation. d2dContext->PushAxisAlignedClip(clipRect, D2D1_ANTIALIAS_MODE_ALIASED); - _clipRect = clipRect; - _hasClipPushed = true; } // Draw the background diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index 9352eee77c0..a821e0546a4 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -134,7 +134,6 @@ namespace Microsoft::Console::Render _In_ const DWRITE_GLYPH_RUN* glyphRun, _In_opt_ const DWRITE_GLYPH_RUN_DESCRIPTION* glyphRunDescription) noexcept; - D2D1_RECT_F _clipRect = { 0 }; - bool _hasClipPushed = false; + std::optional _clipRect; }; } From d24986f2dbc626371bd5a5c5b07a5eb427202e37 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 17 Jun 2020 13:18:41 -0700 Subject: [PATCH 11/14] move grayscale aa calculation to helper, use it in brush updates to ensure that its state propagates correctly to the drawing context. --- src/renderer/dx/DxRenderer.cpp | 76 +++++++++++++++++++++++----------- src/renderer/dx/DxRenderer.hpp | 2 + 2 files changed, 53 insertions(+), 25 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index b00b5156c25..2bb610cad36 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -652,6 +652,45 @@ void DxEngine::_ReleaseDeviceResources() noexcept CATCH_LOG(); } +// Routine Description: +// - Calculates whether or not we should force grayscale AA based on the +// current renderer state. +// Arguments: +// - - Uses internal state of _antialiasingMode, _defaultTextBackgroundOpacity, +// _backgroundColor, and _defaultBackgroundColor. +// Return Value: +// - True if we must render this text in grayscale AA as cleartype simply won't work. False otherwise. +[[nodiscard]] bool DxEngine::_ShouldForceGrayscaleAA() noexcept +{ + // GH#5098: If we're rendering with cleartype text, we need to always + // render onto an opaque background. If our background's opacity is + // 1.0f, that's great, we can use that. Otherwise, we need to force the + // text renderer to render this text in grayscale. In + // UpdateDrawingBrushes, we'll set the backgroundColor's a channel to + // 1.0 if we're in cleartype mode and the background's opacity is 1.0. + // Otherwise, at this point, the _backgroundColor's alpha is <1.0. + // + // Currently, only text with the default background color uses an alpha + // of 0, every other background uses 1.0 + // + // DANGER: Layers slow us down. Only do this in the specific case where + // someone has chosen the slower ClearType antialiasing (versus the faster + // grayscale antialiasing) + const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE; + const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f; + // Another way of naming "bgIsDefault" is "bgHasTransparency" + const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) && + (_backgroundColor.r == _defaultBackgroundColor.r) && + (_backgroundColor.g == _defaultBackgroundColor.g) && + (_backgroundColor.b == _defaultBackgroundColor.b); + const bool forceGrayscaleAA = usingCleartype && + usingTransparency && + bgIsDefault; + + return forceGrayscaleAA; +} + + // Routine Description: // - Helper to create a DirectWrite text layout object // out of a string. @@ -1006,36 +1045,12 @@ try DWRITE_LINE_SPACING spacing; RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); - // GH#5098: If we're rendering with cleartype text, we need to always - // render onto an opaque background. If our background's opacity is - // 1.0f, that's great, we can use that. Otherwise, we need to force the - // text renderer to render this text in grayscale. In - // UpdateDrawingBrushes, we'll set the backgroundColor's a channel to - // 1.0 if we're in cleartype mode and the background's opacity is 1.0. - // Otherwise, at this point, the _backgroundColor's alpha is <1.0. - // - // Currently, only text with the default background color uses an alpha - // of 0, every other background uses 1.0 - // - // DANGER: Layers slow us down. Only do this in the specific case where - // someone has chosen the slower ClearType antialiasing (versus the faster - // grayscale antialiasing) - const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE; - const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f; - // Another way of naming "bgIsDefault" is "bgHasTransparency" - const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) && - (_backgroundColor.r == _defaultBackgroundColor.r) && - (_backgroundColor.g == _defaultBackgroundColor.g) && - (_backgroundColor.b == _defaultBackgroundColor.b); - const bool forceGrayscaleAA = usingCleartype && - usingTransparency && - bgIsDefault; // Assemble the drawing context information _drawingContext = std::make_unique(_d2dRenderTarget.Get(), _d2dBrushForeground.Get(), _d2dBrushBackground.Get(), - forceGrayscaleAA, + _ShouldForceGrayscaleAA(), _dwriteFactory.Get(), spacing, _glyphCell, @@ -1567,6 +1582,17 @@ CATCH_RETURN() }*/ } + // If we have a drawing context, it may be choosing its antialiasing based + // on the colors. Update it if it exists. + // We only need to do this here because this is called all the time on painting frames + // and will update it in a timely fashion. Changing the AA mode or opacity do affect + // it, but we will always hit updating the drawing brushes so we don't + // need to update this in those locations. + if (_drawingContext) + { + _drawingContext->forceGrayscaleAA = _ShouldForceGrayscaleAA(); + } + return S_OK; } diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index c9f36bb2297..485fe60eb65 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -225,6 +225,8 @@ namespace Microsoft::Console::Render void _ReleaseDeviceResources() noexcept; + bool _ShouldForceGrayscaleAA() noexcept; + [[nodiscard]] HRESULT _CreateTextLayout( _In_reads_(StringLength) PCWCHAR String, _In_ size_t StringLength, From 5f0adb7fbaa2d3224fefec311f20d8024399d6b8 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 17 Jun 2020 13:44:51 -0700 Subject: [PATCH 12/14] missed a spot with the optional. --- src/renderer/dx/CustomTextRenderer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index 019e22b9553..ba1ba6476a1 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -663,10 +663,10 @@ try DrawingContext* drawingContext = static_cast(clientDrawingContext); RETURN_HR_IF(E_INVALIDARG, !drawingContext); - if (_hasClipPushed) + if (_clipRect.has_value()) { drawingContext->renderTarget->PopAxisAlignedClip(); - _hasClipPushed = false; + _clipRect = std::nullopt; } return S_OK; From 747db5819c3e2a13f3b129ffc62b70f14e171b2b Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 17 Jun 2020 16:23:24 -0700 Subject: [PATCH 13/14] code format DID bark. --- src/renderer/dx/DxRenderer.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 2bb610cad36..6a0f3038c79 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -690,7 +690,6 @@ void DxEngine::_ReleaseDeviceResources() noexcept return forceGrayscaleAA; } - // Routine Description: // - Helper to create a DirectWrite text layout object // out of a string. @@ -1045,7 +1044,6 @@ try DWRITE_LINE_SPACING spacing; RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); - // Assemble the drawing context information _drawingContext = std::make_unique(_d2dRenderTarget.Get(), _d2dBrushForeground.Get(), From 74328f99a9f9059c9d32eaa93ebcda27ccb62edf Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 19 Jun 2020 14:20:33 -0700 Subject: [PATCH 14/14] Fix selection issue. Selection needs to dump the clip if it's leftover from the text drawing phases. Also delete a blank line I found. --- src/renderer/dx/CustomTextLayout.cpp | 1 - src/renderer/dx/CustomTextRenderer.cpp | 2 +- src/renderer/dx/CustomTextRenderer.h | 2 +- src/renderer/dx/DxRenderer.cpp | 6 +++++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index dccba78e2da..4c78a74cf44 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -20,7 +20,6 @@ using namespace Microsoft::Console::Render; // - analyzer - DirectWrite text analyzer from the factory that has been cached at a level above this layout (expensive to create) // - format - The DirectWrite format object representing the size and other text properties to be applied (by default) to a layout // - font - The DirectWrite font face to use while calculating layout (by default, will fallback if necessary) - // - width - The count of pixels available per column (the expected pixel width of every column) // - boxEffect - Box drawing scaling effects that are cached for the base font across layouts. CustomTextLayout::CustomTextLayout(gsl::not_null const factory, diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index ba1ba6476a1..764ef7fdaeb 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -657,7 +657,7 @@ CATCH_RETURN() } #pragma endregion -[[nodiscard]] HRESULT CustomTextRenderer::EndFrame(void* clientDrawingContext) noexcept +[[nodiscard]] HRESULT CustomTextRenderer::EndClip(void* clientDrawingContext) noexcept try { DrawingContext* drawingContext = static_cast(clientDrawingContext); diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index a821e0546a4..3ff50664434 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -101,7 +101,7 @@ namespace Microsoft::Console::Render BOOL isRightToLeft, IUnknown* clientDrawingEffect) noexcept override; - [[nodiscard]] HRESULT STDMETHODCALLTYPE EndFrame(void* clientDrawingContext) noexcept; + [[nodiscard]] HRESULT STDMETHODCALLTYPE EndClip(void* clientDrawingContext) noexcept; private: [[nodiscard]] HRESULT _FillRectangle(void* clientDrawingContext, diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 6a0f3038c79..fdb0f870a3d 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1079,7 +1079,8 @@ try { _isPainting = false; - LOG_IF_FAILED(_customRenderer->EndFrame(_drawingContext.get())); + // If there's still a clip hanging around, remove it. We're all done. + LOG_IF_FAILED(_customRenderer->EndClip(_drawingContext.get())); hr = _d2dRenderTarget->EndDraw(); @@ -1459,6 +1460,9 @@ CATCH_RETURN() [[nodiscard]] HRESULT DxEngine::PaintSelection(const SMALL_RECT rect) noexcept try { + // If a clip rectangle is in place from drawing the text layer, remove it here. + LOG_IF_FAILED(_customRenderer->EndClip(_drawingContext.get())); + const auto existingColor = _d2dBrushForeground->GetColor(); _d2dBrushForeground->SetColor(_selectionBackground);