I wasn't the murderer after all

microsoft · Mar 11, 2021 · 5e655ef · 5e655ef
1 parent acf84f1
commit 5e655ef
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 27 deletions.
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -144,13 +144,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(_terminal.get(), nullptr, 0, std::move(renderThread));
             ::Microsoft::Console::Render::IRenderTarget& renderTarget = *_renderer;
 
-            // !TODO!: We _DO_ want this
-            // _renderer->SetRendererEnteredErrorStateCallback([weakThis = get_weak()]() {
-            //     if (auto strongThis{ weakThis.get() })
-            //     {
-            //         strongThis->_RendererEnteredErrorState();
-            //     }
-            // });
+            _renderer->SetRendererEnteredErrorStateCallback([weakThis = get_weak()]() {
+                if (auto strongThis{ weakThis.get() })
+                {
+                    strongThis->_RendererEnteredErrorStateHandlers(*strongThis, nullptr);
+                }
+            });
 
             THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
 
@@ -225,9 +224,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             THROW_IF_FAILED(dxEngine->Enable());
             _renderEngine = std::move(dxEngine);
 
-            // !TODO! in the past we did _AttachDxgiSwapChainToXaml _before_ calling
-            // EnablePainting. Mild worry that doing EnablePainting first will
-            // break
+            // In the past we did _AttachDxgiSwapChainToXaml _before_ calling
+            // EnablePainting. There's mild worry that doing EnablePainting
+            // first will break something, but this seems to work.
             localPointerToThread->EnablePainting();
 
             _initializedTerminal = true;
@@ -237,10 +236,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         // start writing output immediately.
         _connection.Start();
 
-        // !TODO!: Do we want this?
-        // Likewise, run the event handlers outside of lock (they could
-        // be reentrant)
-        // _InitializedHandlers(*this, nullptr);
         return true;
     }
     // Method Description:
@@ -979,14 +974,6 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
                                                      const int viewHeight,
                                                      const int bufferSize)
     {
-        // !!TODO!! do we need this?
-        // // Since this callback fires from non-UI thread, we might be already
-        // // closed/closing.
-        // if (_closing.load())
-        // {
-        //     return;
-        // }
-
         // Clear the regex pattern tree so the renderer does not try to render
         // them while scrolling
         _terminal->ClearPatternTree();
@@ -1034,8 +1021,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
                              const bool goForward,
                              const bool caseSensitive)
     {
-        // !!TODO!! do we need this?
-        if (text.size() == 0 /* || _closing*/)
+        if (text.size() == 0)
         {
             return;
         }

diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
@@ -203,6 +203,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
         TYPED_EVENT(HoveredHyperlinkChanged, IInspectable, IInspectable);
 
+        TYPED_EVENT(RendererEnteredErrorState, IInspectable, IInspectable);
         TYPED_EVENT(SwapChainChanged, IInspectable, IInspectable);
         TYPED_EVENT(RendererWarning, IInspectable, TerminalControl::RendererWarningArgs);
     };

diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp
@@ -88,6 +88,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         _core->BackgroundColorChanged({ get_weak(), &TermControl::_BackgroundColorChangedHandler });
         _core->ScrollPositionChanged({ get_weak(), &TermControl::_ScrollPositionChanged });
         _core->CursorPositionChanged({ get_weak(), &TermControl::_CursorPositionChanged });
+        _core->RendererEnteredErrorState({ get_weak(), &TermControl::_RendererEnteredErrorState });
 
         // Initialize the terminal only once the swapchainpanel is loaded - that
         //      way, we'll be able to query the real pixel size it got on layout
@@ -1087,7 +1088,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
             const auto cursorPosition = point.Position();
             const auto terminalPosition = _GetTerminalPosition(cursorPosition);
-            const auto clickCount = _NumberOfClicks(cursorPosition, point.Timestamp());
+            // const auto clickCount = _NumberOfClicks(cursorPosition, point.Timestamp());
 
             // GH#9396: we prioritize hyper-link over VT mouse events
             //
@@ -1096,6 +1097,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             if (point.Properties().IsLeftButtonPressed() &&
                 ctrlEnabled && !hyperlink.empty())
             {
+                const auto clickCount = _NumberOfClicks(cursorPosition, point.Timestamp());
                 // Handle hyper-link only on the first click to prevent multiple activations
                 if (clickCount == 1)
                 {
@@ -1108,6 +1110,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
             }
             else if (point.Properties().IsLeftButtonPressed())
             {
+                const auto clickCount = _NumberOfClicks(cursorPosition, point.Timestamp());
                 // This formula enables the number of clicks to cycle properly
                 // between single-, double-, and triple-click. To increase the
                 // number of acceptable click states, simply increment
@@ -2584,7 +2587,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
     // Method Description:
     // - Produces the error dialog that notifies the user that rendering cannot proceed.
-    winrt::fire_and_forget TermControl::_RendererEnteredErrorState()
+    winrt::fire_and_forget TermControl::_RendererEnteredErrorState(IInspectable const& /*sender*/,
+                                                                   IInspectable const& /*args*/)
     {
         auto strongThis{ get_strong() };
         co_await Dispatcher(); // pop up onto the UI thread

diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h
@@ -54,7 +54,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
 
         winrt::fire_and_forget RenderEngineSwapChainChanged(const IInspectable& sender, const IInspectable& args);
         void _AttachDxgiSwapChainToXaml(HANDLE swapChainHandle);
-        winrt::fire_and_forget _RendererEnteredErrorState();
+        winrt::fire_and_forget _RendererEnteredErrorState(const IInspectable& sender, const IInspectable& args);
         void _RenderRetryButton_Click(IInspectable const& button, IInspectable const& args);
         winrt::fire_and_forget _RendererWarning(const IInspectable& sender,
                                                 const TerminalControl::RendererWarningArgs& args);

diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
@@ -1196,6 +1196,7 @@ void Terminal::UpdatePatterns() noexcept
 //   visible region is changing
 void Terminal::ClearPatternTree() noexcept
 {
+    auto lock = LockForWriting();
     auto oldTree = _patternIntervalTree;
     _patternIntervalTree = {};
     _InvalidatePatternTree(oldTree);