Improve Search Highlighting (#16611)

### The changeset involves:
- Decoupling Selection and Search Highlighting code paths.
- We no longer invalidate search highlights when:
  - Left-clicking on terminal
  - A new selection is made
  - Left-clicking on Search-box
- Dispatching Find Next/Prev Match Action. (The search highlight was
removed after pressing the first key of the Action's key combination)
- And, anything that doesn't change buffer content, shouldn't invalidate
the highlighted region (E.g. Cursor movement)
- Highlighting foreground color is *actually* applied to the highlighted
text.
- Double-clicking on SearchBox no longer starts a text selection in the
terminal.
- Selected text is properly populated in the Search Box (#16355)

Closes: #16355


![image](https://github.com/microsoft/terminal/assets/55626797/8fd0345b-a8b2-4bc2-a25e-15d710127b63)

## Some Implementation Details

### Detecting text layout changes in the Control layer

As Search Highlight regions need to be removed when new text is added,
or the existing text is re-arranged due to window resize or similar
events, a new event `TextLayoutUpdated` is added that notifies
`CoreControl` of any text layout changes. The event is used to
invalidate and remove all search highlight regions from the buffer
(because the regions might not be _fresh_ anymore.

The new event is raised when:
1. `AdaptDispatch` writes new text into the buffer.
2. MainBuffer is switched to AltBuffer or vice-versa.
3. The user resized the window.
4. Font size changed.
5. Zoom level changed.

(Intensionally,) It's not raised when:
1. Buffer is scrolled.
2. The text cursor is moved.

When `ControlCore` receives a `TextLayoutUpdated` event, it clears the
Search Highlights in the *render data*, and raises an
`UpdateSearchResults` event to notify `TermControl` to update the Search
UI (`SearchBoxControl`).

In the future, we can use `TextLayoutUpdated` event to start a new
search which would refresh the results automatically after a slight
delay (throttled). *VSCode already does this today*.

### How does AtlasEngine draw the highlighted regions?

We follow a similar idea as for drawing the Selection region. When new
regions are available, the old+new regions are marked invalidated.
Later, a call to `_drawHighlighted()` is made at the end of
`PaintBufferLine()` to override the highlighted regions' colors with
highlight colors. The highlighting colors replace the buffer colors
while search highlights are active.

Note that to paint search highlights, we currently invalidate the row
completely. This forces text shaping for the rows in the viewport that
have at least one highlighted region. This is done to keep the (already
lengthy) PR... simple. We could take advantage of the fact that only
colors have changed and not the characters (or glyphs). I'm expecting
that this could be improved like:
1. When search regions are added, we add the highlighting colors to the
color bitmaps without causing text shaping.
2. When search regions are removed, we re-fill the color bitmaps with
the original colors from the Buffer.

## Validation Steps:
- New text, window resize, font size changes, zooming, and pasting
content into the terminal removes search highlights.
- highlighting colors override the foreground and background color of
the text (in the rendered output).
- Blinking, faded, reverse video, Intense text is highlighted as
expected.

src/buffer/out/search.cpp

@@ -8,7 +8,7 @@
 
 using namespace Microsoft::Console::Types;
 
-bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive)
+bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive, std::vector<til::point_span>* prevResults)
 {
     const auto& textBuffer = renderData.GetTextBuffer();
     const auto lastMutationId = textBuffer.GetLastMutationId();
@@ -26,10 +26,13 @@ bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, c
     _caseInsensitive = caseInsensitive;
     _lastMutationId = lastMutationId;
 
+    if (prevResults)
+    {
+        *prevResults = std::move(_results);
+    }
     _results = textBuffer.SearchText(needle, caseInsensitive);
     _index = reverse ? gsl::narrow_cast<ptrdiff_t>(_results.size()) - 1 : 0;
     _step = reverse ? -1 : 1;
-
     return true;
 }
 
@@ -111,28 +114,6 @@ const til::point_span* Search::GetCurrent() const noexcept
     return nullptr;
 }
 
-void Search::HighlightResults() const
-{
-    std::vector<til::inclusive_rect> toSelect;
-    const auto& textBuffer = _renderData->GetTextBuffer();
-
-    for (const auto& r : _results)
-    {
-        const auto rbStart = textBuffer.BufferToScreenPosition(r.start);
-        const auto rbEnd = textBuffer.BufferToScreenPosition(r.end);
-
-        til::inclusive_rect re;
-        re.top = rbStart.y;
-        re.bottom = rbEnd.y;
-        re.left = rbStart.x;
-        re.right = rbEnd.x;
-
-        toSelect.emplace_back(re);
-    }
-
-    _renderData->SelectSearchRegions(std::move(toSelect));
-}
-
 // Routine Description:
 // - Takes the found word and selects it in the screen buffer
 
@@ -161,4 +142,4 @@ const std::vector<til::point_span>& Search::Results() const noexcept
 ptrdiff_t Search::CurrentMatch() const noexcept
 {
     return _index;
-}
+}

src/buffer/out/search.h

@@ -25,15 +25,18 @@ class Search final
 public:
     Search() = default;
 
-    bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, const std::wstring_view& needle, bool reverse, bool caseInsensitive);
+    bool ResetIfStale(Microsoft::Console::Render::IRenderData& renderData,
+                      const std::wstring_view& needle,
+                      bool reverse,
+                      bool caseInsensitive,
+                      std::vector<til::point_span>* prevResults = nullptr);
 
     void MoveToCurrentSelection();
     void MoveToPoint(til::point anchor) noexcept;
     void MovePastPoint(til::point anchor) noexcept;
     void FindNext() noexcept;
 
     const til::point_span* GetCurrent() const noexcept;
