From ba777dfbf6d80f0bbea97945654c8e6c0437cb65 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Wed, 11 Jan 2023 14:05:13 -0600
Subject: [PATCH 1/6] This doesn't seem to flash in debug anymore. Lemme
 double-check release and be sure.

---
 .../ConptyRoundtripTests.cpp                  |  2 ++
 src/host/screenInfo.cpp                       |  5 +++-
 src/renderer/vt/XtermEngine.cpp               |  2 +-
 src/renderer/vt/invalidate.cpp                |  6 +++++
 src/renderer/vt/paint.cpp                     | 24 +++++++++++++++----
 src/renderer/vt/vtrenderer.hpp                |  1 +
 .../parser/OutputStateMachineEngine.cpp       |  1 +
 7 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index fee9dc57fa8..8b02fa346c8 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -234,6 +234,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
     TEST_METHOD(TestNoExtendedAttrsOptimization);
     TEST_METHOD(TestNoBackgroundAttrsOptimization);
 
+    TEST_METHOD(TestShellIntegrationFlickering);
+
 private:
     bool _writeCallback(const char* const pch, const size_t cch);
     void _flushFirstFrame();
diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp
index c6249b0459d..d75c8b608a3 100644
--- a/src/host/screenInfo.cpp
+++ b/src/host/screenInfo.cpp
@@ -2349,7 +2349,10 @@ void SCREEN_INFORMATION::SetTerminalConnection(_In_ VtEngine* const pTtyConnecti
     if (pTtyConnection)
     {
         engine.SetTerminalConnection(pTtyConnection,
-                                     std::bind(&StateMachine::FlushToTerminal, _stateMachine.get()));
+                                     [this]() -> bool {
+                                         ServiceLocator::LocateGlobals().pRender->NotifyPaintFrame();
+                                         return _stateMachine->FlushToTerminal();
+                                     });
     }
     else
     {
diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp
index 13bcca7ab04..08d14497ea7 100644
--- a/src/renderer/vt/XtermEngine.cpp
+++ b/src/renderer/vt/XtermEngine.cpp
@@ -537,7 +537,7 @@ CATCH_RETURN();
     // To fix this, flush here, so this string is sent to the connected terminal
     // application.
 
-    return _Flush();
+    return S_OK; // _Flush();
 }
 
 // Method Description:
diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp
index d6dd1b9db39..5c497d3cf64 100644
--- a/src/renderer/vt/invalidate.cpp
+++ b/src/renderer/vt/invalidate.cpp
@@ -120,6 +120,12 @@ CATCH_RETURN();
         _circled = circled;
     }
 
+    // If we flushed for any reason other than circling, we don't need to push
+    // the buffer out on EndPaint. We're doing this because we encountered some
+    // VT sequence that required we sync the state of the buffers before passing
+    // it through.
+    _noFlushOnEnd = !circled;
+
     _trace.TraceTriggerCircling(*pForcePaint);
 
     return S_OK;
diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index 3eb561f97f6..d9d0bc2cbea 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -43,6 +43,12 @@ using namespace Microsoft::Console::Types;
                            _cursorMoved,
                            _wrappedRow);
 
+    //if (_buffer.size() > 0 && _quickReturn)
+    //{
+    //    RETURN_IF_FAILED(_Flush());
+    //    return S_FALSE;
+    //}
+
     return _quickReturn ? S_FALSE : S_OK;
 }
 
@@ -94,7 +100,14 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
     }
 
-    RETURN_IF_FAILED(_Flush());
+    if (_noFlushOnEnd)
+    {
+        _noFlushOnEnd = false;
+    }
+    else
+    {
+        RETURN_IF_FAILED(_Flush());
+    }
 
     return S_OK;
 }
@@ -538,10 +551,11 @@ using namespace Microsoft::Console::Types;
     // Write the actual text string. If we're using a soft font, the character
     // set should have already been selected, so we just need to map our internal
     // representation back to ASCII (handled by the _WriteTerminalDrcs method).
-    if (_usingSoftFont) [[unlikely]]
-    {
-        RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
-    }
+    if (_usingSoftFont)
+        [[unlikely]]
+        {
+            RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
+        }
     else
     {
         RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8({ _bufferLine.data(), cchActual }));
diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp
index a7cdb7326e7..2bd5eb57f31 100644
--- a/src/renderer/vt/vtrenderer.hpp
+++ b/src/renderer/vt/vtrenderer.hpp
@@ -139,6 +139,7 @@ namespace Microsoft::Console::Render
 
         bool _resizeQuirk{ false };
         bool _passthrough{ false };
