Make sure DCS strings are flushed to conpty without delay (#17195)

When the `DCS` passthrough code was first implemented, it relied on the
`ActionPassThroughString` method flushing the given string immediately.
However, that has since stopped being the case, so `DCS` operations end
up being delayed until the entire sequence has been parsed.

This PR fixes the issue by introducing a `flush` parameter to force an
immediate flush on the `ActionPassThroughString` method, as well as the
`XtermEngine::WriteTerminalW` method that it calls.

## Validation Steps Performed

I've confirmed that the test case in issue #17111 now updates the color
table as soon as each color entry is parsed, instead of delaying the
updates until the end of the sequence.

Closes #17111

(cherry picked from commit 9c16c5ca82)
Service-Card-Id: 92501226
Service-Version: 1.21

build/pgo/Terminal.PGO.props

@@ -9,7 +9,7 @@
 
   <PropertyGroup>
     <!-- Optional, defaults to main.  Name of the branch which will be used for calculating branch point. -->
-    <PGOBranch>main</PGOBranch>
+    <PGOBranch>release-1.21</PGOBranch>
 
     <!-- Mandatory.  Name of the NuGet package which will contain PGO databases for consumption by build system. -->
     <PGOPackageName>Microsoft.Internal.Windows.Terminal.PGODatabase</PGOPackageName>

src/cascadia/TerminalControl/TermControl.cpp

@@ -1120,7 +1120,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             co_return;
         }
 
-        const auto hr = args.Result();
+        // HRESULT is a signed 32-bit integer which would result in a hex output like "-0x7766FFF4",
+        // but canonically HRESULTs are displayed unsigned as "0x8899000C". See GH#11556.
+        const auto hr = std::bit_cast<uint32_t>(args.Result());
         const auto parameter = args.Parameter();
         winrt::hstring message;
 

src/cascadia/TerminalSettingsEditor/Resources/es-ES/Resources.resw

@@ -733,10 +733,16 @@
     <value>Examinar...</value>
     <comment>Button label that opens a file picker in a new window. The "..." is standard to mean it will open a new window.</comment>
   </data>
+  <data name="Profile_AddFontAxisButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Agregar nuevo eje de fuente</value>
+  </data>
   <data name="Profile_AddNewFontAxis.Text" xml:space="preserve">
     <value>Agregar nuevo</value>
     <comment>Button label that adds a new font axis for the current font.</comment>
   </data>
+  <data name="Profile_AddFontFeatureButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Agregar nueva característica de fuente</value>
+  </data>
   <data name="Profile_AddNewFontFeature.Text" xml:space="preserve">
     <value>Agregar nuevo</value>
     <comment>Button label that adds a new font feature for the current font.</comment>

src/cascadia/TerminalSettingsEditor/Resources/it-IT/Resources.resw

@@ -733,10 +733,16 @@
     <value>Sfoglia...</value>
     <comment>Button label that opens a file picker in a new window. The "..." is standard to mean it will open a new window.</comment>
   </data>
+  <data name="Profile_AddFontAxisButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Aggiungi nuovo asse dei tipi di carattere</value>
+  </data>
   <data name="Profile_AddNewFontAxis.Text" xml:space="preserve">
     <value>Aggiungi nuovo</value>
     <comment>Button label that adds a new font axis for the current font.</comment>
   </data>
+  <data name="Profile_AddFontFeatureButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Aggiungi nuova funzionalità dei tipi di carattere</value>
+  </data>
   <data name="Profile_AddNewFontFeature.Text" xml:space="preserve">
     <value>Aggiungi nuovo</value>
     <comment>Button label that adds a new font feature for the current font.</comment>

src/cascadia/TerminalSettingsEditor/Resources/ja-JP/Resources.resw

@@ -733,10 +733,16 @@
     <value>参照...</value>
     <comment>Button label that opens a file picker in a new window. The "..." is standard to mean it will open a new window.</comment>
   </data>
+  <data name="Profile_AddFontAxisButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>新しいフォント軸の追加</value>
+  </data>
   <data name="Profile_AddNewFontAxis.Text" xml:space="preserve">
     <value>新規追加</value>
     <comment>Button label that adds a new font axis for the current font.</comment>
   </data>
+  <data name="Profile_AddFontFeatureButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>新しいフォント機能の追加</value>
+  </data>
   <data name="Profile_AddNewFontFeature.Text" xml:space="preserve">
     <value>新規追加</value>
     <comment>Button label that adds a new font feature for the current font.</comment>

src/cascadia/TerminalSettingsEditor/Resources/ko-KR/Resources.resw