-    void HighlightResults() const;
     bool SelectCurrent() const;
 
     const std::vector<til::point_span>& Results() const noexcept;

src/cascadia/TerminalControl/ControlCore.cpp

@@ -120,6 +120,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         auto pfnShowWindowChanged = std::bind(&ControlCore::_terminalShowWindowChanged, this, std::placeholders::_1);
         _terminal->SetShowWindowCallback(pfnShowWindowChanged);
 
+        auto pfnTextLayoutUpdated = std::bind(&ControlCore::_terminalTextLayoutUpdated, this);
+        _terminal->SetTextLayoutUpdatedCallback(pfnTextLayoutUpdated);
+
         auto pfnPlayMidiNote = std::bind(&ControlCore::_terminalPlayMidiNote, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3);
         _terminal->SetPlayMidiNoteCallback(pfnPlayMidiNote);
 
@@ -1123,6 +1126,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         if (SUCCEEDED(hr) && hr != S_FALSE)
         {
             _connection.Resize(vp.Height(), vp.Width());
+
+            // let the UI know that the text layout has been updated
+            _terminal->NotifyTextLayoutUpdated();
         }
     }
 
@@ -1638,6 +1644,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         ShowWindowChanged.raise(*this, *showWindow);
     }
 
+    void ControlCore::_terminalTextLayoutUpdated()
+    {
+        ClearSearch();
+
+        // send an UpdateSearchResults event to the UI to put the Search UI into inactive state.
+        auto evArgs = winrt::make_self<implementation::UpdateSearchResultsEventArgs>();
+        evArgs->State(SearchState::Inactive);
+        UpdateSearchResults.raise(*this, *evArgs);
+    }
+
     // Method Description:
     // - Plays a single MIDI note, blocking for the duration.
     // Arguments:
@@ -1701,43 +1717,45 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         const auto lock = _terminal->LockForWriting();
 
-        if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive))
+        std::vector<til::point_span> oldResults;
+        if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive, &oldResults))
         {
-            _searcher.HighlightResults();
-            _searcher.MoveToCurrentSelection();
             _cachedSearchResultRows = {};
+            if (SnapSearchResultToSelection())
+            {
+                _searcher.MoveToCurrentSelection();
+                SnapSearchResultToSelection(false);
+            }
+
+            _terminal->SetSearchHighlights(_searcher.Results());
+            _terminal->SetSearchHighlightFocused(_searcher.CurrentMatch());
         }
         else
         {
             _searcher.FindNext();
+            _terminal->SetSearchHighlightFocused(_searcher.CurrentMatch());
         }
