Fix search highlights during reflow

src/buffer/out/search.cpp

@@ -13,23 +13,25 @@ bool Search::ResetIfStale(Microsoft::Console::Render::IRenderData& renderData, c
     const auto& textBuffer = renderData.GetTextBuffer();
     const auto lastMutationId = textBuffer.GetLastMutationId();
 
-    if (_needle == needle &&
+    if (_renderData == &renderData &&
+        _needle == needle &&
         _caseInsensitive == caseInsensitive &&
         _lastMutationId == lastMutationId)
     {
         _step = reverse ? -1 : 1;
         return false;
     }
 
+    if (prevResults)
+    {
+        *prevResults = std::move(_results);
+    }
+
     _renderData = &renderData;
     _needle = needle;
     _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;
@@ -142,4 +144,4 @@ const std::vector<til::point_span>& Search::Results() const noexcept
 ptrdiff_t Search::CurrentMatch() const noexcept
 {
     return _index;
-}
+}

src/cascadia/TerminalControl/ControlCore.cpp

@@ -29,19 +29,6 @@ using namespace winrt::Windows::Graphics::Display;
 using namespace winrt::Windows::System;
 using namespace winrt::Windows::ApplicationModel::DataTransfer;
 
-// The minimum delay between updates to the scroll bar's values.
-// The updates are throttled to limit power usage.
-constexpr const auto ScrollBarUpdateInterval = std::chrono::milliseconds(8);
-
-// The minimum delay between updating the TSF input control.
-constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100);
-
-// The minimum delay between updating the locations of regex patterns
-constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500);
-
-// The delay before performing the search after change of search criteria
-constexpr const auto SearchAfterChangeDelay = std::chrono::milliseconds(200);
-
 namespace winrt::Microsoft::Terminal::Control::implementation
 {
     static winrt::Microsoft::Terminal::Core::OptionalColor OptionalFromColor(const til::color& c) noexcept
@@ -117,9 +104,6 @@ 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);
 
@@ -167,31 +151,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             _dispatcher = controller.DispatcherQueue();
         }
 
-        // A few different events should be throttled, so they don't fire absolutely all the time:
-        // * _updatePatternLocations: When there's new output, or we scroll the
-        //   viewport, we should re-check if there are any visible hyperlinks.
-        //   But we don't really need to do this every single time text is
-        //   output, we can limit this update to once every 500ms.
-        // * _updateScrollBar: Same idea as the TSF update - we don't _really_
-        //   need to hop across the process boundary every time text is output.
-        //   We can throttle this to once every 8ms, which will get us out of
-        //   the way of the main output & rendering threads.
         const auto shared = _shared.lock();
+        // Raises an OutputIdle event once there hasn't been any output for at least 100ms.
+        // It also updates all regex patterns in the viewport.
+        //
         // NOTE: Calling UpdatePatternLocations from a background
         // thread is a workaround for us to hit GH#12607 less often.
