Expose hyperlink attributes in PaintBufferGridLines (#15090)

Rendering hyperlinks is unneccessarily complex at the moment, because it requires you to implement `UpdateDrawingBrushes`, manually extract the hyperlink flag from the given `TextAttribute` and save it until the next call to `PaintBufferGridLines` which does not get that flag. This isn't particularly clean as it assumes that `PaintBufferGridLines` will be called after `UpdateDrawingBrushes` in the first place. Instead, we can simply pass the hyperlink flag to `UpdateDrawingBrushes` so that the renderers don't need to deal with this anymore. ## PR Checklist * Hyperlinks show up with a dotted line ✅ * Hovering hyperlinks underline them ✅
microsoft · Apr 3, 2023 · 0656afc · 0656afc
1 parent 0d38d17
commit 0656afc
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 47 deletions.
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -723,7 +723,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
                 _lastHoveredId = newId;
                 _lastHoveredInterval = newInterval;
-                _renderEngine->UpdateHyperlinkHoveredId(newId);
+                _renderer->UpdateHyperlinkHoveredId(newId);
                 _renderer->UpdateLastHoveredInterval(newInterval);
                 _renderer->TriggerRedrawAll();
             }

diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp
@@ -913,55 +913,55 @@ void Renderer::_PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine,
 }
 
 // Method Description:
-// - Generates a IRenderEngine::GridLines structure from the values in the
+// - Generates a GridLines structure from the values in the
 //      provided textAttribute
 // Arguments:
 // - textAttribute: the TextAttribute to generate GridLines from.
 // Return Value:
 // - a GridLineSet containing all the gridline info from the TextAttribute
-IRenderEngine::GridLineSet Renderer::s_GetGridlines(const TextAttribute& textAttribute) noexcept
+GridLineSet Renderer::s_GetGridlines(const TextAttribute& textAttribute) noexcept
 {
     // Convert console grid line representations into rendering engine enum representations.
-    IRenderEngine::GridLineSet lines;
+    GridLineSet lines;
 
     if (textAttribute.IsTopHorizontalDisplayed())
     {
-        lines.set(IRenderEngine::GridLines::Top);
+        lines.set(GridLines::Top);
     }
 
     if (textAttribute.IsBottomHorizontalDisplayed())
     {
-        lines.set(IRenderEngine::GridLines::Bottom);
+        lines.set(GridLines::Bottom);
     }
 
     if (textAttribute.IsLeftVerticalDisplayed())
     {
-        lines.set(IRenderEngine::GridLines::Left);
+        lines.set(GridLines::Left);
     }
 
     if (textAttribute.IsRightVerticalDisplayed())
     {
-        lines.set(IRenderEngine::GridLines::Right);
+        lines.set(GridLines::Right);
     }
 
     if (textAttribute.IsCrossedOut())
     {
-        lines.set(IRenderEngine::GridLines::Strikethrough);
+        lines.set(GridLines::Strikethrough);
     }
 
     if (textAttribute.IsUnderlined())
     {
-        lines.set(IRenderEngine::GridLines::Underline);
+        lines.set(GridLines::Underline);
     }
 
     if (textAttribute.IsDoublyUnderlined())
     {
-        lines.set(IRenderEngine::GridLines::DoubleUnderline);
+        lines.set(GridLines::DoubleUnderline);
     }
 
     if (textAttribute.IsHyperlink())
     {
-        lines.set(IRenderEngine::GridLines::HyperlinkUnderline);
+        lines.set(GridLines::HyperlinkUnderline);
     }
     return lines;
 }
@@ -985,19 +985,10 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin
     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
