Some percent 0-100 of this is garbage

I'm not sure any of this is right. It all feels wrong. The problem we're
  having is that when conhost defers the EOL wrap, our cursor is still at the
  last cell of a row, and we still try and paint it there. That paintCursor,
  right before the end of the frame, is what's screwing us up

  I'm pretty sure the RenderData code to move the cursor ++ when we're at a
  delayed EOL is definitely wrong

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -628,7 +628,7 @@ void ConptyRoundtripTests::MoveCursorAtEOL()
     // might change, but the buffer contents shouldn't.
     // If they do change and these tests break, that's to be expected.
     expectedOutput.push_back(std::string(TerminalViewWidth, 'A'));
-    expectedOutput.push_back("\x1b[1;80H");
+    // expectedOutput.push_back("\x1b[1;80H");
 
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 
@@ -654,8 +654,11 @@ void ConptyRoundtripTests::MoveCursorAtEOL()
 
     verifyData1(hostTb);
 
+    // expectedOutput.push_back(" ");
+    // expectedOutput.push_back("\x1b[1;80H");
+    expectedOutput.push_back("\x08");
     expectedOutput.push_back(" ");
-    expectedOutput.push_back("\x1b[1;80H");
+    expectedOutput.push_back("\x08");
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 
     verifyData1(termTb);

src/host/renderData.cpp

@@ -123,7 +123,12 @@ COORD RenderData::GetCursorPosition() const noexcept
 {
     const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
     const auto& cursor = gci.GetActiveOutputBuffer().GetTextBuffer().GetCursor();
-    return cursor.GetPosition();
+    COORD effectiveCursor = cursor.GetPosition();
+    if (cursor.IsDelayedEOLWrap() && cursor.GetDelayedAtPosition() == effectiveCursor)
+    {
+        effectiveCursor.X++;
+    }
+    return effectiveCursor;
 }
 
 // Method Description:

src/renderer/vt/XtermEngine.cpp

@@ -120,7 +120,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
     }
     // Otherwise, if the cursor previously was visible, and it should be hidden
     // (on -> off), hide it at the end of the frame.
-    else if (!_nextCursorIsVisible && _lastCursorIsVisible)
+    else if (!_nextCursorIsVisible && _lastCursorIsVisible && !_delayedEolWrap)
     {
         RETURN_IF_FAILED(_HideCursor());
     }
@@ -269,20 +269,20 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
                 hr = _Write(seq);
             }
         }
-        else if (_delayedEolWrap)
-        {
-            // GH#1245, GH#357 - If we were in the delayed EOL wrap state, make
-            // sure to _manually_ position the cursor now, with a full CUP
-            // sequence, don't try and be clever with \b or \r or other control
-            // sequences. Different terminals (conhost, gnome-terminal, wt) all
-            // behave differently with how the cursor behaves at an end of line.
-            // This is the only solution that works in all of them, and also
-            // works wrapped lines emitted by conpty.
-            //
-            // Make sure to do this _after_ the possible \r\n branch above,
-            // otherwise we might accidentally break wrapped lines (GH#405)
-            hr = _CursorPosition(coord);
-        }
+        // else if (_delayedEolWrap)
+        // {
+        //     // GH#1245, GH#357 - If we were in the delayed EOL wrap state, make
+        //     // sure to _manually_ position the cursor now, with a full CUP
+        //     // sequence, don't try and be clever with \b or \r or other control
+        //     // sequences. Different terminals (conhost, gnome-terminal, wt) all
+        //     // behave differently with how the cursor behaves at an end of line.
+        //     // This is the only solution that works in all of them, and also
+        //     // works wrapped lines emitted by conpty.
+        //     //
+        //     // Make sure to do this _after_ the possible \r\n branch above,
+        //     // otherwise we might accidentally break wrapped lines (GH#405)
+        //     hr = _CursorPosition(coord);
+        // }
         else if (coord.X == 0 && coord.Y == _lastText.Y)
         {
             // Start of this line
@@ -344,6 +344,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
 // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
 [[nodiscard]] HRESULT XtermEngine::ScrollFrame() noexcept
 {
+    _trace.TraceScrollFrame(_scrollDelta);
+
     if (_scrollDelta.X != 0)
     {
         // No easy way to shift left-right. Everything needs repainting.
@@ -356,38 +358,47 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
     }
 
     const short dy = _scrollDelta.Y;
-    const short absDy = static_cast<short>(abs(dy));
+    // const short absDy = static_cast<short>(abs(dy));
 
     HRESULT hr = S_OK;
-    if (dy < 0)
+
+    _lastText.Y += dy;
+    _trace.TraceLastText(_lastText);
+    if (_wrappedRow.has_value())
     {
-        // Instead of deleting the first line (causing everything to move up)
-        // move to the bottom of the buffer, and newline.
-        //      That will cause everything to move up, by moving the viewport down.
-        // This will let remote conhosts scroll up to see history like normal.
-        const short bottom = _lastViewport.ToOrigin().BottomInclusive();
-        hr = _MoveCursor({ 0, bottom });
-        if (SUCCEEDED(hr))
-        {
-            std::string seq = std::string(absDy, '\n');
-            hr = _Write(seq);
-            // Mark that the bottom line is new, so we won't spend time with an
-            // ECH on it.
-            _newBottomLine = true;
-        }
-        // We don't need to _MoveCursor the cursor again, because it's still
-        //      at the bottom of the viewport.
-    }
-    else if (dy > 0)
-    {
-        // Move to the top of the buffer, and insert some lines of text, to
-        //      cause the viewport contents to shift down.
-        hr = _MoveCursor({ 0, 0 });
-        if (SUCCEEDED(hr))
-        {
-            hr = _InsertLine(absDy);
-        }
+        _wrappedRow.value() += dy;
+        _trace.TraceSetWrapped(_wrappedRow.value());
     }
+    _newBottomLine = true;
+    // if (dy < 0)
+    // {
+    //     // Instead of deleting the first line (causing everything to move up)
+    //     // move to the bottom of the buffer, and newline.
+    //     //      That will cause everything to move up, by moving the viewport down.
+    //     // This will let remote conhosts scroll up to see history like normal.
+    //     const short bottom = _lastViewport.ToOrigin().BottomInclusive();
+    //     hr = _MoveCursor({ 0, bottom });
+    //     if (SUCCEEDED(hr))
+    //     {
+    //         std::string seq = std::string(absDy, '\n');
+    //         hr = _Write(seq);
+    //         // Mark that the bottom line is new, so we won't spend time with an
+    //         // ECH on it.
+    //         _newBottomLine = true;
+    //     }
+    //     // We don't need to _MoveCursor the cursor again, because it's still
+    //     //      at the bottom of the viewport.
+    // }
+    // else if (dy > 0)
+    // {
+    //     // Move to the top of the buffer, and insert some lines of text, to
+    //     //      cause the viewport contents to shift down.
+    //     hr = _MoveCursor({ 0, 0 });
+    //     if (SUCCEEDED(hr))
+    //     {
+    //         hr = _InsertLine(absDy);
+    //     }
+    // }
 
     return hr;
 }

src/renderer/vt/invalidate.cpp

@@ -121,6 +121,7 @@ CATCH_RETURN();
         _circled = true;
     }
 