+        _renderer->TriggerSearchHighlight(oldResults);
 
-        const auto foundMatch = _searcher.SelectCurrent();
-        auto foundResults = winrt::make_self<implementation::FoundResultsArgs>(foundMatch);
-        if (foundMatch)
+        auto evArgs = winrt::make_self<implementation::UpdateSearchResultsEventArgs>();
+        if (!text.empty())
         {
-            // this is used for search,
-            // DO NOT call _updateSelectionUI() here.
-            // We don't want to show the markers so manually tell it to clear it.
-            _terminal->SetBlockSelection(false);
-            UpdateSelectionMarkers.raise(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(true));
-
-            foundResults->TotalMatches(gsl::narrow<int32_t>(_searcher.Results().size()));
-            foundResults->CurrentMatch(gsl::narrow<int32_t>(_searcher.CurrentMatch()));
-
-            _terminal->AlwaysNotifyOnBufferRotation(true);
+            evArgs->State(SearchState::Active);
+            if (_searcher.GetCurrent())
+            {
+                evArgs->FoundMatch(true);
+                evArgs->TotalMatches(gsl::narrow<int32_t>(_searcher.Results().size()));
+                evArgs->CurrentMatch(gsl::narrow<int32_t>(_searcher.CurrentMatch()));
+            }
         }
-        _renderer->TriggerSelection();
 
