diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index c34f03a2018..73edbe9825a 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -2444,6 +2444,7 @@ TREX triaged triaging TRIANGLESTRIP +Tribool TRIMZEROHEADINGS truetype trx diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 71f45d42500..6a1622f5ea1 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -90,7 +90,29 @@ class Microsoft::Console::Render::VtRendererTest void TestPaint(VtEngine& engine, std::function pfn); Viewport SetUpViewport(); - void VerifyExpectedInputsDrained(); + void VerifyFirstPaint(VtEngine& engine) + { + // Verify the first BeginPaint emits a clear and go home + qExpectedInput.push_back("\x1b[2J"); + // Verify the first EndPaint sets the cursor state + qExpectedInput.push_back("\x1b[?25l"); + VERIFY_IS_TRUE(engine._firstPaint); + TestPaint(engine, [&]() { + VERIFY_IS_FALSE(engine._firstPaint); + }); + } + + void VerifyExpectedInputsDrained() + { + if (!qExpectedInput.empty()) + { + for (const auto& exp : qExpectedInput) + { + Log::Error(NoThrowString().Format(L"EXPECTED INPUT NEVER RECEIVED: %hs", exp.c_str())); + } + VERIFY_FAIL(L"there should be no remaining un-drained expected input"); + } + } }; Viewport VtRendererTest::SetUpViewport() @@ -103,18 +125,6 @@ Viewport VtRendererTest::SetUpViewport() return Viewport::FromInclusive(view); } -void VtRendererTest::VerifyExpectedInputsDrained() -{ - if (!qExpectedInput.empty()) - { - for (const auto& exp : qExpectedInput) - { - Log::Error(NoThrowString().Format(L"EXPECTED INPUT NEVER RECEIVED: %hs", exp.c_str())); - } - VERIFY_FAIL(L"there should be no remaining un-drained expected input"); - } -} - bool VtRendererTest::WriteCallback(const char* const pch, size_t const cch) { std::string actualString = std::string(pch, cch); @@ -212,12 +222,7 @@ void VtRendererTest::Xterm256TestInvalidate() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); const Viewport view = SetUpViewport(); @@ -402,12 +407,7 @@ void VtRendererTest::Xterm256TestColors() RenderSettings renderSettings; RenderData renderData; - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -585,12 +585,7 @@ void VtRendererTest::Xterm256TestCursor() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -766,12 +761,7 @@ void VtRendererTest::Xterm256TestExtendedAttributes() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -907,12 +897,7 @@ void VtRendererTest::XtermTestInvalidate() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -1096,12 +1081,7 @@ void VtRendererTest::XtermTestColors() RenderSettings renderSettings; RenderData renderData; - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -1234,12 +1214,7 @@ void VtRendererTest::XtermTestCursor() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -1412,12 +1387,7 @@ void VtRendererTest::TestWrapping() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_firstPaint); - }); + VerifyFirstPaint(*engine); Viewport view = SetUpViewport(); @@ -1465,8 +1435,10 @@ void VtRendererTest::TestResize() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear and go home + // Verify the first BeginPaint emits a clear and go home qExpectedInput.push_back("\x1b[2J"); + // Verify the first EndPaint sets the cursor state + qExpectedInput.push_back("\x1b[?25l"); VERIFY_IS_TRUE(engine->_firstPaint); VERIFY_IS_TRUE(engine->_suppressResizeRepaint); @@ -1502,21 +1474,6 @@ void VtRendererTest::TestCursorVisibility() auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); engine->SetTestCallback(pfn); - // Verify the first paint emits a clear - qExpectedInput.push_back("\x1b[2J"); - VERIFY_IS_TRUE(engine->_firstPaint); - VERIFY_IS_FALSE(engine->_lastCursorIsVisible); - VERIFY_IS_TRUE(engine->_nextCursorIsVisible); - TestPaint(*engine, [&]() { - // During StartPaint, we'll mark the cursor as off. make sure that happens. - VERIFY_IS_FALSE(engine->_nextCursorIsVisible); - VERIFY_IS_FALSE(engine->_firstPaint); - }); - - // The cursor wasn't painted in the last frame. - VERIFY_IS_FALSE(engine->_lastCursorIsVisible); - VERIFY_IS_FALSE(engine->_nextCursorIsVisible); - COORD origin{ 0, 0 }; VERIFY_ARE_NOT_EQUAL(origin, engine->_lastText); @@ -1527,8 +1484,8 @@ void VtRendererTest::TestCursorVisibility() // Frame 1: Paint the cursor at the home position. At the end of the frame, // the cursor should be on. Because we're moving the cursor with CUP, we // need to disable the cursor during this frame. + qExpectedInput.push_back("\x1b[2J"); TestPaint(*engine, [&]() { - VERIFY_IS_FALSE(engine->_lastCursorIsVisible); VERIFY_IS_FALSE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1539,10 +1496,13 @@ void VtRendererTest::TestCursorVisibility() VERIFY_IS_TRUE(engine->_nextCursorIsVisible); VERIFY_IS_TRUE(engine->_needToDisableCursor); + // GH#12401: + // The other tests verify that the cursor is explicitly hidden on the + // first frame (VerifyFirstPaint). This test on the other hand does + // the opposite by calling PaintCursor() during the first paint cycle. qExpectedInput.push_back("\x1b[?25h"); }); - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_TRUE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1550,7 +1510,6 @@ void VtRendererTest::TestCursorVisibility() // frame, the cursor should be on, the same as before. We aren't moving the // cursor during this frame, so _needToDisableCursor will stay false. TestPaint(*engine, [&]() { - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_FALSE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1561,7 +1520,6 @@ void VtRendererTest::TestCursorVisibility() VERIFY_IS_FALSE(engine->_needToDisableCursor); }); - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_TRUE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1569,7 +1527,6 @@ void VtRendererTest::TestCursorVisibility() // should be on, the same as before. Because we're moving the cursor with // CUP, we need to disable the cursor during this frame. TestPaint(*engine, [&]() { - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_FALSE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1580,7 +1537,6 @@ void VtRendererTest::TestCursorVisibility() VERIFY_SUCCEEDED(engine->PaintCursor(options)); - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_TRUE(engine->_nextCursorIsVisible); VERIFY_IS_TRUE(engine->_needToDisableCursor); @@ -1590,7 +1546,6 @@ void VtRendererTest::TestCursorVisibility() qExpectedInput.push_back("\x1b[?25h"); }); - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_TRUE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); @@ -1598,14 +1553,12 @@ void VtRendererTest::TestCursorVisibility() // should be off. Log::Comment(NoThrowString().Format(L"Painting without calling PaintCursor will hide the cursor")); TestPaint(*engine, [&]() { - VERIFY_IS_TRUE(engine->_lastCursorIsVisible); VERIFY_IS_FALSE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); qExpectedInput.push_back("\x1b[?25l"); }); - VERIFY_IS_FALSE(engine->_lastCursorIsVisible); VERIFY_IS_FALSE(engine->_nextCursorIsVisible); VERIFY_IS_FALSE(engine->_needToDisableCursor); } diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index 4b3807df270..cb8004cfd0f 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -15,7 +15,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, VtEngine(std::move(hPipe), initialViewport), _fUseAsciiOnly(fUseAsciiOnly), _needToDisableCursor(false), - _lastCursorIsVisible(false), + // GH#12401: Ensure a DECTCEM cursor show/hide sequence + // is emitted on the first frame no matter what. + _lastCursorIsVisible(Tribool::Invalid), _nextCursorIsVisible(true) { // Set out initial cursor position to -1, -1. This will force our initial @@ -98,31 +100,20 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, { // If the cursor was previously visible, let's hide it for this frame, // by prepending a cursor off. - if (_lastCursorIsVisible) + if (_lastCursorIsVisible != Tribool::False) { _buffer.insert(0, "\x1b[?25l"); - _lastCursorIsVisible = false; + _lastCursorIsVisible = Tribool::False; } // If the cursor was NOT previously visible, then that's fine! we don't // need to worry, it's already off. } - // If the cursor is moving from off -> on (including cases where we just - // disabled if for this frame), show the cursor at the end of the frame - if (_nextCursorIsVisible && !_lastCursorIsVisible) + if (_lastCursorIsVisible != static_cast(_nextCursorIsVisible)) { - RETURN_IF_FAILED(_ShowCursor()); + RETURN_IF_FAILED(_nextCursorIsVisible ? _ShowCursor() : _HideCursor()); + _lastCursorIsVisible = static_cast(_nextCursorIsVisible); } - // 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) - { - RETURN_IF_FAILED(_HideCursor()); - } - - // Update our tracker of what we thought the last cursor state of the - // terminal was. - _lastCursorIsVisible = _nextCursorIsVisible; RETURN_IF_FAILED(VtEngine::EndPaint()); diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 1cda1782710..2e2b58affea 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -54,9 +54,18 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT WriteTerminalW(const std::wstring_view str) noexcept override; protected: + // I'm using a non-class enum here, so that the values + // are trivially convertible and comparable to bool. + enum class Tribool : uint8_t + { + False = 0, + True, + Invalid, + }; + const bool _fUseAsciiOnly; bool _needToDisableCursor; - bool _lastCursorIsVisible; + Tribool _lastCursorIsVisible; bool _nextCursorIsVisible; [[nodiscard]] HRESULT _MoveCursor(const COORD coord) noexcept override;