+        bool _noFlushOnEnd{ false };
         std::optional<TextColor> _newBottomLineBG{ std::nullopt };
 
         [[nodiscard]] HRESULT _WriteFill(const size_t n, const char c) noexcept;
diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp
index 48625cd5af7..55c2ec39b1b 100644
--- a/src/terminal/parser/OutputStateMachineEngine.cpp
+++ b/src/terminal/parser/OutputStateMachineEngine.cpp
@@ -168,6 +168,7 @@ bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view s
     if (_pTtyConnection != nullptr)
     {
         const auto hr = _pTtyConnection->WriteTerminalW(string);
+        
         LOG_IF_FAILED(hr);
         success = SUCCEEDED(hr);
     }

From 12053fe8b30069f10fcd1b37ef6fdeaa00a68fdd Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Fri, 13 Jan 2023 10:29:08 -0600
Subject: [PATCH 2/6] comments

---
 .../ConptyRoundtripTests.cpp                  |  2 --
 src/host/screenInfo.cpp                       |  4 ++--
 src/renderer/vt/XtermEngine.cpp               | 12 ++++++++---
 src/renderer/vt/paint.cpp                     | 20 ++++++++++---------
 .../parser/OutputStateMachineEngine.cpp       |  2 +-
 5 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
index 8b02fa346c8..fee9dc57fa8 100644
--- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
+++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
@@ -234,8 +234,6 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
     TEST_METHOD(TestNoExtendedAttrsOptimization);
     TEST_METHOD(TestNoBackgroundAttrsOptimization);
 
-    TEST_METHOD(TestShellIntegrationFlickering);
-
 private:
     bool _writeCallback(const char* const pch, const size_t cch);
     void _flushFirstFrame();
diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp
index d75c8b608a3..09f57f581b8 100644
--- a/src/host/screenInfo.cpp
+++ b/src/host/screenInfo.cpp
@@ -2349,9 +2349,9 @@ void SCREEN_INFORMATION::SetTerminalConnection(_In_ VtEngine* const pTtyConnecti
     if (pTtyConnection)
     {
         engine.SetTerminalConnection(pTtyConnection,
-                                     [this]() -> bool {
+                                     [&stateMachine = *_stateMachine]() -> bool {
                                          ServiceLocator::LocateGlobals().pRender->NotifyPaintFrame();
-                                         return _stateMachine->FlushToTerminal();
+                                         return stateMachine.FlushToTerminal();
                                      });
     }
     else
diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp
index 08d14497ea7..7306597919e 100644
--- a/src/renderer/vt/XtermEngine.cpp
+++ b/src/renderer/vt/XtermEngine.cpp
@@ -534,10 +534,16 @@ CATCH_RETURN();
     // decided to pass through would have gotten buffered here until the next
     // actual frame is triggered.
     //
-    // To fix this, flush here, so this string is sent to the connected terminal
-    // application.
+    // Originally, we would _Flush()  here, so this string (and anything buffered) is sent to the connected
+    // terminal application. As of GH#13710, we won't. We'll just buffer the string.
+    //
+    // Now, we've added a NotifyPaintFrame to the state machine's
+    // FlushToTerminal callback. That callback will allow us to add this wstr to
+    // the buffer now, without pushing the entire buffer out to the pipe right
+    // now. The NotifyPaintFrame in that callback will ensure that a Flush
+    // definitely _does_ happen (after this whole string is processed).
 
-    return S_OK; // _Flush();
+    return S_OK;
 }
 
 // Method Description:
diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index d9d0bc2cbea..3f85758904b 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -43,12 +43,6 @@ using namespace Microsoft::Console::Types;
                            _cursorMoved,
                            _wrappedRow);
 
-    //if (_buffer.size() > 0 && _quickReturn)
-    //{
-    //    RETURN_IF_FAILED(_Flush());
-    //    return S_FALSE;
-    //}
-
     return _quickReturn ? S_FALSE : S_OK;
 }
 
@@ -100,7 +94,15 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
     }
 
+    // If this frame was triggered because we encoutnered a VT sequence which
+    // required the buffered state to get printed, we don't want to flush this
+    // frame to the pipe. That might result in us rendering half the output of a
+    // particular frame (as emitted by the client).
+    //
+    // Instead, we'll leave this frame in _bufffer, and just keep appending to
+    // it as needed.
     if (_noFlushOnEnd)
+        [[unlikely]]
     {
         _noFlushOnEnd = false;
     }