@@ -733,10 +733,16 @@
     <value>찾아보기...</value>
     <comment>Button label that opens a file picker in a new window. The "..." is standard to mean it will open a new window.</comment>
   </data>
+  <data name="Profile_AddFontAxisButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>새 글꼴 추가 축</value>
+  </data>
   <data name="Profile_AddNewFontAxis.Text" xml:space="preserve">
     <value>새로 추가</value>
     <comment>Button label that adds a new font axis for the current font.</comment>
   </data>
+  <data name="Profile_AddFontFeatureButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>새 글꼴 추가 기능</value>
+  </data>
   <data name="Profile_AddNewFontFeature.Text" xml:space="preserve">
     <value>새로 추가</value>
     <comment>Button label that adds a new font feature for the current font.</comment>

src/cascadia/TerminalSettingsEditor/Resources/pt-BR/Resources.resw

@@ -733,10 +733,16 @@
     <value>Procurar...</value>
     <comment>Button label that opens a file picker in a new window. The "..." is standard to mean it will open a new window.</comment>
   </data>
+  <data name="Profile_AddFontAxisButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Adicionar novo eixo de fonte</value>
+  </data>
   <data name="Profile_AddNewFontAxis.Text" xml:space="preserve">
     <value>Adicionar novo</value>
     <comment>Button label that adds a new font axis for the current font.</comment>
   </data>
+  <data name="Profile_AddFontFeatureButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
+    <value>Adicionar novo recurso de fonte</value>
+  </data>
   <data name="Profile_AddNewFontFeature.Text" xml:space="preserve">
     <value>Adicionar novo</value>
     <comment>Button label that adds a new font feature for the current font.</comment>

src/renderer/atlas/AtlasEngine.r.cpp

@@ -55,7 +55,7 @@ catch (const wil::ResultException& exception)
 {
     const auto hr = exception.GetErrorCode();
 
-    if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET)
+    if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET || hr == D2DERR_RECREATE_TARGET)
     {
         _p.dxgi = {};
         return E_PENDING;

src/renderer/atlas/BackendD2D.cpp

@@ -148,11 +148,11 @@ void BackendD2D::_handleSettingsUpdate(const RenderingPayload& p)
     _viewportCellCount = p.s->viewportCellCount;
 }
 
-void BackendD2D::_drawBackground(const RenderingPayload& p) noexcept
+void BackendD2D::_drawBackground(const RenderingPayload& p)
 {
     if (_backgroundBitmapGeneration != p.colorBitmapGenerations[0])
     {
-        _backgroundBitmap->CopyFromMemory(nullptr, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32)));
+        THROW_IF_FAILED(_backgroundBitmap->CopyFromMemory(nullptr, p.backgroundBitmap.data(), gsl::narrow_cast<UINT32>(p.colorBitmapRowStride * sizeof(u32))));
         _backgroundBitmapGeneration = p.colorBitmapGenerations[0];
     }
 
@@ -356,7 +356,7 @@ f32r BackendD2D::_getGlyphRunDesignBounds(const DWRITE_GLYPH_RUN& glyphRun, f32
         _glyphMetrics = Buffer<DWRITE_GLYPH_METRICS>{ size };
     }
 