-    // the pattern range if we aren't currently hovering.
-    if (_hoveredInterval.has_value())
+    if (_isHoveredHyperlink(textAttribute) || _isInHoveredInterval(coordTarget))
     {
-        const til::point coordTargetTil{ coordTarget };
-        if (_hoveredInterval->start <= coordTargetTil &&
-            coordTargetTil <= _hoveredInterval->stop)
-        {
-            if (_pData->GetPatternId(coordTarget).size() > 0)
-            {
-                lines.set(IRenderEngine::GridLines::Underline);
-            }
-        }
+        lines.reset(GridLines::HyperlinkUnderline);
+        lines.set(GridLines::Underline);
     }
 
     // Return early if there are no lines to paint.
@@ -1010,6 +1001,18 @@ void Renderer::_PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngin
     }
 }
 
+bool Renderer::_isHoveredHyperlink(const TextAttribute& textAttribute) const noexcept
+{
+    return _hyperlinkHoveredId && _hyperlinkHoveredId == textAttribute.GetHyperlinkId();
+}
+
+bool Renderer::_isInHoveredInterval(const til::point coordTarget) const noexcept
+{
+    return _hoveredInterval &&
+           _hoveredInterval->start <= coordTarget && coordTarget <= _hoveredInterval->stop &&
+           _pData->GetPatternId(coordTarget).size() > 0;
+}
+
 // Routine Description:
 // - Retrieve information about the cursor, and pack it into a CursorOptions
 //   which the render engine can use for painting the cursor.
@@ -1362,6 +1365,15 @@ void Renderer::ResetErrorStateAndResume()
     EnablePainting();
 }
 
+void Renderer::UpdateHyperlinkHoveredId(uint16_t id) noexcept
+{
+    _hyperlinkHoveredId = id;
+    FOREACH_ENGINE(pEngine)
+    {
+        pEngine->UpdateHyperlinkHoveredId(id);
+    }
+}
+
 void Renderer::UpdateLastHoveredInterval(const std::optional<PointTree::interval>& newInterval)
 {
     _hoveredInterval = newInterval;

diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp
@@ -89,10 +89,11 @@ namespace Microsoft::Console::Render
         void SetRendererEnteredErrorStateCallback(std::function<void()> pfn);
         void ResetErrorStateAndResume();
 
+        void UpdateHyperlinkHoveredId(uint16_t id) noexcept;
         void UpdateLastHoveredInterval(const std::optional<interval_tree::IntervalTree<til::point, size_t>::interval>& newInterval);
 
     private:
-        static IRenderEngine::GridLineSet s_GetGridlines(const TextAttribute& textAttribute) noexcept;
+        static GridLineSet s_GetGridlines(const TextAttribute& textAttribute) noexcept;
         static bool s_IsSoftFontChar(const std::wstring_view& v, const size_t firstSoftFontChar, const size_t lastSoftFontChar);
 
         [[nodiscard]] HRESULT _PaintFrameForEngine(_In_ IRenderEngine* const pEngine) noexcept;
@@ -101,6 +102,7 @@ namespace Microsoft::Console::Render
         void _PaintBufferOutput(_In_ IRenderEngine* const pEngine);
         void _PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, TextBufferCellIterator it, const til::point target, const bool lineWrapped);
         void _PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngine, const TextAttribute textAttribute, const size_t cchLine, const til::point coordTarget);
+        bool _isHoveredHyperlink(const TextAttribute& textAttribute) const noexcept;
         void _PaintSelection(_In_ IRenderEngine* const pEngine);
         void _PaintCursor(_In_ IRenderEngine* const pEngine);
         void _PaintOverlays(_In_ IRenderEngine* const pEngine);