-        // Raise a FoundMatch event, which the control will use to notify
-        // narrator if there was any results in the buffer
-        FoundMatch.raise(*this, *foundResults);
+        // Raise an UpdateSearchResults event, which the control will use to update the
+        // UI and notify the narrator about the updated search results in the buffer
+        UpdateSearchResults.raise(*this, *evArgs);
     }
 
     Windows::Foundation::Collections::IVector<int32_t> ControlCore::SearchResultRows()
     {
-        const auto lock = _terminal->LockForReading();
-
         if (!_cachedSearchResultRows)
         {
             auto results = std::vector<int32_t>();
@@ -1761,8 +1779,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void ControlCore::ClearSearch()
     {
-        _terminal->AlwaysNotifyOnBufferRotation(false);
-        _searcher = {};
+        // nothing to clear if there's no results
+        if (_searcher.GetCurrent())
+        {
+            const auto lock = _terminal->LockForWriting();
+            _terminal->SetSearchHighlights({});
+            _terminal->SetSearchHighlightFocused({});
+            _renderer->TriggerSearchHighlight(_searcher.Results());
+            _searcher = {};
+            _cachedSearchResultRows = {};
+        }
+    }
+
+    // Method Description:
+    // - Tells ControlCore to snap the current search result index to currently
+    //   selected text if the search was started using it.
+    void ControlCore::SnapSearchResultToSelection(bool shouldSnap) noexcept
+    {
+        _snapSearchResultToSelection = shouldSnap;
+    }
+
+    // Method Description:
+    // - Returns true, if we should snap the current search result index to
+    //   the currently selected text after a new search is started, else false.
+    bool ControlCore::SnapSearchResultToSelection() const noexcept
+    {
+        return _snapSearchResultToSelection;
     }
 
     void ControlCore::Close()

src/cascadia/TerminalControl/ControlCore.h

@@ -221,6 +221,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         void Search(const winrt::hstring& text, const bool goForward, const bool caseSensitive);
         void ClearSearch();
+        void SnapSearchResultToSelection(bool snap) noexcept;
+        bool SnapSearchResultToSelection() const noexcept;
 
         Windows::Foundation::Collections::IVector<int32_t> SearchResultRows();
 
@@ -281,7 +283,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         til::typed_event<IInspectable, Control::NoticeEventArgs> RaiseNotice;
         til::typed_event<IInspectable, Control::TransparencyChangedEventArgs> TransparencyChanged;
         til::typed_event<> ReceivedOutput;
-        til::typed_event<IInspectable, Control::FoundResultsArgs> FoundMatch;
+        til::typed_event<IInspectable, Control::UpdateSearchResultsEventArgs> UpdateSearchResults;
         til::typed_event<IInspectable, Control::ShowWindowArgs> ShowWindowChanged;
         til::typed_event<IInspectable, Control::UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
         til::typed_event<IInspectable, Control::OpenHyperlinkEventArgs> OpenHyperlink;
@@ -321,6 +323,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr };
 
         ::Search _searcher;
+        bool _snapSearchResultToSelection;
 
         winrt::handle _lastSwapChainHandle{ nullptr };
 
@@ -379,6 +382,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _terminalCursorPositionChanged();
         void _terminalTaskbarProgressChanged();
         void _terminalShowWindowChanged(bool showOrHide);
+        void _terminalTextLayoutUpdated();
         void _terminalPlayMidiNote(const int noteNumber,
                                    const int velocity,
                                    const std::chrono::microseconds duration);

src/cascadia/TerminalControl/ControlCore.idl

@@ -131,6 +131,7 @@ namespace Microsoft.Terminal.Control
         void Search(String text, Boolean goForward, Boolean caseSensitive);
         void ClearSearch();
         IVector<Int32> SearchResultRows { get; };
+        Boolean SnapSearchResultToSelection;
 
         Microsoft.Terminal.Core.Color ForegroundColor { get; };
         Microsoft.Terminal.Core.Color BackgroundColor { get; };
@@ -179,7 +180,7 @@ namespace Microsoft.Terminal.Control
         event Windows.Foundation.TypedEventHandler<Object, NoticeEventArgs> RaiseNotice;
         event Windows.Foundation.TypedEventHandler<Object, TransparencyChangedEventArgs> TransparencyChanged;
         event Windows.Foundation.TypedEventHandler<Object, Object> ReceivedOutput;
-        event Windows.Foundation.TypedEventHandler<Object, FoundResultsArgs> FoundMatch;
+        event Windows.Foundation.TypedEventHandler<Object, UpdateSearchResultsEventArgs> UpdateSearchResults;
         event Windows.Foundation.TypedEventHandler<Object, UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
         event Windows.Foundation.TypedEventHandler<Object, OpenHyperlinkEventArgs> OpenHyperlink;
         event Windows.Foundation.TypedEventHandler<Object, Object> CloseTerminalRequested;

src/cascadia/TerminalControl/EventArgs.cpp

@@ -12,7 +12,7 @@
 #include "ScrollPositionChangedArgs.g.cpp"
 #include "RendererWarningArgs.g.cpp"
 #include "TransparencyChangedEventArgs.g.cpp"
-#include "FoundResultsArgs.g.cpp"
+#include "UpdateSearchResultsEventArgs.g.cpp"
 #include "ShowWindowArgs.g.cpp"
 #include "UpdateSelectionMarkersEventArgs.g.cpp"
 #include "CompletionsChangedEventArgs.g.cpp"

src/cascadia/TerminalControl/EventArgs.h

@@ -12,7 +12,7 @@
 #include "ScrollPositionChangedArgs.g.h"
 #include "RendererWarningArgs.g.h"
 #include "TransparencyChangedEventArgs.g.h"
-#include "FoundResultsArgs.g.h"
+#include "UpdateSearchResultsEventArgs.g.h"
 #include "ShowWindowArgs.g.h"
 #include "UpdateSelectionMarkersEventArgs.g.h"
 #include "CompletionsChangedEventArgs.g.h"
@@ -141,14 +141,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         WINRT_PROPERTY(double, Opacity);
     };
 
-    struct FoundResultsArgs : public FoundResultsArgsT<FoundResultsArgs>
+    struct UpdateSearchResultsEventArgs : public UpdateSearchResultsEventArgsT<UpdateSearchResultsEventArgs>
     {
     public:
-        FoundResultsArgs(const bool foundMatch) :
-            _FoundMatch(foundMatch)
-        {
-        }
+        UpdateSearchResultsEventArgs() = default;
 
+        WINRT_PROPERTY(SearchState, State, SearchState::Inactive);
         WINRT_PROPERTY(bool, FoundMatch);
         WINRT_PROPERTY(int32_t, TotalMatches);
         WINRT_PROPERTY(int32_t, CurrentMatch);

src/cascadia/TerminalControl/EventArgs.idl

@@ -78,8 +78,15 @@ namespace Microsoft.Terminal.Control
         Double Opacity { get; };
     }
 