+    _trace.TraceTriggerCircling(*pForcePaint);
     return S_OK;
 }
 

src/renderer/vt/paint.cpp

@@ -32,7 +32,12 @@ using namespace Microsoft::Console::Types;
                          _titleChanged;
 
     _quickReturn = !somethingToDo;
-    _trace.TraceStartPaint(_quickReturn, _invalidMap, _lastViewport.ToInclusive(), _scrollDelta, _cursorMoved);
+    _trace.TraceStartPaint(_quickReturn,
+                           _invalidMap,
+                           _lastViewport.ToInclusive(),
+                           _scrollDelta,
+                           _cursorMoved,
+                           _wrappedRow);
 
     return _quickReturn ? S_FALSE : S_OK;
 }
@@ -155,8 +160,15 @@ using namespace Microsoft::Console::Types;
 {
     _trace.TracePaintCursor(options.coordCursor);
 
-    // MSFT:15933349 - Send the terminal the updated cursor information, if it's changed.
-    LOG_IF_FAILED(_MoveCursor(options.coordCursor));
+    if (_delayedEolWrap && (options.coordCursor.Y == _lastText.Y && _lastText.X == _lastViewport.RightExclusive()))
+    {
+    }
+    else
+    // if (!_delayedEolWrap)
+    {
+        // MSFT:15933349 - Send the terminal the updated cursor information, if it's changed.
+        LOG_IF_FAILED(_MoveCursor(options.coordCursor));
+    }
 
     return S_OK;
 }
@@ -458,6 +470,7 @@ using namespace Microsoft::Console::Types;
         // the cursor is still waiting on that character for the next character
         // to follow it.
         _wrappedRow = std::nullopt;
+        _trace.TraceClearWrapped();
     }
 
     // Move the cursor to the start of this run.
@@ -479,6 +492,7 @@ using namespace Microsoft::Console::Types;
         lastWrittenChar > _lastViewport.RightInclusive())
     {
         _wrappedRow = coord.Y;
+        _trace.TraceSetWrapped(coord.Y);
     }
 
     // Update our internal tracker of the cursor's position.

src/renderer/vt/tracing.cpp

@@ -131,7 +131,8 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
                                     const til::bitmap invalidMap,
                                     const til::rectangle lastViewport,
                                     const til::point scrollDelt,
