From dacff61f8862fa7c28f0244e74555cb2658455ad Mon Sep 17 00:00:00 2001 From: James Holderness Date: Wed, 29 Sep 2021 11:48:32 +0100 Subject: [PATCH] Use the til::enumset type for the GridLines enum in the renderers (#11345) ## Summary of the Pull Request This replaces the `GridLines` enum in the renderers with a `til::enumset` type, avoiding the need for the various `WI_IsFlagSet` macros and flag operators. ## References This is followup to PR #10492 which introduced the `enumset` class. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. ## Validation Steps Performed I've manually confirmed that all the different gridlines are still rendering correctly in both the GDI and DX renderers. --- src/interactivity/onecore/BgfxEngine.cpp | 2 +- src/interactivity/onecore/BgfxEngine.hpp | 2 +- src/renderer/base/renderer.cpp | 28 ++++++++++++------------ src/renderer/base/renderer.hpp | 2 +- src/renderer/dx/DxRenderer.cpp | 24 ++++++++++---------- src/renderer/dx/DxRenderer.hpp | 2 +- src/renderer/gdi/gdirenderer.hpp | 2 +- src/renderer/gdi/paint.cpp | 16 +++++++------- src/renderer/inc/IRenderEngine.hpp | 25 ++++++++++----------- src/renderer/uia/UiaRenderer.cpp | 2 +- src/renderer/uia/UiaRenderer.hpp | 2 +- src/renderer/vt/paint.cpp | 2 +- src/renderer/vt/vtrenderer.hpp | 2 +- src/renderer/wddmcon/WddmConRenderer.cpp | 2 +- src/renderer/wddmcon/WddmConRenderer.hpp | 2 +- 15 files changed, 57 insertions(+), 58 deletions(-) diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index dcd3f923e..ef9e95112 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -166,7 +166,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid CATCH_RETURN(); } -[[nodiscard]] HRESULT BgfxEngine::PaintBufferGridLines(GridLines const /*lines*/, +[[nodiscard]] HRESULT BgfxEngine::PaintBufferGridLines(GridLineSet const /*lines*/, COLORREF const /*color*/, size_t const /*cchLine*/, COORD const /*coordTarget*/) noexcept diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 2b4e787a3..4a01837d9 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -53,7 +53,7 @@ namespace Microsoft::Console::Render const COORD coord, const bool trimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; + [[nodiscard]] HRESULT PaintBufferGridLines(GridLineSet const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override; diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 9654df2aa..13a72d4ba 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -898,50 +898,50 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, // Arguments: // - textAttribute: the TextAttribute to generate GridLines from. // Return Value: -// - a GridLines containing all the gridline info from the TextAttribute -IRenderEngine::GridLines Renderer::s_GetGridlines(const TextAttribute& textAttribute) noexcept +// - a GridLineSet containing all the gridline info from the TextAttribute +IRenderEngine::GridLineSet Renderer::s_GetGridlines(const TextAttribute& textAttribute) noexcept { // Convert console grid line representations into rendering engine enum representations. - IRenderEngine::GridLines lines = IRenderEngine::GridLines::None; + IRenderEngine::GridLineSet lines; if (textAttribute.IsTopHorizontalDisplayed()) { - lines |= IRenderEngine::GridLines::Top; + lines.set(IRenderEngine::GridLines::Top); } if (textAttribute.IsBottomHorizontalDisplayed()) { - lines |= IRenderEngine::GridLines::Bottom; + lines.set(IRenderEngine::GridLines::Bottom); } if (textAttribute.IsLeftVerticalDisplayed()) { - lines |= IRenderEngine::GridLines::Left; + lines.set(IRenderEngine::GridLines::Left); } if (textAttribute.IsRightVerticalDisplayed()) { - lines |= IRenderEngine::GridLines::Right; + lines.set(IRenderEngine::GridLines::Right); } if (textAttribute.IsCrossedOut()) { - lines |= IRenderEngine::GridLines::Strikethrough; + lines.set(IRenderEngine::GridLines::Strikethrough); } if (textAttribute.IsUnderlined()) { - lines |= IRenderEngine::GridLines::Underline; + lines.set(IRenderEngine::GridLines::Underline); } if (textAttribute.IsDoublyUnderlined()) { - lines |= IRenderEngine::GridLines::DoubleUnderline; + lines.set(IRenderEngine::GridLines::DoubleUnderline); } if (textAttribute.IsHyperlink()) { - lines |= IRenderEngine::GridLines::HyperlinkUnderline; + lines.set(IRenderEngine::GridLines::HyperlinkUnderline); } return lines; } @@ -962,7 +962,7 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin const COORD coordTarget) { // Convert console grid line representations into rendering engine enum representations. - IRenderEngine::GridLines lines = Renderer::s_GetGridlines(textAttribute); + auto lines = Renderer::s_GetGridlines(textAttribute); // For now, we dash underline patterns and switch to regular underline on hover // Since we're only rendering pattern links on *hover*, there's no point in checking @@ -975,13 +975,13 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin { if (_pData->GetPatternId(coordTarget).size() > 0) { - lines |= IRenderEngine::GridLines::Underline; + lines.set(IRenderEngine::GridLines::Underline); } } } // Return early if there are no lines to paint. - if (lines != IRenderEngine::GridLines::None) + if (lines.any()) { // Get the current foreground color to render the lines. const COLORREF rgb = _pData->GetAttributeColors(textAttribute).first; diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index 10d853d85..1c1fb01ad 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -87,7 +87,7 @@ namespace Microsoft::Console::Render void UpdateLastHoveredInterval(const std::optional::interval>& newInterval); private: - static IRenderEngine::GridLines s_GetGridlines(const TextAttribute& textAttribute) noexcept; + static IRenderEngine::GridLineSet s_GetGridlines(const TextAttribute& textAttribute) noexcept; static bool s_IsSoftFontChar(const std::wstring_view& v, const size_t firstSoftFontChar, const size_t lastSoftFontChar); void _NotifyPaintFrame(); diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index fc8111a63..bbbdb8b20 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1706,7 +1706,7 @@ CATCH_RETURN() // - We will draw rightward (+X) from here // Return Value: // - S_OK or relevant DirectX error -[[nodiscard]] HRESULT DxEngine::PaintBufferGridLines(GridLines const lines, +[[nodiscard]] HRESULT DxEngine::PaintBufferGridLines(GridLineSet const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept @@ -1733,13 +1733,13 @@ try // 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. const DxFontRenderData::LineMetrics lineMetrics = _fontRenderData->GetLineMetrics(); - if (WI_IsAnyFlagSet(lines, (GridLines::Left | GridLines::Right))) + if (lines.any(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 (WI_IsFlagSet(lines, GridLines::Left)) + if (lines.test(GridLines::Left)) { auto x = target.x + halfGridlineWidth; for (size_t i = 0; i < cchLine; i++, x += font.width) @@ -1748,7 +1748,7 @@ try } } - if (WI_IsFlagSet(lines, GridLines::Right)) + if (lines.test(GridLines::Right)) { auto x = target.x + font.width - halfGridlineWidth; for (size_t i = 0; i < cchLine; i++, x += font.width) @@ -1758,19 +1758,19 @@ try } } - if (WI_IsAnyFlagSet(lines, GridLines::Top | GridLines::Bottom)) + if (lines.any(GridLines::Top, GridLines::Bottom)) { const auto halfGridlineWidth = lineMetrics.gridlineWidth / 2.0f; const auto startX = target.x + halfGridlineWidth; const auto endX = target.x + fullRunWidth - halfGridlineWidth; - if (WI_IsFlagSet(lines, GridLines::Top)) + if (lines.test(GridLines::Top)) { const auto y = target.y + halfGridlineWidth; DrawLine(startX, y, endX, y, lineMetrics.gridlineWidth); } - if (WI_IsFlagSet(lines, GridLines::Bottom)) + if (lines.test(GridLines::Bottom)) { const auto y = target.y + font.height - halfGridlineWidth; DrawLine(startX, y, endX, y, lineMetrics.gridlineWidth); @@ -1780,24 +1780,24 @@ try // In the case of the underline and strikethrough offsets, the stroke width // is already accounted for, so they don't require further adjustments. - if (WI_IsAnyFlagSet(lines, GridLines::Underline | GridLines::DoubleUnderline | GridLines::HyperlinkUnderline)) + if (lines.any(GridLines::Underline, GridLines::DoubleUnderline, GridLines::HyperlinkUnderline)) { 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; - if (WI_IsFlagSet(lines, GridLines::Underline)) + if (lines.test(GridLines::Underline)) { DrawLine(startX, y, endX, y, lineMetrics.underlineWidth); } - if (WI_IsFlagSet(lines, GridLines::HyperlinkUnderline)) + if (lines.test(GridLines::HyperlinkUnderline)) { DrawHyperlinkLine(startX, y, endX, y, lineMetrics.underlineWidth); } - if (WI_IsFlagSet(lines, GridLines::DoubleUnderline)) + if (lines.test(GridLines::DoubleUnderline)) { DrawLine(startX, y, endX, y, lineMetrics.underlineWidth); const auto y2 = target.y + lineMetrics.underlineOffset2; @@ -1805,7 +1805,7 @@ try } } - if (WI_IsFlagSet(lines, GridLines::Strikethrough)) + if (lines.test(GridLines::Strikethrough)) { const auto halfStrikethroughWidth = lineMetrics.strikethroughWidth / 2.0f; const auto startX = target.x + halfStrikethroughWidth; diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 9c96e7f06..de9c69fd0 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -100,7 +100,7 @@ namespace Microsoft::Console::Render bool const fTrimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; + [[nodiscard]] HRESULT PaintBufferGridLines(GridLineSet const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 4caf62ad6..2e09f374d 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -52,7 +52,7 @@ namespace Microsoft::Console::Render const COORD coord, const bool trimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(const GridLines lines, + [[nodiscard]] HRESULT PaintBufferGridLines(const GridLineSet lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept override; diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp index 347135974..598ce3489 100644 --- a/src/renderer/gdi/paint.cpp +++ b/src/renderer/gdi/paint.cpp @@ -472,7 +472,7 @@ using namespace Microsoft::Console::Render; // - coordTarget - The starting X/Y position of the first character to draw on. // Return Value: // - S_OK or suitable GDI HRESULT error or E_FAIL for GDI errors in functions that don't reliably return a specific error code. -[[nodiscard]] HRESULT GdiEngine::PaintBufferGridLines(const GridLines lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept +[[nodiscard]] HRESULT GdiEngine::PaintBufferGridLines(const GridLineSet lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept { LOG_IF_FAILED(_FlushBufferLines()); @@ -499,7 +499,7 @@ using namespace Microsoft::Console::Render; return PatBlt(_hdcMemoryContext, x, y, w, h, PATCOPY); }; - if (lines & GridLines::Left) + if (lines.test(GridLines::Left)) { auto x = ptTarget.x; for (size_t i = 0; i < cchLine; i++, x += fontWidth) @@ -508,7 +508,7 @@ using namespace Microsoft::Console::Render; } } - if (lines & GridLines::Right) + if (lines.test(GridLines::Right)) { // NOTE: We have to subtract the stroke width from the cell width // to ensure the x coordinate remains inside the clipping rectangle. @@ -519,13 +519,13 @@ using namespace Microsoft::Console::Render; } } - if (lines & GridLines::Top) + if (lines.test(GridLines::Top)) { const auto y = ptTarget.y; RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.gridlineWidth)); } - if (lines & GridLines::Bottom) + if (lines.test(GridLines::Bottom)) { // NOTE: We have to subtract the stroke width from the cell height // to ensure the y coordinate remains inside the clipping rectangle. @@ -533,19 +533,19 @@ using namespace Microsoft::Console::Render; RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.gridlineWidth)); } - if (lines & (GridLines::Underline | GridLines::DoubleUnderline)) + if (lines.any(GridLines::Underline, GridLines::DoubleUnderline)) { const auto y = ptTarget.y + _lineMetrics.underlineOffset; RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.underlineWidth)); - if (lines & GridLines::DoubleUnderline) + if (lines.test(GridLines::DoubleUnderline)) { const auto y2 = ptTarget.y + _lineMetrics.underlineOffset2; RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y2, widthOfAllCells, _lineMetrics.underlineWidth)); } } - if (lines & GridLines::Strikethrough) + if (lines.test(GridLines::Strikethrough)) { const auto y = ptTarget.y + _lineMetrics.strikethroughOffset; RETURN_HR_IF(E_FAIL, !DrawLine(ptTarget.x, y, widthOfAllCells, _lineMetrics.strikethroughWidth)); diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index bcc98dfc4..4dd69be8a 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -30,18 +30,19 @@ namespace Microsoft::Console::Render class IRenderEngine { public: - enum GridLines + enum class GridLines { - None = 0x0, - Top = 0x1, - Bottom = 0x2, - Left = 0x4, - Right = 0x8, - Underline = 0x10, - DoubleUnderline = 0x20, - Strikethrough = 0x40, - HyperlinkUnderline = 0x80 + None, + Top, + Bottom, + Left, + Right, + Underline, + DoubleUnderline, + Strikethrough, + HyperlinkUnderline }; + using GridLineSet = til::enumset; virtual ~IRenderEngine() = 0; @@ -86,7 +87,7 @@ namespace Microsoft::Console::Render const COORD coord, const bool fTrimLeft, const bool lineWrapped) noexcept = 0; - [[nodiscard]] virtual HRESULT PaintBufferGridLines(const GridLines lines, + [[nodiscard]] virtual HRESULT PaintBufferGridLines(const GridLineSet lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept = 0; @@ -118,5 +119,3 @@ namespace Microsoft::Console::Render inline Microsoft::Console::Render::IRenderEngine::~IRenderEngine() {} } - -DEFINE_ENUM_FLAG_OPERATORS(Microsoft::Console::Render::IRenderEngine::GridLines) diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index 2d5d17196..f3206e79d 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -324,7 +324,7 @@ CATCH_RETURN(); // - coordTarget - // Return Value: // - S_FALSE -[[nodiscard]] HRESULT UiaEngine::PaintBufferGridLines(GridLines const /*lines*/, +[[nodiscard]] HRESULT UiaEngine::PaintBufferGridLines(GridLineSet const /*lines*/, COLORREF const /*color*/, size_t const /*cchLine*/, COORD const /*coordTarget*/) noexcept diff --git a/src/renderer/uia/UiaRenderer.hpp b/src/renderer/uia/UiaRenderer.hpp index 6591579c5..8c438f055 100644 --- a/src/renderer/uia/UiaRenderer.hpp +++ b/src/renderer/uia/UiaRenderer.hpp @@ -55,7 +55,7 @@ namespace Microsoft::Console::Render COORD const coord, bool const fTrimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; + [[nodiscard]] HRESULT PaintBufferGridLines(GridLineSet const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override; diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp index f8b8a1266..1b9efb35e 100644 --- a/src/renderer/vt/paint.cpp +++ b/src/renderer/vt/paint.cpp @@ -142,7 +142,7 @@ using namespace Microsoft::Console::Types; // - coordTarget - The starting X/Y position of the first character to draw on. // Return Value: // - S_OK -[[nodiscard]] HRESULT VtEngine::PaintBufferGridLines(const GridLines /*lines*/, +[[nodiscard]] HRESULT VtEngine::PaintBufferGridLines(const GridLineSet /*lines*/, const COLORREF /*color*/, const size_t /*cchLine*/, const COORD /*coordTarget*/) noexcept diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index bb8f0413f..1965034db 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -65,7 +65,7 @@ namespace Microsoft::Console::Render const COORD coord, const bool trimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(const GridLines lines, + [[nodiscard]] HRESULT PaintBufferGridLines(const GridLineSet lines, const COLORREF color, const size_t cchLine, const COORD coordTarget) noexcept override; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index ba67819ab..1ee11d741 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -287,7 +287,7 @@ bool WddmConEngine::IsInitialized() CATCH_RETURN(); } -[[nodiscard]] HRESULT WddmConEngine::PaintBufferGridLines(GridLines const /*lines*/, +[[nodiscard]] HRESULT WddmConEngine::PaintBufferGridLines(GridLineSet const /*lines*/, COLORREF const /*color*/, size_t const /*cchLine*/, COORD const /*coordTarget*/) noexcept diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index 57663d5cb..0da9f5cba 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -45,7 +45,7 @@ namespace Microsoft::Console::Render const COORD coord, const bool trimLeft, const bool lineWrapped) noexcept override; - [[nodiscard]] HRESULT PaintBufferGridLines(GridLines const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; + [[nodiscard]] HRESULT PaintBufferGridLines(GridLineSet const lines, COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override;