-    runtimeclass FoundResultsArgs
+    enum SearchState
     {
+        Inactive = 0,
+        Active = 1,
+    };
+
+    runtimeclass UpdateSearchResultsEventArgs
+    {
+        SearchState State { get; };
         Boolean FoundMatch { get; };
         Int32 TotalMatches { get; };
         Int32 CurrentMatch { get; };

src/cascadia/TerminalControl/SearchBoxControl.cpp

@@ -26,18 +26,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         });
         this->CharacterReceived({ this, &SearchBoxControl::_CharacterHandler });
         this->KeyDown({ this, &SearchBoxControl::_KeyDownHandler });
-        this->RegisterPropertyChangedCallback(UIElement::VisibilityProperty(), [this](auto&&, auto&&) {
-            // Once the control is visible again we trigger SearchChanged event.
-            // We do this since we probably have a value from the previous search,
-            // and in such case logically the search changes from "nothing" to this value.
-            // A good example for SearchChanged event consumer is Terminal Control.
-            // Once the Search Box is open we want the Terminal Control
-            // to immediately perform the search with the value appearing in the box.
-            if (Visibility() == Visibility::Visible)
-            {
-                SearchChanged.raise(TextBox().Text(), _GoForward(), _CaseSensitive());
-            }
-        });
 
         _focusableElements.insert(TextBox());
         _focusableElements.insert(CloseButton());
@@ -421,6 +409,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         SearchChanged.raise(TextBox().Text(), _GoForward(), _CaseSensitive());
     }
 
+    // Method Description:
+    // - Handler for searchbox pointer-pressed.
+    // - Marks pointer events as handled so they don't bubble up to the terminal.
+    void SearchBoxControl::SearchBoxPointerPressedHandler(winrt::Windows::Foundation::IInspectable const& /*sender*/, winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e)
+    {
+        e.Handled(true);
+    }
+
+    // Method Description:
+    // - Handler for searchbox pointer-released.
+    // - Marks pointer events as handled so they don't bubble up to the terminal.
+    void SearchBoxControl::SearchBoxPointerReleasedHandler(winrt::Windows::Foundation::IInspectable const& /*sender*/, winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e)
+    {
+        e.Handled(true);
+    }
+
     // Method Description:
     // - Formats a status message representing the search state:
     // * "Searching" - if totalMatches is negative
@@ -518,6 +522,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         StatusBox().Text(status);
     }
 
+    // Method Description:
+    // - Removes the status message in the status box.
+    void SearchBoxControl::ClearStatus()
+    {
+        StatusBox().Text(L"");
+    }
+
     // Method Description:
     // - Enables / disables results navigation buttons
     // Arguments:

src/cascadia/TerminalControl/SearchBoxControl.h

@@ -39,6 +39,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void PopulateTextbox(const winrt::hstring& text);
         bool ContainsFocus();
         void SetStatus(int32_t totalMatches, int32_t currentMatch);
+        void ClearStatus();
         bool NavigationEnabled();
         void NavigationEnabled(bool enabled);
 
@@ -48,6 +49,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         void TextBoxTextChanged(winrt::Windows::Foundation::IInspectable const& sender, winrt::Windows::UI::Xaml::RoutedEventArgs const& e);
         void CaseSensitivityButtonClicked(winrt::Windows::Foundation::IInspectable const& sender, winrt::Windows::UI::Xaml::RoutedEventArgs const& e);
+        void SearchBoxPointerPressedHandler(winrt::Windows::Foundation::IInspectable const& /*sender*/, winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e);
+        void SearchBoxPointerReleasedHandler(winrt::Windows::Foundation::IInspectable const& /*sender*/, winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs const& e);
 
         til::event<SearchHandler> Search;
         til::event<SearchHandler> SearchChanged;