-    glyphRun.fontFace->GetDesignGlyphMetrics(glyphRun.glyphIndices, glyphRun.glyphCount, _glyphMetrics.data(), false);
+    THROW_IF_FAILED(glyphRun.fontFace->GetDesignGlyphMetrics(glyphRun.glyphIndices, glyphRun.glyphCount, _glyphMetrics.data(), false));
 
     const f32 fontScale = glyphRun.fontEmSize / fontMetrics.designUnitsPerEm;
     f32r accumulatedBounds{
@@ -541,7 +541,8 @@ void BackendD2D::_resizeCursorBitmap(const RenderingPayload& p, const til::size
     const D2D1_SIZE_F sizeF{ static_cast<f32>(newSizeInPx.width), static_cast<f32>(newSizeInPx.height) };
     const D2D1_SIZE_U sizeU{ gsl::narrow_cast<UINT32>(newSizeInPx.width), gsl::narrow_cast<UINT32>(newSizeInPx.height) };
     wil::com_ptr<ID2D1BitmapRenderTarget> cursorRenderTarget;
-    _renderTarget->CreateCompatibleRenderTarget(&sizeF, &sizeU, nullptr, D2D1_COMPATIBLE_RENDER_TARGET_OPTIONS_NONE, cursorRenderTarget.addressof());
+    THROW_IF_FAILED(_renderTarget->CreateCompatibleRenderTarget(&sizeF, &sizeU, nullptr, D2D1_COMPATIBLE_RENDER_TARGET_OPTIONS_NONE, cursorRenderTarget.addressof()));
+
     cursorRenderTarget->SetAntialiasMode(D2D1_ANTIALIAS_MODE_ALIASED);
 
     cursorRenderTarget->BeginDraw();
@@ -553,7 +554,7 @@ void BackendD2D::_resizeCursorBitmap(const RenderingPayload& p, const til::size
     }
     THROW_IF_FAILED(cursorRenderTarget->EndDraw());
 
-    cursorRenderTarget->GetBitmap(_cursorBitmap.put());
+    THROW_IF_FAILED(cursorRenderTarget->GetBitmap(_cursorBitmap.put()));
     _cursorBitmapSize = newSize;
 }
 

src/renderer/atlas/BackendD2D.h

@@ -17,7 +17,7 @@ namespace Microsoft::Console::Render::Atlas
 
     private:
         ATLAS_ATTR_COLD void _handleSettingsUpdate(const RenderingPayload& p);
-        void _drawBackground(const RenderingPayload& p) noexcept;
+        void _drawBackground(const RenderingPayload& p);
         void _drawText(RenderingPayload& p);
         ATLAS_ATTR_COLD f32 _drawTextPrepareLineRendition(const RenderingPayload& p, const ShapedRow* row, f32 baselineY) const noexcept;
         ATLAS_ATTR_COLD void _drawTextResetLineRendition(const ShapedRow* row) const noexcept;

src/renderer/vt/XtermEngine.cpp

@@ -37,7 +37,16 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
 //      the pipe.
 [[nodiscard]] HRESULT XtermEngine::StartPaint() noexcept
 {
-    RETURN_IF_FAILED(VtEngine::StartPaint());
+    const auto hr = VtEngine::StartPaint();
+    if (hr != S_OK)
+    {
+        // If _noFlushOnEnd was set, and we didn't return early, it would usually
+        // have been reset in the EndPaint call. But since that's not going to
+        // happen now, we need to reset it here, otherwise we may mistakenly skip
+        // the flush on the next frame.
+        _noFlushOnEnd = false;
+        return hr;
+    }
 
     _trace.TraceLastText(_lastText);
 
@@ -83,15 +92,6 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
         }
     }
 
-    if (!_quickReturn)
-    {
-        if (_WillWriteSingleChar())
-        {
-            // Don't re-enable the cursor.
-            _quickReturn = true;
-        }
-    }
-
     return S_OK;
 }
 
@@ -520,9 +520,10 @@ CATCH_RETURN();
 //      proper utf-8 string, depending on our mode.
 // Arguments:
 // - wstr - wstring of text to be written
+// - flush - set to true if the string should be flushed immediately
 // Return Value:
 // - S_OK or suitable HRESULT error from either conversion or writing pipe.
-[[nodiscard]] HRESULT XtermEngine::WriteTerminalW(const std::wstring_view wstr) noexcept
+[[nodiscard]] HRESULT XtermEngine::WriteTerminalW(const std::wstring_view wstr, const bool flush) noexcept
 {
     RETURN_IF_FAILED(_fUseAsciiOnly ?
                          VtEngine::_WriteTerminalAscii(wstr) :
@@ -535,8 +536,11 @@ CATCH_RETURN();
     // 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 flush it when the render thread comes around to paint the
-    // frame normally.
-
+    // frame normally, unless a flush has been explicitly requested.
+    if (flush)
+    {
+        _flushImpl();
+    }
     return S_OK;
 }
 

src/renderer/vt/XtermEngine.hpp

@@ -51,7 +51,7 @@ namespace Microsoft::Console::Render
 
         [[nodiscard]] HRESULT InvalidateScroll(const til::point* const pcoordDelta) noexcept override;
 
-        [[nodiscard]] HRESULT WriteTerminalW(const std::wstring_view str) noexcept override;
+        [[nodiscard]] HRESULT WriteTerminalW(const std::wstring_view str, const bool flush) noexcept override;
 
         [[nodiscard]] HRESULT SetWindowVisibility(const bool showOrHide) noexcept override;
 

src/renderer/vt/invalidate.cpp

@@ -75,7 +75,7 @@ CATCH_RETURN();
     }
     _skipCursor = false;
 
-    _cursorMoved = true;
+    _cursorMoved = psrRegion->origin() != _lastText;
     return S_OK;
 }
 

src/renderer/vt/math.cpp