@@ -553,9 +555,9 @@ using namespace Microsoft::Console::Types;
     // representation back to ASCII (handled by the _WriteTerminalDrcs method).
     if (_usingSoftFont)
         [[unlikely]]
-        {
-            RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
-        }
+    {
+        RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
+    }
     else
     {
         RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8({ _bufferLine.data(), cchActual }));
diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp
index 55c2ec39b1b..52689265e11 100644
--- a/src/terminal/parser/OutputStateMachineEngine.cpp
+++ b/src/terminal/parser/OutputStateMachineEngine.cpp
@@ -168,7 +168,7 @@ bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view s
     if (_pTtyConnection != nullptr)
     {
         const auto hr = _pTtyConnection->WriteTerminalW(string);
-        
+
         LOG_IF_FAILED(hr);
         success = SUCCEEDED(hr);
     }

From 9bb2bcbd1960c4b6e57244ebc5c832d7c024b3c0 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Fri, 13 Jan 2023 10:39:48 -0600
Subject: [PATCH 3/6] spel

---
 src/renderer/vt/paint.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index 3f85758904b..b5c8946fa0b 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -94,18 +94,18 @@ using namespace Microsoft::Console::Types;
         RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
     }
 
-    // If this frame was triggered because we encoutnered a VT sequence which
+    // If this frame was triggered because we encountered a VT sequence which
     // required the buffered state to get printed, we don't want to flush this
     // frame to the pipe. That might result in us rendering half the output of a
     // particular frame (as emitted by the client).
     //
-    // Instead, we'll leave this frame in _bufffer, and just keep appending to
+    // Instead, we'll leave this frame in _buffer, and just keep appending to
     // it as needed.
     if (_noFlushOnEnd)
         [[unlikely]]