-        shared->updatePatternLocations = std::make_unique<til::throttled_func_trailing<>>(
-            UpdatePatternLocationsInterval,
-            [weakTerminal = std::weak_ptr{ _terminal }]() {
+        shared->outputIdle = std::make_unique<til::debounced_func_trailing<>>(
+            std::chrono::milliseconds{ 100 },
+            [weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() {
+                dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() {
+                    if (const auto self = weakThis.get(); !self->_IsClosing())
+                    {
+                        self->OutputIdle.raise(*self, nullptr);
+                    }
+                });
+
                 if (const auto t = weakTerminal.lock())
                 {
                     const auto lock = t->LockForWriting();
                     t->UpdatePatternsUnderLock();
                 }
             });
 
+        // Scrollbar updates are also expensive (XAML), so we'll throttle them as well.
         shared->updateScrollBar = std::make_shared<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>>(
             _dispatcher,
-            ScrollBarUpdateInterval,
+            std::chrono::milliseconds{ 8 },
             [weakThis = get_weak()](const auto& update) {
                 if (auto core{ weakThis.get() }; !core->_IsClosing())
                 {
@@ -218,7 +204,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // thread. These will be recreated in _setupDispatcherAndCallbacks, when
         // we're re-attached to a new control (on a possibly new UI thread).
         const auto shared = _shared.lock();
-        shared->updatePatternLocations.reset();
+        shared->outputIdle.reset();
         shared->updateScrollBar.reset();
     }
 
@@ -671,9 +657,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
 
         const auto shared = _shared.lock_shared();
-        if (shared->updatePatternLocations)
+        if (shared->outputIdle)
         {
-            (*shared->updatePatternLocations)();
+            (*shared->outputIdle)();
         }
     }
 
@@ -1100,12 +1086,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // If this function succeeds with S_FALSE, then the terminal didn't
         // actually change size. No need to notify the connection of this no-op.
         const auto hr = _terminal->UserResize({ vp.Width(), vp.Height() });
-        if (SUCCEEDED(hr) && hr != S_FALSE)
+        if (FAILED(hr) || hr == S_FALSE)
         {
-            _connection.Resize(vp.Height(), vp.Width());
+            return;
+        }
+
+        _connection.Resize(vp.Height(), vp.Width());
 
-            // let the UI know that the text layout has been updated
-            _terminal->NotifyTextLayoutUpdated();
+        // TermControl will call Search() once the OutputIdle even fires after 100ms.
+        // Until then we need to hide the now-stale search results from the renderer.
+        ClearSearch();
+        const auto shared = _shared.lock_shared();
+        if (shared->outputIdle)
+        {
+            (*shared->outputIdle)();
         }
     }
 
@@ -1603,16 +1597,6 @@ 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:
@@ -1672,13 +1656,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - caseSensitive: boolean that represents if the current search is case sensitive
     // Return Value:
     // - <none>
-    void ControlCore::Search(const winrt::hstring& text, const bool goForward, const bool caseSensitive)
+    SearchResults ControlCore::Search(const std::wstring_view& text, const bool goForward, const bool caseSensitive, const bool reset)
     {
         const auto lock = _terminal->LockForWriting();
 
+        bool searchInvalidated = false;
         std::vector<til::point_span> oldResults;
         if (_searcher.ResetIfStale(*GetRenderData(), text, !goForward, !caseSensitive, &oldResults))
         {
+            searchInvalidated = true;
+
             _cachedSearchResultRows = {};
             if (SnapSearchResultToSelection())
             {
@@ -1687,30 +1674,28 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             }
 
             _terminal->SetSearchHighlights(_searcher.Results());
-            _terminal->SetSearchHighlightFocused(_searcher.CurrentMatch());
         }
-        else
+        else if (!reset)
         {
             _searcher.FindNext();
-            _terminal->SetSearchHighlightFocused(_searcher.CurrentMatch());
         }
-        _renderer->TriggerSearchHighlight(oldResults);
 
-        auto evArgs = winrt::make_self<implementation::UpdateSearchResultsEventArgs>();
-        if (!text.empty())
+        int32_t totalMatches = 0;
+        int32_t currentMatch = 0;
+        if (const auto idx = _searcher.CurrentMatch(); idx >= 0)
         {
-            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()));
-            }
+            totalMatches = gsl::narrow<int32_t>(_searcher.Results().size());
+            currentMatch = gsl::narrow<int32_t>(idx);
+            _terminal->SetSearchHighlightFocused(gsl::narrow<size_t>(idx));
         }
 
-        // 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);
+        _renderer->TriggerSearchHighlight(oldResults);
+
+        return {
+            .TotalMatches = totalMatches,
+            .CurrentMatch = currentMatch,
+            .SearchInvalidated = searchInvalidated,
+        };
     }
 
     Windows::Foundation::Collections::IVector<int32_t> ControlCore::SearchResultRows()
@@ -1738,16 +1723,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void ControlCore::ClearSearch()
     {
-        // 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 = {};
-        }
+        const auto lock = _terminal->LockForWriting();
+        _terminal->SetSearchHighlights({});
+        _terminal->SetSearchHighlightFocused({});
+        _renderer->TriggerSearchHighlight(_searcher.Results());
+        _searcher = {};
+        _cachedSearchResultRows = {};
     }
 
     // Method Description:
@@ -2130,9 +2111,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
             // Start the throttled update of where our hyperlinks are.
             const auto shared = _shared.lock_shared();
-            if (shared->updatePatternLocations)
+            if (shared->outputIdle)
             {
-                (*shared->updatePatternLocations)();
+                (*shared->outputIdle)();
             }
         }
         catch (...)

src/cascadia/TerminalControl/ControlCore.h

@@ -219,7 +219,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void SetSelectionAnchor(const til::point position);
         void SetEndSelectionPoint(const til::point position);
 