@@ -110,6 +112,7 @@ namespace Microsoft::Console::Render
         std::vector<til::rect> _GetSelectionRects() const;
         void _ScrollPreviousSelection(const til::point delta);
         [[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine);
+        bool _isInHoveredInterval(til::point coordTarget) const noexcept;
         [[nodiscard]] std::optional<CursorOptions> _GetCursorInfo();
         [[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine);
 
@@ -119,6 +122,7 @@ namespace Microsoft::Console::Render
         std::unique_ptr<RenderThread> _pThread;
         static constexpr size_t _firstSoftFontChar = 0xEF20;
         size_t _lastSoftFontChar = 0;
+        uint16_t _hyperlinkHoveredId = 0;
         std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _hoveredInterval;
         Microsoft::Console::Types::Viewport _viewport;
         std::vector<Cluster> _clusterBuffer;

diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp
@@ -821,7 +821,6 @@ static constexpr D2D1_ALPHA_MODE _dxgiAlphaToD2d1Alpha(DXGI_ALPHA_MODE mode) noe
         // 1234123412341234
         static constexpr std::array<float, 2> hyperlinkDashes{ 1.f, 3.f };
         RETURN_IF_FAILED(_d2dFactory->CreateStrokeStyle(&_dashStrokeStyleProperties, hyperlinkDashes.data(), gsl::narrow_cast<UINT32>(hyperlinkDashes.size()), &_dashStrokeStyle));
-        _hyperlinkStrokeStyle = _dashStrokeStyle;
 
         // If in composition mode, apply scaling factor matrix
         if (_chainMode == SwapChainMode::ForComposition)
@@ -1723,7 +1722,7 @@ try
     };
 
     const auto DrawHyperlinkLine = [=](const auto x0, const auto y0, const auto x1, const auto y1, const auto strokeWidth) noexcept {
-        _d2dDeviceContext->DrawLine({ x0, y0 }, { x1, y1 }, _d2dBrushForeground.Get(), strokeWidth, _hyperlinkStrokeStyle.Get());
+        _d2dDeviceContext->DrawLine({ x0, y0 }, { x1, y1 }, _d2dBrushForeground.Get(), strokeWidth, _dashStrokeStyle.Get());
     };
 
     // NOTE: Line coordinates are centered within the line, so they need to be
@@ -1980,11 +1979,6 @@ try
         _drawingContext->useItalicFont = textAttributes.IsItalic();
     }
 
-    if (textAttributes.IsHyperlink())
-    {
-        _hyperlinkStrokeStyle = (textAttributes.GetHyperlinkId() == _hyperlinkHoveredId) ? _strokeStyle : _dashStrokeStyle;
-    }
-
     // Update pixel shader settings as background color might have changed
     _ComputePixelShaderSettings();
 

diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp
@@ -203,7 +203,6 @@ namespace Microsoft::Console::Render
         ::Microsoft::WRL::ComPtr<CustomTextRenderer> _customRenderer;
         ::Microsoft::WRL::ComPtr<ID2D1StrokeStyle> _strokeStyle;
         ::Microsoft::WRL::ComPtr<ID2D1StrokeStyle> _dashStrokeStyle;
-        ::Microsoft::WRL::ComPtr<ID2D1StrokeStyle> _hyperlinkStrokeStyle;
 
         std::unique_ptr<DxFontRenderData> _fontRenderData;
         DxSoftFont _softFont;

diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp
@@ -32,23 +32,23 @@ namespace Microsoft::Console::Render
         std::optional<CursorOptions> cursorInfo;
     };
 
+    enum class GridLines
+    {
+        None,
+        Top,
+        Bottom,
+        Left,
+        Right,
+        Underline,
+        DoubleUnderline,
+        Strikethrough,
+        HyperlinkUnderline
+    };
+    using GridLineSet = til::enumset<GridLines>;
+
     class __declspec(novtable) IRenderEngine
     {
     public:
-        enum class GridLines
-        {
-            None,
-            Top,
-            Bottom,
-            Left,
-            Right,
-            Underline,
-            DoubleUnderline,
-            Strikethrough,
-            HyperlinkUnderline
-        };
-        using GridLineSet = til::enumset<GridLines>;
-
 #pragma warning(suppress : 26432) // If you define or delete any default operation in the type '...', define or delete them all (c.21).
         virtual ~IRenderEngine() = default;