@@ -52,38 +52,3 @@ void VtEngine::_OrRect(_Inout_ til::inclusive_rect* const pRectExisting, const t
     pRectExisting->right = std::max(pRectExisting->right, pRectToOr->right);
     pRectExisting->bottom = std::max(pRectExisting->bottom, pRectToOr->bottom);
 }
-
-// Method Description:
-// - Returns true if the invalidated region indicates that we only need to
-//      simply print text from the current cursor position. This will prevent us
-//      from sending extra VT set-up/tear down sequences (?12h/l) when all we
-//      need to do is print more text at the current cursor position.
-// Arguments:
-// - <none>
-// Return Value:
-// - true iff only the next character is invalid
-bool VtEngine::_WillWriteSingleChar() const
-{
-    // If there is no scroll delta, return false.
-    if (til::point{ 0, 0 } != _scrollDelta)
-    {
-        return false;
-    }
-
-    // If there is more than one invalid char, return false.
-    if (!_invalidMap.one())
-    {
-        return false;
-    }
-
-    // Get the single point at which things are invalid.
-    const auto invalidPoint = _invalidMap.runs().front().origin();
-
-    // Either the next character to the right or the immediately previous
-    //      character should follow this code path
-    //      (The immediate previous character would suggest a backspace)
-    auto invalidIsNext = invalidPoint == _lastText;
-    auto invalidIsLast = invalidPoint == til::point{ _lastText.x - 1, _lastText.y };
-
-    return invalidIsNext || invalidIsLast;
-}

src/renderer/vt/paint.cpp

@@ -20,30 +20,30 @@ using namespace Microsoft::Console::Types;
 //      HRESULT error code if painting didn't start successfully.
 [[nodiscard]] HRESULT VtEngine::StartPaint() noexcept
 {
+    // When unit testing, there may be no pipe, but we still need to paint.
     if (!_hFile)
     {
-        return S_FALSE;
+        return S_OK;
     }
 
     // If we're using line renditions, and this is a full screen paint, we can
     // potentially stop using them at the end of this frame.
     _stopUsingLineRenditions = _usingLineRenditions && _AllIsInvalid();
 
-    // If there's nothing to do, quick return
+    // If there's nothing to do, we won't need to paint.
     auto somethingToDo = _invalidMap.any() ||
                          _scrollDelta != til::point{ 0, 0 } ||
                          _cursorMoved ||
                          _titleChanged;
 
-    _quickReturn = !somethingToDo;
-    _trace.TraceStartPaint(_quickReturn,
+    _trace.TraceStartPaint(!somethingToDo,
                            _invalidMap,
                            _lastViewport.ToExclusive(),
                            _scrollDelta,
                            _cursorMoved,
                            _wrappedRow);
 
-    return _quickReturn ? S_FALSE : S_OK;
+    return somethingToDo ? S_OK : S_FALSE;
 }
 
 // Routine Description:
@@ -142,9 +142,9 @@ using namespace Microsoft::Console::Types;
         _usingLineRenditions = true;
     }
     // One simple optimization is that we can skip sending the line attributes