-                                    const bool cursorMoved) const
+                                    const bool cursorMoved,
+                                    std::optional<short> wrappedRow) const
 {
 #ifndef UNIT_TESTING
     if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
@@ -142,14 +143,29 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
         const auto lastView = lastViewStr.c_str();
         const auto scrollDeltaStr = scrollDelt.to_string();
         const auto scrollDelta = scrollDeltaStr.c_str();
-        TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
-                          "VtEngine_TraceStartPaint",
-                          TraceLoggingBool(quickReturn),
-                          TraceLoggingWideString(invalidated),
-                          TraceLoggingWideString(lastView),
-                          TraceLoggingWideString(scrollDelta),
-                          TraceLoggingBool(cursorMoved),
-                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+        if (wrappedRow.has_value())
+        {
+            TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
+                              "VtEngine_TraceStartPaint",
+                              TraceLoggingBool(quickReturn),
+                              TraceLoggingWideString(invalidated),
+                              TraceLoggingWideString(lastView),
+                              TraceLoggingWideString(scrollDelta),
+                              TraceLoggingBool(cursorMoved),
+                              TraceLoggingValue(wrappedRow.value()),
+                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+        }
+        else
+        {
+            TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
+                              "VtEngine_TraceStartPaint",
+                              TraceLoggingBool(quickReturn),
+                              TraceLoggingWideString(invalidated),
+                              TraceLoggingWideString(lastView),
+                              TraceLoggingWideString(scrollDelta),
+                              TraceLoggingBool(cursorMoved),
+                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+        }
     }
 #else
     UNREFERENCED_PARAMETER(quickReturn);
@@ -157,6 +173,7 @@ void RenderTracing::TraceStartPaint(const bool quickReturn,
     UNREFERENCED_PARAMETER(lastViewport);
     UNREFERENCED_PARAMETER(scrollDelt);
     UNREFERENCED_PARAMETER(cursorMoved);
+    UNREFERENCED_PARAMETER(wrappedRow);
 #endif UNIT_TESTING
 }
 
@@ -186,6 +203,24 @@ void RenderTracing::TraceLastText(const til::point lastTextPos) const
     UNREFERENCED_PARAMETER(lastTextPos);
 #endif UNIT_TESTING
 }
+
+void RenderTracing::TraceScrollFrame(const til::point scrollDeltaPos) const
+{
+#ifndef UNIT_TESTING
+    if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
+    {
+        const auto scrollDeltaStr = scrollDeltaPos.to_string();
+        const auto scrollDelta = scrollDeltaStr.c_str();
+        TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
+                          "VtEngine_TraceScrollFrame",
+                          TraceLoggingWideString(scrollDelta),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+    }
+#else
+    UNREFERENCED_PARAMETER(scrollDeltaPos);
+#endif UNIT_TESTING
+}
+
 void RenderTracing::TraceMoveCursor(const til::point lastTextPos, const til::point cursor) const
 {
 #ifndef UNIT_TESTING
@@ -224,6 +259,36 @@ void RenderTracing::TraceWrapped() const
 #endif UNIT_TESTING
 }
 
+void RenderTracing::TraceSetWrapped(const short wrappedRow) const
+{
+#ifndef UNIT_TESTING
+    if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
+    {
+        TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
+                          "VtEngine_TraceSetWrapped",
+                          TraceLoggingValue(wrappedRow),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+    }
+#else
+    UNREFERENCED_PARAMETER(wrappedRow);
+#endif UNIT_TESTING
+}
+
+void RenderTracing::TraceClearWrapped() const
+{
+#ifndef UNIT_TESTING
+    if (TraceLoggingProviderEnabled(g_hConsoleVtRendererTraceProvider, WINEVENT_LEVEL_VERBOSE, 0))
+    {
+        const auto* const msg = "Cleared wrap state";
+        TraceLoggingWrite(g_hConsoleVtRendererTraceProvider,
+                          "VtEngine_TraceClearWrapped",
+                          TraceLoggingString(msg),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE));
+    }
+#else
+#endif UNIT_TESTING
+}
+
 void RenderTracing::TracePaintCursor(const til::point coordCursor) const
 {
 #ifndef UNIT_TESTING

src/renderer/vt/tracing.hpp

@@ -29,7 +29,10 @@ namespace Microsoft::Console::VirtualTerminal
         void TraceString(const std::string_view& str) const;
         void TraceInvalidate(const til::rectangle view) const;
         void TraceLastText(const til::point lastText) const;
+        void TraceScrollFrame(const til::point scrollDelta) const;
         void TraceMoveCursor(const til::point lastText, const til::point cursor) const;
+        void TraceSetWrapped(const short wrappedRow) const;
+        void TraceClearWrapped() const;
         void TraceWrapped() const;
         void TracePaintCursor(const til::point coordCursor) const;
         void TraceInvalidateAll(const til::rectangle view) const;
@@ -38,7 +41,8 @@ namespace Microsoft::Console::VirtualTerminal
                              const til::bitmap invalidMap,
                              const til::rectangle lastViewport,
                              const til::point scrollDelta,
-                             const bool cursorMoved) const;
+                             const bool cursorMoved,
+                             std::optional<short> wrappedRow) const;
         void TraceEndPaint() const;
     };
 }