Show a double width cursor for double width characters (#5319)

# Summary of the Pull Request
This PR will allow the cursor to be double width when on top of a double width character. This required changing `IsCursorDoubleWidth` to check whether the glyph the cursor's on top of is double width. This code is exactly the same as the original PR that addressed this issue in #2932. That one got reverted at some point due to the crashes related to it, but due to a combination of Terminal having come further since that PR and other changes to address use-after-frees, some of the crashes may/may not be relevant now. The ones that seemed to be relevant/repro-able, I attempt to address in this PR.

The `IsCursorDoubleWidth` check would fail during the `TextBuffer::Reflow` call inside of `Terminal::UserResize` occasionally, particularly when `newCursor.EndDeferDrawing()` is called. This is because when we tell the newCursor to `EndDefer`, the renderer will attempt to redraw the cursor. As part of this redraw, it'll ask if `IsCursorDoubleWidth`, and if the renderer managed to ask this before `UserResize` swapped out the old buffer with the new one from `Reflow`, the renderer will be asking the old buffer if its out-of-bounds cursor is double width. This was pretty easily repro'd using `cmatrix -u0` and resizing the window like a madman.

As a solution, I've moved the Start/End DeferDrawing calls out of `Reflow` and into `UserResize`. This way, I can "clamp" the portion of the code where the newBuffer is getting created and reflowed and swapped into the Terminal buffer, and only allow the renderer to draw once the swap is done. This also means that ConHost's `ResizeWithReflow` needed to change slightly.

In addition, I've added a WriteLock to `SetCursorOn`. It was mentioned as a fix for a crash in #2965 (although I can't repro), and I also figured it would be good to try to emulate where ConHost locks with regards to Cursor operations, and this seemed to be one that we were missing.

## PR Checklist
* [x] Closes #2713
* [x] CLA signed
* [x] Tests added/passed

## Validation Steps Performed
Manual validation that the cursor is indeed chonky, added a test case to check that we are correctly saying that the cursor is double width (not too sure if I put it in the right place). Also open to other test case ideas and thoughts on what else I should be careful for since I am quite nervous about what other crashes might occur.

src/buffer/out/textBuffer.cpp

@@ -1967,11 +1967,8 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
                            const std::optional<Viewport> lastCharacterViewport,
                            std::optional<std::reference_wrapper<PositionInformation>> positionInfo)
 {
-    Cursor& oldCursor = oldBuffer.GetCursor();
+    const Cursor& oldCursor = oldBuffer.GetCursor();
     Cursor& newCursor = newBuffer.GetCursor();
-    // skip any drawing updates that might occur as we manipulate the new buffer
-    oldCursor.StartDeferDrawing();
-    newCursor.StartDeferDrawing();
 
     // We need to save the old cursor position so that we can
     // place the new cursor back on the equivalent character in
@@ -2197,8 +2194,5 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
         newCursor.SetSize(ulSize);
     }
 
-    newCursor.EndDeferDrawing();
-    oldCursor.EndDeferDrawing();
-
     return hr;
 }

src/cascadia/TerminalCore/Terminal.cpp

@@ -190,6 +190,11 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
     // bottom in the new buffer as well. Track that case now.
     const bool originalOffsetWasZero = _scrollOffset == 0;
 
+    // skip any drawing updates that might occur until we swap _buffer with the new buffer or if we exit early.
+    _buffer->GetCursor().StartDeferDrawing();
+    // we're capturing _buffer by reference here because when we exit, we want to EndDefer on the current active buffer.
+    auto endDefer = wil::scope_exit([&]() noexcept { _buffer->GetCursor().EndDeferDrawing(); });
+
     // First allocate a new text buffer to take the place of the current one.
     std::unique_ptr<TextBuffer> newTextBuffer;
     try
@@ -199,6 +204,8 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
                                                      0, // temporarily set size to 0 so it won't render.
                                                      _buffer->GetRenderTarget());
 
+        newTextBuffer->GetCursor().StartDeferDrawing();
+
         // Build a PositionInformation to track the position of both the top of
         // the mutable viewport and the top of the visible viewport in the new
         // buffer.
@@ -886,8 +893,9 @@ CATCH_LOG()
 //   Visible, then it will immediately become visible.
 // Arguments:
 // - isVisible: whether the cursor should be visible
-void Terminal::SetCursorOn(const bool isOn) noexcept
+void Terminal::SetCursorOn(const bool isOn)
 {
+    auto lock = LockForWriting();
     _buffer->GetCursor().SetIsOn(isOn);
 }
 

src/cascadia/TerminalCore/Terminal.hpp

@@ -11,6 +11,7 @@
 #include "../../terminal/input/terminalInput.hpp"
 
 #include "../../types/inc/Viewport.hpp"
+#include "../../types/inc/GlyphWidth.hpp"
 #include "../../types/IUiaData.h"
 #include "../../cascadia/terminalcore/ITerminalApi.hpp"
 #include "../../cascadia/terminalcore/ITerminalInput.hpp"
@@ -147,7 +148,7 @@ class Microsoft::Terminal::Core::Terminal final :
     ULONG GetCursorPixelWidth() const noexcept override;
     CursorType GetCursorStyle() const noexcept override;
     COLORREF GetCursorColor() const noexcept override;