-        void Search(const winrt::hstring& text, const bool goForward, const bool caseSensitive);
+        SearchResults Search(const std::wstring_view& text, bool goForward, bool caseSensitive, bool reset);
         void ClearSearch();
         void SnapSearchResultToSelection(bool snap) noexcept;
         bool SnapSearchResultToSelection() const noexcept;
@@ -279,8 +279,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         til::typed_event<IInspectable, Control::RendererWarningArgs> RendererWarning;
         til::typed_event<IInspectable, Control::NoticeEventArgs> RaiseNotice;
         til::typed_event<IInspectable, Control::TransparencyChangedEventArgs> TransparencyChanged;
-        til::typed_event<> ReceivedOutput;
-        til::typed_event<IInspectable, Control::UpdateSearchResultsEventArgs> UpdateSearchResults;
+        til::typed_event<> OutputIdle;
         til::typed_event<IInspectable, Control::ShowWindowArgs> ShowWindowChanged;
         til::typed_event<IInspectable, Control::UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
         til::typed_event<IInspectable, Control::OpenHyperlinkEventArgs> OpenHyperlink;
@@ -295,7 +294,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     private:
         struct SharedState
         {
-            std::unique_ptr<til::throttled_func_trailing<>> updatePatternLocations;
+            std::unique_ptr<til::debounced_func_trailing<>> outputIdle;
             std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> updateScrollBar;
         };
 
@@ -376,7 +375,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                                             const int bufferSize);
         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

@@ -49,6 +49,13 @@ namespace Microsoft.Terminal.Control
         Boolean EndAtRightBoundary;
     };
 
+    struct SearchResults
+    {
+        Int32 TotalMatches;
+        Int32 CurrentMatch;
+        Boolean SearchInvalidated;
+    };
+
     [default_interface] runtimeclass SelectionColor
     {
         SelectionColor();
@@ -127,7 +134,7 @@ namespace Microsoft.Terminal.Control
         void ResumeRendering();
         void BlinkAttributeTick();
 
-        void Search(String text, Boolean goForward, Boolean caseSensitive);
+        SearchResults Search(String text, Boolean goForward, Boolean caseSensitive, Boolean reset);
         void ClearSearch();
         IVector<Int32> SearchResultRows { get; };
         Boolean SnapSearchResultToSelection;
@@ -177,8 +184,7 @@ namespace Microsoft.Terminal.Control
         event Windows.Foundation.TypedEventHandler<Object, RendererWarningArgs> RendererWarning;
         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, UpdateSearchResultsEventArgs> UpdateSearchResults;
+        event Windows.Foundation.TypedEventHandler<Object, Object> OutputIdle;
         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,6 @@
 #include "ScrollPositionChangedArgs.g.cpp"
 #include "RendererWarningArgs.g.cpp"
 #include "TransparencyChangedEventArgs.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,6 @@
 #include "ScrollPositionChangedArgs.g.h"
 #include "RendererWarningArgs.g.h"
 #include "TransparencyChangedEventArgs.g.h"
-#include "UpdateSearchResultsEventArgs.g.h"
 #include "ShowWindowArgs.g.h"
 #include "UpdateSelectionMarkersEventArgs.g.h"
 #include "CompletionsChangedEventArgs.g.h"
@@ -141,17 +140,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         WINRT_PROPERTY(float, Opacity);
     };
 
-    struct UpdateSearchResultsEventArgs : public UpdateSearchResultsEventArgsT<UpdateSearchResultsEventArgs>
-    {
-    public:
-        UpdateSearchResultsEventArgs() = default;
-
-        WINRT_PROPERTY(SearchState, State, SearchState::Inactive);
-        WINRT_PROPERTY(bool, FoundMatch);
-        WINRT_PROPERTY(int32_t, TotalMatches);
-        WINRT_PROPERTY(int32_t, CurrentMatch);
-    };
-
     struct ShowWindowArgs : public ShowWindowArgsT<ShowWindowArgs>
     {
     public:

src/cascadia/TerminalControl/EventArgs.idl

@@ -84,14 +84,6 @@ namespace Microsoft.Terminal.Control
         Active = 1,
     };
 
-    runtimeclass UpdateSearchResultsEventArgs
-    {
-        SearchState State { get; };
-        Boolean FoundMatch { get; };
-        Int32 TotalMatches { get; };
-        Int32 CurrentMatch { get; };
-    }
-
     runtimeclass ShowWindowArgs
     {
         Boolean ShowOrHide { get; };