-    {
-        _noFlushOnEnd = false;
-    }
+        {
+            _noFlushOnEnd = false;
+        }
     else
     {
         RETURN_IF_FAILED(_Flush());
@@ -555,9 +555,9 @@ using namespace Microsoft::Console::Types;
     // representation back to ASCII (handled by the _WriteTerminalDrcs method).
     if (_usingSoftFont)
         [[unlikely]]
-    {
-        RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
-    }
+        {
+            RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
+        }
     else
     {
         RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8({ _bufferLine.data(), cchActual }));

From f440101bf443fe8a3e0f40eb061a7d602939e19f Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Fri, 13 Jan 2023 11:32:06 -0600
Subject: [PATCH 4/6] format doesn't like this does it

---
 src/renderer/vt/paint.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index b5c8946fa0b..9a0c92aac7b 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -103,9 +103,9 @@ using namespace Microsoft::Console::Types;
     // it as needed.
     if (_noFlushOnEnd)
         [[unlikely]]
-        {
-            _noFlushOnEnd = false;
-        }
+    {
+        _noFlushOnEnd = false;
+    }
     else
     {
         RETURN_IF_FAILED(_Flush());
@@ -555,9 +555,9 @@ using namespace Microsoft::Console::Types;
     // representation back to ASCII (handled by the _WriteTerminalDrcs method).
     if (_usingSoftFont)
         [[unlikely]]
-        {
-            RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
-        }
+    {
+        RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
+    }
     else
     {
         RETURN_IF_FAILED(VtEngine::_WriteTerminalUtf8({ _bufferLine.data(), cchActual }));

From c2fc9314c5f91d0b01918e0f7dd152501a60f9a6 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Tue, 25 Apr 2023 14:46:37 -0500
Subject: [PATCH 5/6] reword comments for clarity

---
 src/renderer/vt/XtermEngine.cpp               | 23 +++++++------------
 src/renderer/vt/invalidate.cpp                |  6 ++---
 src/renderer/vt/paint.cpp                     |  6 ++---
 .../parser/OutputStateMachineEngine.cpp       |  1 -
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp
index dda34fbe9a2..ef1776031b8 100644
--- a/src/renderer/vt/XtermEngine.cpp
+++ b/src/renderer/vt/XtermEngine.cpp
@@ -526,22 +526,15 @@ CATCH_RETURN();
     RETURN_IF_FAILED(_fUseAsciiOnly ?
                          VtEngine::_WriteTerminalAscii(wstr) :
                          VtEngine::_WriteTerminalUtf8(wstr));
-    // GH#4106, GH#2011 - WriteTerminalW is only ever called by the
+    // GH#4106, GH#2011, GH#13710 - WriteTerminalW is only ever called by the
     // StateMachine, when we've encountered a string we don't understand. When
-    // this happens, we usually don't actually trigger another frame, but we
-    // _do_ want this string to immediately be sent to the terminal. Since we
-    // only flush our buffer on actual frames, this means that strings we've
-    // decided to pass through would have gotten buffered here until the next
-    // actual frame is triggered.
-    //
-    // Originally, we would _Flush()  here, so this string (and anything buffered) is sent to the connected
-    // terminal application. As of GH#13710, we won't. We'll just buffer the string.
-    //
-    // Now, we've added a NotifyPaintFrame to the state machine's
-    // FlushToTerminal callback. That callback will allow us to add this wstr to
-    // the buffer now, without pushing the entire buffer out to the pipe right
-    // now. The NotifyPaintFrame in that callback will ensure that a Flush
-    // definitely _does_ happen (after this whole string is processed).
+    // this happens, we will trigger a new frame in the renderer, and
+    // immediately buffer this wstr (representing the sequence we didn't
+    // understand). We won't immediately _Flush to the terminal - that might
+    // cause flickering (where we've buffered some state but not the whole
+    // "frame" as specified by the app). We'll just immediately buffer this
+    // sequence, and fluch it when the render thread comes around to paint the
+    // frame normally.
 
     return S_OK;
 }
diff --git a/src/renderer/vt/invalidate.cpp b/src/renderer/vt/invalidate.cpp
index 5c497d3cf64..1c7f3fe498f 100644
--- a/src/renderer/vt/invalidate.cpp
+++ b/src/renderer/vt/invalidate.cpp
@@ -120,10 +120,8 @@ CATCH_RETURN();
         _circled = circled;
     }
 
-    // If we flushed for any reason other than circling, we don't need to push
-    // the buffer out on EndPaint. We're doing this because we encountered some
-    // VT sequence that required we sync the state of the buffers before passing
-    // it through.
+    // If we flushed for any reason other than circling (i.e, a sequence that we
+    // didn't understand), we don't need to push the buffer out on EndPaint.
     _noFlushOnEnd = !circled;
 
     _trace.TraceTriggerCircling(*pForcePaint);
diff --git a/src/renderer/vt/paint.cpp b/src/renderer/vt/paint.cpp
index 738d1e2c7ed..dda813b92fe 100644
--- a/src/renderer/vt/paint.cpp
+++ b/src/renderer/vt/paint.cpp
@@ -101,8 +101,7 @@ using namespace Microsoft::Console::Types;
     //
     // Instead, we'll leave this frame in _buffer, and just keep appending to
     // it as needed.
-    if (_noFlushOnEnd)
-        [[unlikely]]
+    if (_noFlushOnEnd) [[unlikely]]
     {
         _noFlushOnEnd = false;
     }
@@ -553,8 +552,7 @@ using namespace Microsoft::Console::Types;
     // Write the actual text string. If we're using a soft font, the character
     // set should have already been selected, so we just need to map our internal
     // representation back to ASCII (handled by the _WriteTerminalDrcs method).
-    if (_usingSoftFont)
-        [[unlikely]]
+    if (_usingSoftFont) [[unlikely]]
     {
         RETURN_IF_FAILED(VtEngine::_WriteTerminalDrcs({ _bufferLine.data(), cchActual }));
     }
diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp
index 0031b5e9f96..56eb7b0f091 100644
--- a/src/terminal/parser/OutputStateMachineEngine.cpp
+++ b/src/terminal/parser/OutputStateMachineEngine.cpp
@@ -183,7 +183,6 @@ bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view s
     if (_pTtyConnection != nullptr)
     {
         const auto hr = _pTtyConnection->WriteTerminalW(string);
-
         LOG_IF_FAILED(hr);
         success = SUCCEEDED(hr);
     }

From 392d5c5cf96d5b826081331faff7093d576ca587 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Tue, 25 Apr 2023 14:49:19 -0500
Subject: [PATCH 6/6] fluch

---
 src/renderer/vt/XtermEngine.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp
index ef1776031b8..413779b0479 100644
--- a/src/renderer/vt/XtermEngine.cpp
+++ b/src/renderer/vt/XtermEngine.cpp
@@ -533,7 +533,7 @@ CATCH_RETURN();
     // understand). We won't immediately _Flush to the terminal - that might
     // cause flickering (where we've buffered some state but not the whole
     // "frame" as specified by the app). We'll just immediately buffer this
-    // sequence, and fluch it when the render thread comes around to paint the
+    // sequence, and flush it when the render thread comes around to paint the
     // frame normally.
 
     return S_OK;