-    bool IsCursorDoubleWidth() const noexcept override;
+    bool IsCursorDoubleWidth() const override;
     bool IsScreenReversed() const noexcept override;
     const std::vector<Microsoft::Console::Render::RenderOverlay> GetOverlays() const noexcept override;
     const bool IsGridLineDrawingAllowed() noexcept override;
@@ -171,7 +172,7 @@ class Microsoft::Terminal::Core::Terminal final :
     void SetCursorPositionChangedCallback(std::function<void()> pfn) noexcept;
     void SetBackgroundCallback(std::function<void(const uint32_t)> pfn) noexcept;
 
-    void SetCursorOn(const bool isOn) noexcept;
+    void SetCursorOn(const bool isOn);
     bool IsCursorBlinkingAllowed() const noexcept;
 
 #pragma region TextSelection

src/cascadia/TerminalCore/terminalrenderdata.cpp

@@ -105,9 +105,11 @@ COLORREF Terminal::GetCursorColor() const noexcept
     return _buffer->GetCursor().GetColor();
 }
 
-bool Terminal::IsCursorDoubleWidth() const noexcept
+bool Terminal::IsCursorDoubleWidth() const
 {
-    return false;
+    const auto position = _buffer->GetCursor().GetPosition();
+    TextBufferTextIterator it(TextBufferCellIterator(*_buffer, position));
+    return IsGlyphFullWidth(*it);
 }
 
 const std::vector<RenderOverlay> Terminal::GetOverlays() const noexcept

src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp

@@ -33,6 +33,7 @@ namespace TerminalCoreUnitTests
         // This test ensures that Terminal::_WriteBuffer doesn't get stuck when
         // PrintString() is called with more code units than the buffer width.
         TEST_METHOD(PrintStringOfSurrogatePairs);
+        TEST_METHOD(CheckDoubleWidthCursor);
     };
 };
 
@@ -208,3 +209,45 @@ void TerminalApiTest::CursorVisibilityViaStateMachine()
     VERIFY_IS_FALSE(cursor.IsBlinkingAllowed());
     VERIFY_IS_FALSE(cursor.IsVisible());
 }
+
+void TerminalApiTest::CheckDoubleWidthCursor()
+{
+    DummyRenderTarget renderTarget;
+    Terminal term;
+    term.Create({ 100, 100 }, 0, renderTarget);
+
+    auto& tbi = *(term._buffer);
+    auto& stateMachine = *(term._stateMachine);
+    auto& cursor = tbi.GetCursor();
+
+    // Lets stuff the buffer with single width characters,
+    // but leave the last two columns empty for double width.
+    std::wstring singleWidthText;
+    singleWidthText.reserve(98);
+    for (size_t i = 0; i < 98; ++i)
+    {
+        singleWidthText.append(L"A");
+    }
+    stateMachine.ProcessString(singleWidthText);
+    VERIFY_IS_TRUE(cursor.GetPosition().X == 98);
+
+    // Stuff two double width characters.
+    std::wstring doubleWidthText{ L"我愛" };
+    stateMachine.ProcessString(doubleWidthText);
+
+    // The last 'A'
+    term.SetCursorPosition(97, 0);
+    VERIFY_IS_FALSE(term.IsCursorDoubleWidth());
+
+    // This and the next CursorPos are taken up by '我‘
+    term.SetCursorPosition(98, 0);
+    VERIFY_IS_TRUE(term.IsCursorDoubleWidth());
+    term.SetCursorPosition(99, 0);
+    VERIFY_IS_TRUE(term.IsCursorDoubleWidth());
+
+    // This and the next CursorPos are taken up by ’愛‘
+    term.SetCursorPosition(0, 1);
+    VERIFY_IS_TRUE(term.IsCursorDoubleWidth());
+    term.SetCursorPosition(1, 1);
+    VERIFY_IS_TRUE(term.IsCursorDoubleWidth());
+}

src/host/screenInfo.cpp

@@ -1403,6 +1403,12 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
     // Save cursor's relative height versus the viewport
     SHORT const sCursorHeightInViewportBefore = _textBuffer->GetCursor().GetPosition().Y - _viewport.Top();
 
+    // skip any drawing updates that might occur until we swap _textBuffer with the new buffer or we exit early.
+    newTextBuffer->GetCursor().StartDeferDrawing();
+    _textBuffer->GetCursor().StartDeferDrawing();
+    // we're capturing _textBuffer by reference here because when we exit, we want to EndDefer on the current active buffer.
+    auto endDefer = wil::scope_exit([&]() noexcept { _textBuffer->GetCursor().EndDeferDrawing(); });
+
     HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get(), std::nullopt, std::nullopt);
 
     if (SUCCEEDED(hr))

src/renderer/inc/IRenderData.hpp

@@ -58,7 +58,7 @@ namespace Microsoft::Console::Render
         virtual CursorType GetCursorStyle() const noexcept = 0;
         virtual ULONG GetCursorPixelWidth() const noexcept = 0;
         virtual COLORREF GetCursorColor() const noexcept = 0;
-        virtual bool IsCursorDoubleWidth() const noexcept = 0;
+        virtual bool IsCursorDoubleWidth() const = 0;
 
         virtual bool IsScreenReversed() const noexcept = 0;