From 171e0846c868b1d212d69a5386db0ec60d28560c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jun 2020 11:33:01 -0500 Subject: [PATCH 1/6] Move CursorOptions up into it's own file --- src/renderer/dx/CustomTextRenderer.h | 4 ++ src/renderer/dx/DxRenderer.cpp | 3 +- src/renderer/gdi/paint.cpp | 2 +- src/renderer/inc/CursorOptions.h | 50 ++++++++++++++++++++++++ src/renderer/inc/IRenderEngine.hpp | 32 +-------------- src/renderer/uia/UiaRenderer.cpp | 2 +- src/renderer/vt/XtermEngine.cpp | 2 +- src/renderer/vt/paint.cpp | 2 +- src/renderer/wddmcon/WddmConRenderer.cpp | 2 +- 9 files changed, 62 insertions(+), 37 deletions(-) create mode 100644 src/renderer/inc/CursorOptions.h diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index 3ba355cc1a6..f97e48f58a6 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -5,6 +5,7 @@ #include #include "BoxDrawingEffect.h" +#include "../../renderer/inc/CursorOptions.h" namespace Microsoft::Console::Render { @@ -17,6 +18,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory, const DWRITE_LINE_SPACING spacing, const D2D_SIZE_F cellSize, + const CursorOptions cursorInfo, const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept { this->renderTarget = renderTarget; @@ -26,6 +28,7 @@ namespace Microsoft::Console::Render this->dwriteFactory = dwriteFactory; this->spacing = spacing; this->cellSize = cellSize; + this->cursorInfo = cursorInfo; this->options = options; } @@ -36,6 +39,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory; DWRITE_LINE_SPACING spacing; D2D_SIZE_F cellSize; + CursorOptions cursorInfo; D2D1_DRAW_TEXT_OPTIONS options; }; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index a36c1856222..9dc45751d49 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1298,6 +1298,7 @@ try _dwriteFactory.Get(), spacing, _glyphCell, + {}, D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); // Layout then render the text @@ -1427,7 +1428,7 @@ enum class CursorPaintType // - options - Packed options relevant to how to draw the cursor // Return Value: // - S_OK or relevant DirectX error. -[[nodiscard]] HRESULT DxEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +[[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& options) noexcept try { // if the cursor is off, do nothing - it should not be visible. diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index 112479bcb87..7682a232523 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -507,7 +507,7 @@ using namespace Microsoft::Console::Render; // - options - Parameters that affect the way that the cursor is drawn // Return Value: // - S_OK, suitable GDI HRESULT error, or safemath error, or E_FAIL in a GDI error where a specific error isn't set. -[[nodiscard]] HRESULT GdiEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +[[nodiscard]] HRESULT GdiEngine::PaintCursor(const CursorOptions& options) noexcept { // if the cursor is off, do nothing - it should not be visible. if (!options.isOn) diff --git a/src/renderer/inc/CursorOptions.h b/src/renderer/inc/CursorOptions.h new file mode 100644 index 00000000000..27ddc795cd8 --- /dev/null +++ b/src/renderer/inc/CursorOptions.h @@ -0,0 +1,50 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- CursorOptions.h + +Abstract: +- A collection of state about the cursor that a render engine might need to display the cursor correctly. + +Author(s): +- Mike Griese, 03-Jun-2020 +--*/ + +#pragma once + +#include "../../inc/conattrs.hpp" + +namespace Microsoft::Console::Render +{ + struct CursorOptions + { + // Character cell in the grid to draw at + // This is relative to the viewport, not the buffer. + COORD coordCursor; + + // For an underscore type _ cursor, how tall it should be as a % of cell height + ULONG ulCursorHeightPercent; + + // For a vertical bar type | cursor, how many pixels wide it should be per ease of access preferences + ULONG cursorPixelWidth; + + // Whether to draw the cursor 2 cells wide (+X from the coordinate given) + bool fIsDoubleWidth; + + // Chooses a special cursor type like a full box, a vertical bar, etc. + CursorType cursorType; + + // Specifies to use the color below instead of the default color + bool fUseColor; + + // Color to use for drawing instead of the default + COLORREF cursorColor; + + // Is the cursor currently visually visible? + // If the cursor has blinked off, this is false. + // if the cursor has blinked on, this is true. + bool isOn; + }; +} diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index a55e0b64b3b..52827d3f048 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -14,7 +14,7 @@ Author(s): #pragma once -#include "../../inc/conattrs.hpp" +#include "CursorOptions.h" #include "Cluster.hpp" #include "FontInfoDesired.hpp" @@ -32,36 +32,6 @@ namespace Microsoft::Console::Render Right = 0x8 }; - struct CursorOptions - { - // Character cell in the grid to draw at - // This is relative to the viewport, not the buffer. - COORD coordCursor; - - // For an underscore type _ cursor, how tall it should be as a % of cell height - ULONG ulCursorHeightPercent; - - // For a vertical bar type | cursor, how many pixels wide it should be per ease of access preferences - ULONG cursorPixelWidth; - - // Whether to draw the cursor 2 cells wide (+X from the coordinate given) - bool fIsDoubleWidth; - - // Chooses a special cursor type like a full box, a vertical bar, etc. - CursorType cursorType; - - // Specifies to use the color below instead of the default color - bool fUseColor; - - // Color to use for drawing instead of the default - COLORREF cursorColor; - - // Is the cursor currently visually visible? - // If the cursor has blinked off, this is false. - // if the cursor has blinked on, this is true. - bool isOn; - }; - virtual ~IRenderEngine() = 0; protected: diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index 8aabf3df7ae..0d04bc9f0d9 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -352,7 +352,7 @@ CATCH_RETURN(); // - options - Packed options relevant to how to draw the cursor // Return Value: // - S_FALSE -[[nodiscard]] HRESULT UiaEngine::PaintCursor(const IRenderEngine::CursorOptions& /*options*/) noexcept +[[nodiscard]] HRESULT UiaEngine::PaintCursor(const CursorOptions& /*options*/) noexcept { return S_FALSE; } diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 6073b0358f6..20d872842f5 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -199,7 +199,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // - options - Options that affect the presentation of the cursor // Return Value: // - S_OK or suitable HRESULT error from writing pipe. -[[nodiscard]] HRESULT XtermEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +[[nodiscard]] HRESULT XtermEngine::PaintCursor(const CursorOptions& options) noexcept { // PaintCursor is only called when the cursor is in fact visible in a single // frame. When this is called, mark _nextCursorIsVisible as true. At the end diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index d9604f6856f..b4abab752bd 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -156,7 +156,7 @@ using namespace Microsoft::Console::Types; // - options - Options that affect the presentation of the cursor // Return Value: // - S_OK or suitable HRESULT error from writing pipe. -[[nodiscard]] HRESULT VtEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +[[nodiscard]] HRESULT VtEngine::PaintCursor(const CursorOptions& options) noexcept { _trace.TracePaintCursor(options.coordCursor); diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index 5819511c0a3..d2d1890581b 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -300,7 +300,7 @@ bool WddmConEngine::IsInitialized() return S_OK; } -[[nodiscard]] HRESULT WddmConEngine::PaintCursor(const IRenderEngine::CursorOptions& /*options*/) noexcept +[[nodiscard]] HRESULT WddmConEngine::PaintCursor(const CursorOptions& /*options*/) noexcept { return S_OK; } From f9aae9d5b139ea702c2c127db911d00971ba3900 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jun 2020 12:18:58 -0500 Subject: [PATCH 2/6] Wow, this works incredibly well for a ~first~ third attempt --- src/host/ut_host/VtRendererTests.cpp | 2 +- src/interactivity/onecore/BgfxEngine.cpp | 2 +- src/renderer/base/renderer.cpp | 53 +++++- src/renderer/base/renderer.hpp | 2 + src/renderer/dx/CustomTextRenderer.cpp | 104 ++++++++++++ src/renderer/dx/CustomTextRenderer.h | 14 +- src/renderer/dx/DxRenderer.cpp | 206 ++++++++++++----------- src/renderer/dx/DxRenderer.hpp | 4 + src/renderer/inc/IRenderEngine.hpp | 7 + 9 files changed, 288 insertions(+), 106 deletions(-) diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 7e5bfdd2104..38d9306ea93 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -1492,7 +1492,7 @@ void VtRendererTest::TestCursorVisibility() VERIFY_ARE_NOT_EQUAL(origin, engine->_lastText); - IRenderEngine::CursorOptions options{}; + CursorOptions options{}; options.coordCursor = origin; // Frame 1: Paint the cursor at the home position. At the end of the frame, diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index bfdcfa720f3..715ebd31f5d 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -179,7 +179,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid return S_OK; } -[[nodiscard]] HRESULT BgfxEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +[[nodiscard]] HRESULT BgfxEngine::PaintCursor(const CursorOptions& options) noexcept { // TODO: MSFT: 11448021 - Modify BGFX to support rendering full-width // characters and a full-width cursor. diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 5a817b28bf1..4d7a94733e9 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -139,6 +139,9 @@ try // B. Perform Scroll Operations RETURN_IF_FAILED(_PerformScrolling(pEngine)); + // C. + RETURN_IF_FAILED(_PrepareRenderInfo(pEngine)); + // 1. Paint Background RETURN_IF_FAILED(_PaintBackground(pEngine)); @@ -867,7 +870,7 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) bool useColor = cursorColor != INVALID_COLOR; // Build up the cursor parameters including position, color, and drawing options - IRenderEngine::CursorOptions options; + CursorOptions options; options.coordCursor = coordCursor; options.ulCursorHeightPercent = _pData->GetCursorHeight(); options.cursorPixelWidth = _pData->GetCursorPixelWidth(); @@ -883,6 +886,54 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) } } +// Routine Description: +// - TODO +// Arguments: +// - +// Return Value: +// - +[[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine) +{ + RenderFrameInfo info; + info.cursorInfo = std::nullopt; + + if (_pData->IsCursorVisible()) + { + // Get cursor position in buffer + COORD coordCursor = _pData->GetCursorPosition(); + + // GH#3166: Only draw the cursor if it's actually in the viewport. It + // might be on the line that's in that partially visible row at the + // bottom of the viewport, the space that's not quite a full line in + // height. Since we don't draw that text, we shouldn't draw the cursor + // there either. + Viewport view = _pData->GetViewport(); + if (view.IsInBounds(coordCursor)) + { + // Adjust cursor to viewport + view.ConvertToOrigin(&coordCursor); + + COLORREF cursorColor = _pData->GetCursorColor(); + bool useColor = cursorColor != INVALID_COLOR; + + // Build up the cursor parameters including position, color, and drawing options + CursorOptions options; + options.coordCursor = coordCursor; + options.ulCursorHeightPercent = _pData->GetCursorHeight(); + options.cursorPixelWidth = _pData->GetCursorPixelWidth(); + options.fIsDoubleWidth = _pData->IsCursorDoubleWidth(); + options.cursorType = _pData->GetCursorStyle(); + options.fUseColor = useColor; + options.cursorColor = cursorColor; + options.isOn = _pData->IsCursorOn(); + + info.cursorInfo = options; + } + } + + return pEngine->PrepareRenderInfo(info); +} + // Routine Description: // - Paint helper to draw text that overlays the main buffer to provide user interactivity regions // - This supports IME composition. diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index 74b4acff86f..92f5bbc56e2 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -127,6 +127,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine); + [[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine); + // Helper functions to diagnose issues with painting and layout. // These are only actually effective/on in Debug builds when the flag is set using an attached debugger. bool _fDebug = false; diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index b51b9561c45..097968b5378 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -5,6 +5,8 @@ #include "CustomTextRenderer.h" +#include "../../inc/DefaultSettings.h" + #include #include #include @@ -220,6 +222,106 @@ using namespace Microsoft::Console::Render; clientDrawingEffect); } +[[nodiscard]] HRESULT _DrawCursorHelper(::Microsoft::WRL::ComPtr d2dContext, + const DrawingContext& drawingContext) +try +{ + if (!drawingContext.cursorInfo.has_value()) + { + return S_FALSE; + } + + const auto& options = drawingContext.cursorInfo.value(); + const til::size glyphSize{ til::math::flooring, drawingContext.cellSize.width, drawingContext.cellSize.height }; + // const til::size glyphSize{ til::math::flooring, drawingContext.cellSize }; + // if the cursor is off, do nothing - it should not be visible. + if (!options.isOn) + { + return S_FALSE; + } + // Create rectangular block representing where the cursor can fill. + D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(glyphSize); + + // If we're double-width, make it one extra glyph wider + if (options.fIsDoubleWidth) + { + rect.right += glyphSize.width(); + } + + CursorPaintType paintType = CursorPaintType::Fill; + + switch (options.cursorType) + { + case CursorType::Legacy: + { + // Enforce min/max cursor height + ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, MinCursorHeightPercent, MaxCursorHeightPercent); + ulHeight = (glyphSize.height() * ulHeight) / 100; + rect.top = rect.bottom - ulHeight; + break; + } + case CursorType::VerticalBar: + { + // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. + // It's either the left + the proposed width from the ease of access setting, or + // it's the right edge of the block cursor as a maximum. + rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); + break; + } + case CursorType::Underscore: + { + rect.top = rect.bottom - 1; + break; + } + case CursorType::EmptyBox: + { + paintType = CursorPaintType::Outline; + break; + } + case CursorType::FullBox: + { + break; + } + default: + return E_NOTIMPL; + } + + Microsoft::WRL::ComPtr brush; // = _d2dBrushForeground; + + if (options.fUseColor) + { + // Make sure to make the cursor opaque + RETURN_IF_FAILED(d2dContext->CreateSolidColorBrush(til::color{ OPACITY_OPAQUE | options.cursorColor }, + &brush)); + } + + switch (paintType) + { + case CursorPaintType::Fill: + { + d2dContext->FillRectangle(rect, brush.Get()); + break; + } + case CursorPaintType::Outline: + { + // DrawRectangle in straddles physical pixels in an attempt to draw a line + // between them. To avoid this, bump the rectangle around by half the stroke width. + rect.top += 0.5f; + rect.left += 0.5f; + rect.bottom -= 0.5f; + rect.right -= 0.5f; + + d2dContext->DrawRectangle(rect, brush.Get()); + break; + } + default: + return E_NOTIMPL; + } + + return S_OK; +} +CATCH_RETURN() + // Routine Description: // - Implementation of IDWriteTextRenderer::DrawInlineObject // - Passes drawing control from the outer layout down into the context of an embedded object @@ -292,6 +394,8 @@ using namespace Microsoft::Console::Render; d2dContext->FillRectangle(rect, drawingContext->backgroundBrush); + RETURN_IF_FAILED(_DrawCursorHelper(d2dContext, *drawingContext)); + // GH#5098: If we're rendering with cleartype text, we need to always render // onto an opaque background. If our background _isn't_ opaque, then we need // to use grayscale AA for this run of text. diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index f97e48f58a6..6ade578ca09 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -18,7 +18,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory, const DWRITE_LINE_SPACING spacing, const D2D_SIZE_F cellSize, - const CursorOptions cursorInfo, + const std::optional cursorInfo, const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept { this->renderTarget = renderTarget; @@ -39,10 +39,20 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory; DWRITE_LINE_SPACING spacing; D2D_SIZE_F cellSize; - CursorOptions cursorInfo; + std::optional cursorInfo; D2D1_DRAW_TEXT_OPTIONS options; }; + // Helper to choose which Direct2D method to use when drawing the cursor rectangle + enum class CursorPaintType + { + Fill, + Outline + }; + + constexpr const ULONG MinCursorHeightPercent = 25; + constexpr const ULONG MaxCursorHeightPercent = 100; + class CustomTextRenderer : public ::Microsoft::WRL::RuntimeClass<::Microsoft::WRL::RuntimeClassFlags<::Microsoft::WRL::ClassicCom | ::Microsoft::WRL::InhibitFtmBase>, IDWriteTextRenderer> { public: diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 9dc45751d49..ea2ca2c109f 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1298,7 +1298,7 @@ try _dwriteFactory.Get(), spacing, _glyphCell, - {}, + _frameInfo.cursorInfo, D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); // Layout then render the text @@ -1414,109 +1414,107 @@ try } CATCH_RETURN() -// Helper to choose which Direct2D method to use when drawing the cursor rectangle -enum class CursorPaintType +[[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& /*options*/) noexcept { - Fill, - Outline -}; - -// Routine Description: -// - Draws a block at the given position to represent the cursor -// - May be a styled cursor at the character cell location that is less than a full block -// Arguments: -// - options - Packed options relevant to how to draw the cursor -// Return Value: -// - S_OK or relevant DirectX error. -[[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& options) noexcept -try -{ - // if the cursor is off, do nothing - it should not be visible. - if (!options.isOn) - { - return S_FALSE; - } - // Create rectangular block representing where the cursor can fill. - D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(_glyphCell); - - // If we're double-width, make it one extra glyph wider - if (options.fIsDoubleWidth) - { - rect.right += _glyphCell.width(); - } - - CursorPaintType paintType = CursorPaintType::Fill; - - switch (options.cursorType) - { - case CursorType::Legacy: - { - // Enforce min/max cursor height - ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent); - ulHeight = (_glyphCell.height() * ulHeight) / 100; - rect.top = rect.bottom - ulHeight; - break; - } - case CursorType::VerticalBar: - { - // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. - // It's either the left + the proposed width from the ease of access setting, or - // it's the right edge of the block cursor as a maximum. - rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); - break; - } - case CursorType::Underscore: - { - rect.top = rect.bottom - 1; - break; - } - case CursorType::EmptyBox: - { - paintType = CursorPaintType::Outline; - break; - } - case CursorType::FullBox: - { - break; - } - default: - return E_NOTIMPL; - } - - Microsoft::WRL::ComPtr brush = _d2dBrushForeground; - - if (options.fUseColor) - { - // Make sure to make the cursor opaque - RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(_ColorFFromColorRef(OPACITY_OPAQUE | options.cursorColor), &brush)); - } - - switch (paintType) - { - case CursorPaintType::Fill: - { - _d2dRenderTarget->FillRectangle(rect, brush.Get()); - break; - } - case CursorPaintType::Outline: - { - // DrawRectangle in straddles physical pixels in an attempt to draw a line - // between them. To avoid this, bump the rectangle around by half the stroke width. - rect.top += 0.5f; - rect.left += 0.5f; - rect.bottom -= 0.5f; - rect.right -= 0.5f; - - _d2dRenderTarget->DrawRectangle(rect, brush.Get()); - break; - } - default: - return E_NOTIMPL; - } - return S_OK; } -CATCH_RETURN() + +// // Routine Description: +// // - Draws a block at the given position to represent the cursor +// // - May be a styled cursor at the character cell location that is less than a full block +// // Arguments: +// // - options - Packed options relevant to how to draw the cursor +// // Return Value: +// // - S_OK or relevant DirectX error. +// [[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& options) noexcept +// try +// { +// // if the cursor is off, do nothing - it should not be visible. +// if (!options.isOn) +// { +// return S_FALSE; +// } +// // Create rectangular block representing where the cursor can fill. +// D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(_glyphCell); + +// // If we're double-width, make it one extra glyph wider +// if (options.fIsDoubleWidth) +// { +// rect.right += _glyphCell.width(); +// } + +// CursorPaintType paintType = CursorPaintType::Fill; + +// switch (options.cursorType) +// { +// case CursorType::Legacy: +// { +// // Enforce min/max cursor height +// ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent); +// ulHeight = (_glyphCell.height() * ulHeight) / 100; +// rect.top = rect.bottom - ulHeight; +// break; +// } +// case CursorType::VerticalBar: +// { +// // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. +// // It's either the left + the proposed width from the ease of access setting, or +// // it's the right edge of the block cursor as a maximum. +// rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); +// break; +// } +// case CursorType::Underscore: +// { +// rect.top = rect.bottom - 1; +// break; +// } +// case CursorType::EmptyBox: +// { +// paintType = CursorPaintType::Outline; +// break; +// } +// case CursorType::FullBox: +// { +// break; +// } +// default: +// return E_NOTIMPL; +// } + +// Microsoft::WRL::ComPtr brush = _d2dBrushForeground; + +// if (options.fUseColor) +// { +// // Make sure to make the cursor opaque +// RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(_ColorFFromColorRef(OPACITY_OPAQUE | options.cursorColor), &brush)); +// } + +// switch (paintType) +// { +// case CursorPaintType::Fill: +// { +// _d2dRenderTarget->FillRectangle(rect, brush.Get()); +// break; +// } +// case CursorPaintType::Outline: +// { +// // DrawRectangle in straddles physical pixels in an attempt to draw a line +// // between them. To avoid this, bump the rectangle around by half the stroke width. +// rect.top += 0.5f; +// rect.left += 0.5f; +// rect.bottom -= 0.5f; +// rect.right -= 0.5f; + +// _d2dRenderTarget->DrawRectangle(rect, brush.Get()); +// break; +// } +// default: +// return E_NOTIMPL; +// } + +// return S_OK; +// } +// CATCH_RETURN() // Routine Description: // - Paint terminal effects. @@ -2247,3 +2245,9 @@ try LOG_IF_FAILED(InvalidateAll()); } CATCH_LOG() + +[[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept +{ + _frameInfo = info; + return S_OK; +} diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 95d2d1b02f4..0fa88f6df2f 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -77,6 +77,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT ScrollFrame() noexcept override; + [[nodiscard]] HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept override; + [[nodiscard]] HRESULT PaintBackground() noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, COORD const coord, @@ -202,6 +204,8 @@ 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 { diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 52827d3f048..2a0aef24a62 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -20,6 +20,11 @@ Author(s): namespace Microsoft::Console::Render { + struct RenderFrameInfo + { + std::optional cursorInfo; + }; + class IRenderEngine { public: @@ -60,6 +65,8 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT InvalidateTitle(const std::wstring& proposedTitle) noexcept = 0; + [[nodiscard]] virtual HRESULT PrepareRenderInfo(const RenderFrameInfo& /*info*/) noexcept { return S_FALSE; }; + [[nodiscard]] virtual HRESULT PaintBackground() noexcept = 0; [[nodiscard]] virtual HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, From 9d30b3b75b0c604b7a6d0b286f2506738134cde2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jun 2020 14:43:01 -0500 Subject: [PATCH 3/6] Code cleanup for review --- src/renderer/base/renderer.cpp | 73 +++++++--------- src/renderer/base/renderer.hpp | 1 + src/renderer/dx/CustomTextRenderer.cpp | 53 ++++++++++-- src/renderer/dx/DxRenderer.cpp | 115 ++++--------------------- src/renderer/dx/DxRenderer.hpp | 3 - 5 files changed, 97 insertions(+), 148 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 4d7a94733e9..b2ed720ccc6 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -843,12 +843,16 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin } // Routine Description: -// - Paint helper to draw the cursor within the buffer. +// - Retrieves information about the cursor, and pack it into a CursorOptions +// that the render engine can use for painting the cursor. +// - If the cursor is "off", or the cursor is out of bounds of the viewport, +// this will return nullopt (indicating the cursor shoudln't be painted this +// frame) // Arguments: // - // Return Value: -// - -void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) +// - nullopt if the cursor is off our out-of-frame, otherwise a CursorOptions +[[nodiscard]] std::optional Renderer::_GetCursorInfo() { if (_pData->IsCursorVisible()) { @@ -880,57 +884,42 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) options.cursorColor = cursorColor; options.isOn = _pData->IsCursorOn(); - // Draw it within the viewport - LOG_IF_FAILED(pEngine->PaintCursor(options)); + return { options }; } } + return std::nullopt; } // Routine Description: -// - TODO +// - Paint helper to draw the cursor within the buffer. // Arguments: -// - +// - engine - The render engine that we're targeting. // Return Value: // - -[[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine) +void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine) { - RenderFrameInfo info; - info.cursorInfo = std::nullopt; - - if (_pData->IsCursorVisible()) + const auto cursorInfo = _GetCursorInfo(); + if (cursorInfo.has_value()) { - // Get cursor position in buffer - COORD coordCursor = _pData->GetCursorPosition(); - - // GH#3166: Only draw the cursor if it's actually in the viewport. It - // might be on the line that's in that partially visible row at the - // bottom of the viewport, the space that's not quite a full line in - // height. Since we don't draw that text, we shouldn't draw the cursor - // there either. - Viewport view = _pData->GetViewport(); - if (view.IsInBounds(coordCursor)) - { - // Adjust cursor to viewport - view.ConvertToOrigin(&coordCursor); - - COLORREF cursorColor = _pData->GetCursorColor(); - bool useColor = cursorColor != INVALID_COLOR; - - // Build up the cursor parameters including position, color, and drawing options - CursorOptions options; - options.coordCursor = coordCursor; - options.ulCursorHeightPercent = _pData->GetCursorHeight(); - options.cursorPixelWidth = _pData->GetCursorPixelWidth(); - options.fIsDoubleWidth = _pData->IsCursorDoubleWidth(); - options.cursorType = _pData->GetCursorStyle(); - options.fUseColor = useColor; - options.cursorColor = cursorColor; - options.isOn = _pData->IsCursorOn(); - - info.cursorInfo = options; - } + LOG_IF_FAILED(pEngine->PaintCursor(cursorInfo.value())); } +} +// Routine Description: +// - Retrieves info from the render data to prepare the engine with, before the +// frame is drawn. Some renderers might want to use this information to affect +// later drawing decisions. +// * Namely, the DX renderer uses this to know the cursor position and state +// before PaintCursor is called, so it can draw the cursor underneath the +// text. +// Arguments: +// - engine - The render engine that we're targeting. +// Return Value: +// - S_OK if the engine prepared successfully, or a relevant error via HRESULT. +[[nodiscard]] HRESULT Renderer::_PrepareRenderInfo(_In_ IRenderEngine* const pEngine) +{ + RenderFrameInfo info; + info.cursorInfo = _GetCursorInfo(); return pEngine->PrepareRenderInfo(info); } diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index 92f5bbc56e2..37a72c56925 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -127,6 +127,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine); + [[nodiscard]] std::optional _GetCursorInfo(); [[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine); // Helper functions to diagnose issues with painting and layout. diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index 097968b5378..ffe9bf1ebc6 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -222,8 +222,26 @@ using namespace Microsoft::Console::Render; clientDrawingEffect); } -[[nodiscard]] HRESULT _DrawCursorHelper(::Microsoft::WRL::ComPtr d2dContext, - const DrawingContext& drawingContext) +// Function Description: +// - Attempt to draw the cursor. If the cursor isn't visible or on, this +// function will do nothing. If the cursor isn't within the bounds of the +// current run of text, then this function will do nothing. +// - This function will get called twice during a run, once before the text is +// drawn (underneath the text), and again after the text is drawn (above the +// text). Depending on if the cursor wants to be drawn above or below the +// text, this function will do nothing for the first/second pass +// (respectively). +// Arguments: +// - d2dContext - Pointer to the current D2D drawing context +// - textRunBounds - The bounds of the current run of text. +// - drawingContext - Pointer to structure of information required to draw +// - firstPass - true if we're being called before the text is drawn, false afterwards. +// Return Value: +// - S_FALSE if we did nothing, S_OK if we successfully painted, otherwise an appropriate HRESULT +[[nodiscard]] HRESULT _drawCursorHelper(::Microsoft::WRL::ComPtr d2dContext, + D2D1_RECT_F textRunBounds, + const DrawingContext& drawingContext, + const bool firstPass) try { if (!drawingContext.cursorInfo.has_value()) @@ -232,13 +250,24 @@ try } const auto& options = drawingContext.cursorInfo.value(); - const til::size glyphSize{ til::math::flooring, drawingContext.cellSize.width, drawingContext.cellSize.height }; - // const til::size glyphSize{ til::math::flooring, drawingContext.cellSize }; + // if the cursor is off, do nothing - it should not be visible. if (!options.isOn) { return S_FALSE; } + + // TODO GH#TODO: Add support for `"cursorTextColor": null` for letting the + // cursor draw on top again. + if (!firstPass) + { + return S_FALSE; + } + + const til::size glyphSize{ til::math::flooring, + drawingContext.cellSize.width, + drawingContext.cellSize.height }; + // Create rectangular block representing where the cursor can fill. D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(glyphSize); @@ -248,6 +277,15 @@ try rect.right += glyphSize.width(); } + // If the cursor isn't within the bounds of this current run of text, do nothing. + if (rect.top > textRunBounds.bottom || + rect.bottom <= textRunBounds.top || + rect.left > textRunBounds.right || + rect.right <= textRunBounds.left) + { + return S_FALSE; + } + CursorPaintType paintType = CursorPaintType::Fill; switch (options.cursorType) @@ -286,7 +324,7 @@ try return E_NOTIMPL; } - Microsoft::WRL::ComPtr brush; // = _d2dBrushForeground; + Microsoft::WRL::ComPtr brush; if (options.fUseColor) { @@ -394,7 +432,7 @@ CATCH_RETURN() d2dContext->FillRectangle(rect, drawingContext->backgroundBrush); - RETURN_IF_FAILED(_DrawCursorHelper(d2dContext, *drawingContext)); + RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, true)); // GH#5098: If we're rendering with cleartype text, we need to always render // onto an opaque background. If our background _isn't_ opaque, then we need @@ -583,6 +621,9 @@ CATCH_RETURN() drawingContext->foregroundBrush, clientDrawingEffect)); } + + RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, false)); + return S_OK; } #pragma endregion diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index ea2ca2c109f..5e95cbe31a7 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1414,108 +1414,18 @@ try } CATCH_RETURN() +// Routine Description: +// - Does nothing. Our cursor is drawn in CustomTextRenderer::DrawGlyphRun, +// either above or below the text. +// Arguments: +// - options - unused +// Return Value: +// - S_OK [[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& /*options*/) noexcept { return S_OK; } -// // Routine Description: -// // - Draws a block at the given position to represent the cursor -// // - May be a styled cursor at the character cell location that is less than a full block -// // Arguments: -// // - options - Packed options relevant to how to draw the cursor -// // Return Value: -// // - S_OK or relevant DirectX error. -// [[nodiscard]] HRESULT DxEngine::PaintCursor(const CursorOptions& options) noexcept -// try -// { -// // if the cursor is off, do nothing - it should not be visible. -// if (!options.isOn) -// { -// return S_FALSE; -// } -// // Create rectangular block representing where the cursor can fill. -// D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(_glyphCell); - -// // If we're double-width, make it one extra glyph wider -// if (options.fIsDoubleWidth) -// { -// rect.right += _glyphCell.width(); -// } - -// CursorPaintType paintType = CursorPaintType::Fill; - -// switch (options.cursorType) -// { -// case CursorType::Legacy: -// { -// // Enforce min/max cursor height -// ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent); -// ulHeight = (_glyphCell.height() * ulHeight) / 100; -// rect.top = rect.bottom - ulHeight; -// break; -// } -// case CursorType::VerticalBar: -// { -// // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. -// // It's either the left + the proposed width from the ease of access setting, or -// // it's the right edge of the block cursor as a maximum. -// rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); -// break; -// } -// case CursorType::Underscore: -// { -// rect.top = rect.bottom - 1; -// break; -// } -// case CursorType::EmptyBox: -// { -// paintType = CursorPaintType::Outline; -// break; -// } -// case CursorType::FullBox: -// { -// break; -// } -// default: -// return E_NOTIMPL; -// } - -// Microsoft::WRL::ComPtr brush = _d2dBrushForeground; - -// if (options.fUseColor) -// { -// // Make sure to make the cursor opaque -// RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(_ColorFFromColorRef(OPACITY_OPAQUE | options.cursorColor), &brush)); -// } - -// switch (paintType) -// { -// case CursorPaintType::Fill: -// { -// _d2dRenderTarget->FillRectangle(rect, brush.Get()); -// break; -// } -// case CursorPaintType::Outline: -// { -// // DrawRectangle in straddles physical pixels in an attempt to draw a line -// // between them. To avoid this, bump the rectangle around by half the stroke width. -// rect.top += 0.5f; -// rect.left += 0.5f; -// rect.bottom -= 0.5f; -// rect.right -= 0.5f; - -// _d2dRenderTarget->DrawRectangle(rect, brush.Get()); -// break; -// } -// default: -// return E_NOTIMPL; -// } - -// return S_OK; -// } -// CATCH_RETURN() - // Routine Description: // - Paint terminal effects. // Arguments: @@ -2246,6 +2156,17 @@ try } CATCH_LOG() +// Method Description: +// - Informs this render engine about certain state for this frame at the +// beginning of this frame. We'll use it to get information about the cursor +// before PaintCursor is called. This enables the DX renderer to draw the +// cursor underneath the text. +// - This is called every frame. When the cursor is Off or out of frame, the +// info's cursorInfo will be set to std::nullopt; +// Arguments: +// - info - a RenderFrameInfo with information about the state of the cursor in this frame. +// Return Value: +// - S_OK [[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept { _frameInfo = info; diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 0fa88f6df2f..06e869b4d00 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -162,9 +162,6 @@ namespace Microsoft::Console::Render static std::atomic _tracelogCount; - static const ULONG s_ulMinCursorHeightPercent = 25; - static const ULONG s_ulMaxCursorHeightPercent = 100; - // Device-Independent Resources ::Microsoft::WRL::ComPtr _d2dFactory; ::Microsoft::WRL::ComPtr _dwriteFactory; From 9da246160940f15bc4352e64c32f8226f57b92c6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jun 2020 15:04:05 -0500 Subject: [PATCH 4/6] These were some minor missing comments, typos --- src/renderer/base/renderer.cpp | 4 ++-- src/renderer/dx/CustomTextRenderer.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index b2ed720ccc6..00c58a18228 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -139,7 +139,7 @@ try // B. Perform Scroll Operations RETURN_IF_FAILED(_PerformScrolling(pEngine)); - // C. + // C. Prepare the engine with additional information before we start drawing. RETURN_IF_FAILED(_PrepareRenderInfo(pEngine)); // 1. Paint Background @@ -846,7 +846,7 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin // - Retrieves information about the cursor, and pack it into a CursorOptions // that the render engine can use for painting the cursor. // - If the cursor is "off", or the cursor is out of bounds of the viewport, -// this will return nullopt (indicating the cursor shoudln't be painted this +// this will return nullopt (indicating the cursor shouldn't be painted this // frame) // Arguments: // - diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index ffe9bf1ebc6..81eba0b1021 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -257,7 +257,7 @@ try return S_FALSE; } - // TODO GH#TODO: Add support for `"cursorTextColor": null` for letting the + // TODO GH#6338: Add support for `"cursorTextColor": null` for letting the // cursor draw on top again. if (!firstPass) { From 2c14d58cc0bd6106e7358147d5707edae6367959 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jun 2020 16:37:27 -0500 Subject: [PATCH 5/6] PR feedback - only draw the filledBox cursor beneath the character --- src/renderer/base/RenderEngineBase.cpp | 5 +++++ src/renderer/base/renderer.cpp | 6 +++--- src/renderer/dx/CustomTextRenderer.cpp | 21 ++++++++++++++------- src/renderer/dx/CustomTextRenderer.h | 2 +- src/renderer/inc/IRenderEngine.hpp | 2 +- src/renderer/inc/RenderEngineBase.hpp | 2 ++ 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/renderer/base/RenderEngineBase.cpp b/src/renderer/base/RenderEngineBase.cpp index 88d32710e18..f6baafd4cc8 100644 --- a/src/renderer/base/RenderEngineBase.cpp +++ b/src/renderer/base/RenderEngineBase.cpp @@ -35,3 +35,8 @@ HRESULT RenderEngineBase::UpdateTitle(const std::wstring& newTitle) noexcept } return hr; } + +HRESULT RenderEngineBase::PrepareRenderInfo(const RenderFrameInfo& /*info*/) noexcept +{ + return S_FALSE; +} diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 00c58a18228..fc9eeb463da 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -843,15 +843,15 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin } // Routine Description: -// - Retrieves information about the cursor, and pack it into a CursorOptions -// that the render engine can use for painting the cursor. +// - Retrieve information about the cursor, and pack it into a CursorOptions +// which the render engine can use for painting the cursor. // - If the cursor is "off", or the cursor is out of bounds of the viewport, // this will return nullopt (indicating the cursor shouldn't be painted this // frame) // Arguments: // - // Return Value: -// - nullopt if the cursor is off our out-of-frame, otherwise a CursorOptions +// - nullopt if the cursor is off or out-of-frame, otherwise a CursorOptions [[nodiscard]] std::optional Renderer::_GetCursorInfo() { if (_pData->IsCursorVisible()) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index 81eba0b1021..fde61268122 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -238,10 +238,10 @@ using namespace Microsoft::Console::Render; // - firstPass - true if we're being called before the text is drawn, false afterwards. // Return Value: // - S_FALSE if we did nothing, S_OK if we successfully painted, otherwise an appropriate HRESULT -[[nodiscard]] HRESULT _drawCursorHelper(::Microsoft::WRL::ComPtr d2dContext, - D2D1_RECT_F textRunBounds, - const DrawingContext& drawingContext, - const bool firstPass) +[[nodiscard]] HRESULT _drawCursor(ID2D1DeviceContext* d2dContext, + D2D1_RECT_F textRunBounds, + const DrawingContext& drawingContext, + const bool firstPass) try { if (!drawingContext.cursorInfo.has_value()) @@ -259,7 +259,14 @@ try // TODO GH#6338: Add support for `"cursorTextColor": null` for letting the // cursor draw on top again. - if (!firstPass) + + // Only draw the filled box in the first pass. All the other cursors should + // be drawn in the second pass. + // | type==FullBox | + // firstPass | T | F | + // T | draw | skip | + // F | skip | draw | + if ((options.cursorType == CursorType::FullBox) != firstPass) { return S_FALSE; } @@ -432,7 +439,7 @@ CATCH_RETURN() d2dContext->FillRectangle(rect, drawingContext->backgroundBrush); - RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, true)); + RETURN_IF_FAILED(_drawCursor(d2dContext.Get(), rect, *drawingContext, true)); // GH#5098: If we're rendering with cleartype text, we need to always render // onto an opaque background. If our background _isn't_ opaque, then we need @@ -622,7 +629,7 @@ CATCH_RETURN() clientDrawingEffect)); } - RETURN_IF_FAILED(_drawCursorHelper(d2dContext, rect, *drawingContext, false)); + RETURN_IF_FAILED(_drawCursor(d2dContext.Get(), rect, *drawingContext, false)); return S_OK; } diff --git a/src/renderer/dx/CustomTextRenderer.h b/src/renderer/dx/CustomTextRenderer.h index 6ade578ca09..2538c14974d 100644 --- a/src/renderer/dx/CustomTextRenderer.h +++ b/src/renderer/dx/CustomTextRenderer.h @@ -18,7 +18,7 @@ namespace Microsoft::Console::Render IDWriteFactory* dwriteFactory, const DWRITE_LINE_SPACING spacing, const D2D_SIZE_F cellSize, - const std::optional cursorInfo, + const std::optional& cursorInfo, const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept { this->renderTarget = renderTarget; diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 2a0aef24a62..ad9b6e49559 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -65,7 +65,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT InvalidateTitle(const std::wstring& proposedTitle) noexcept = 0; - [[nodiscard]] virtual HRESULT PrepareRenderInfo(const RenderFrameInfo& /*info*/) noexcept { return S_FALSE; }; + [[nodiscard]] virtual HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept = 0; [[nodiscard]] virtual HRESULT PaintBackground() noexcept = 0; [[nodiscard]] virtual HRESULT PaintBufferLine(std::basic_string_view const clusters, diff --git a/src/renderer/inc/RenderEngineBase.hpp b/src/renderer/inc/RenderEngineBase.hpp index eae934bc775..4d6298a1d97 100644 --- a/src/renderer/inc/RenderEngineBase.hpp +++ b/src/renderer/inc/RenderEngineBase.hpp @@ -38,6 +38,8 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateTitle(const std::wstring& newTitle) noexcept override; + [[nodiscard]] HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept override; + protected: [[nodiscard]] virtual HRESULT _DoUpdateTitle(const std::wstring& newTitle) noexcept = 0; From c278540544826cc77779e7da1dc4bac223b8f3c7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 4 Jun 2020 07:20:01 -0500 Subject: [PATCH 6/6] Fix audit mode --- src/renderer/dx/CustomTextRenderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/dx/CustomTextRenderer.cpp b/src/renderer/dx/CustomTextRenderer.cpp index fde61268122..32231bc8afc 100644 --- a/src/renderer/dx/CustomTextRenderer.cpp +++ b/src/renderer/dx/CustomTextRenderer.cpp @@ -238,7 +238,7 @@ using namespace Microsoft::Console::Render; // - firstPass - true if we're being called before the text is drawn, false afterwards. // Return Value: // - S_FALSE if we did nothing, S_OK if we successfully painted, otherwise an appropriate HRESULT -[[nodiscard]] HRESULT _drawCursor(ID2D1DeviceContext* d2dContext, +[[nodiscard]] HRESULT _drawCursor(gsl::not_null d2dContext, D2D1_RECT_F textRunBounds, const DrawingContext& drawingContext, const bool firstPass)