-    // when _quickReturn is true. That indicates that we're writing out a single
-    // character, which should preclude there being a rendition switch.
-    if (_usingLineRenditions && !_quickReturn)
+    // when we're writing out a single character, which should preclude there
+    // being a rendition switch.
+    if (_usingLineRenditions && !_invalidMap.one())
     {
         RETURN_IF_FAILED(_MoveCursor({ _lastText.x, targetRow }));
         switch (lineRendition)

src/renderer/vt/state.cpp

@@ -37,7 +37,6 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
     _pool(til::pmr::get_default_resource()),
     _invalidMap(initialViewport.Dimensions(), false, &_pool),
     _scrollDelta(0, 0),
-    _quickReturn(false),
     _clearedAllThisFrame(false),
     _cursorMoved(false),
     _resized(false),

src/renderer/vt/vtrenderer.hpp

@@ -78,7 +78,7 @@ namespace Microsoft::Console::Render
         [[nodiscard]] HRESULT RequestCursor() noexcept;
         [[nodiscard]] HRESULT InheritCursor(const til::point coordCursor) noexcept;
         [[nodiscard]] HRESULT WriteTerminalUtf8(const std::string_view str) noexcept;
-        [[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring_view str) noexcept = 0;
+        [[nodiscard]] virtual HRESULT WriteTerminalW(const std::wstring_view str, const bool flush = false) noexcept = 0;
         void SetTerminalOwner(Microsoft::Console::VirtualTerminal::VtIo* const terminalOwner);
         void SetResizeQuirk(const bool resizeQuirk);
         void SetLookingForDSRCallback(std::function<void(bool)> pfnLooking) noexcept;
@@ -113,7 +113,6 @@ namespace Microsoft::Console::Render
         til::point _lastText;
         til::point _scrollDelta;
 
-        bool _quickReturn;
         bool _clearedAllThisFrame;
         bool _cursorMoved;
         bool _resized;
@@ -214,8 +213,6 @@ namespace Microsoft::Console::Render
         [[nodiscard]] HRESULT _RgbUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept;
         [[nodiscard]] HRESULT _16ColorUpdateDrawingBrushes(const TextAttribute& textAttributes) noexcept;
 
-        bool _WillWriteSingleChar() const;
-
         // buffer space for these two functions to build their lines
         // so they don't have to alloc/free in a tight loop
         std::wstring _bufferLine;

src/terminal/adapter/adaptDispatch.cpp

@@ -4882,7 +4882,7 @@ ITermDispatch::StringHandler AdaptDispatch::_CreatePassthroughHandler()
                 {
                     buffer += L'\\';
                 }
-                engine.ActionPassThroughString(buffer);
+                engine.ActionPassThroughString(buffer, true);
                 buffer.clear();
             }
             return !endOfString;

src/terminal/parser/IStateMachineEngine.hpp

@@ -35,7 +35,7 @@ namespace Microsoft::Console::VirtualTerminal
         virtual bool ActionPrint(const wchar_t wch) = 0;
         virtual bool ActionPrintString(const std::wstring_view string) = 0;
 
-        virtual bool ActionPassThroughString(const std::wstring_view string) = 0;
+        virtual bool ActionPassThroughString(const std::wstring_view string, const bool flush = false) = 0;
 
         virtual bool ActionEscDispatch(const VTID id) = 0;
         virtual bool ActionVt52EscDispatch(const VTID id, const VTParameters parameters) = 0;

src/terminal/parser/InputStateMachineEngine.cpp

@@ -276,9 +276,10 @@ bool InputStateMachineEngine::ActionPrintString(const std::wstring_view string)
 //      string of characters given.
 // Arguments:
 // - string - string to dispatch.
+// - flush - not applicable to the input state machine.
 // Return Value:
 // - true iff we successfully dispatched the sequence.
-bool InputStateMachineEngine::ActionPassThroughString(const std::wstring_view string)
+bool InputStateMachineEngine::ActionPassThroughString(const std::wstring_view string, const bool /*flush*/)
 {
     if (_pDispatch->IsVtInputEnabled())
     {

src/terminal/parser/InputStateMachineEngine.hpp

@@ -142,7 +142,7 @@ namespace Microsoft::Console::VirtualTerminal
 
         bool ActionPrintString(const std::wstring_view string) override;
 
-        bool ActionPassThroughString(const std::wstring_view string) override;
+        bool ActionPassThroughString(const std::wstring_view string, const bool flush) override;
 
         bool ActionEscDispatch(const VTID id) override;
 

src/terminal/parser/OutputStateMachineEngine.cpp

@@ -180,14 +180,15 @@ bool OutputStateMachineEngine::ActionPrintString(const std::wstring_view string)
 //      we don't know what to do with it)
 // Arguments:
 // - string - string to dispatch.
+// - flush - set to true if the string should be flushed immediately.
 // Return Value:
 // - true iff we successfully dispatched the sequence.
-bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view string)
+bool OutputStateMachineEngine::ActionPassThroughString(const std::wstring_view string, const bool flush)
 {
     auto success = true;
     if (_pTtyConnection != nullptr)
     {
-        const auto hr = _pTtyConnection->WriteTerminalW(string);
+        const auto hr = _pTtyConnection->WriteTerminalW(string, flush);
         LOG_IF_FAILED(hr);
         success = SUCCEEDED(hr);
     }

src/terminal/parser/OutputStateMachineEngine.hpp

@@ -38,7 +38,7 @@ namespace Microsoft::Console::VirtualTerminal
 
         bool ActionPrintString(const std::wstring_view string) override;
 
-        bool ActionPassThroughString(const std::wstring_view string) override;
+        bool ActionPassThroughString(const std::wstring_view string, const bool flush) override;
 
         bool ActionEscDispatch(const VTID id) override;
 

src/terminal/parser/ut_parser/StateMachineTest.cpp

@@ -59,7 +59,7 @@ public:
         return true;
     };
 
-    bool ActionPassThroughString(const std::wstring_view string) override
+    bool ActionPassThroughString(const std::wstring_view string, const bool /*flush*/) override
     {
         passedThrough += string;
         return true;