maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Render the cursor state to VT (#2829)

Browse source 
* Render the cursor state to VT
* Remove TestPaintXterm entirely, as it's unused now
This commit is contained in:
Mike Griese 2019-10-02 18:11:27 -05:00 committed by Carlos Zamora
parent b97db63030
commit a9f384931e
4 changed files with 192 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

183
src/host/ut_host/VtRendererTests.cpp
View file

@ -121,12 +121,13 @@ class Microsoft::Console::Render::VtRendererTest
TEST_METHOD(TestResize);
TEST_METHOD(TestCursorVisibility);
void Test16Colors(VtEngine* engine);
std::deque<std::string> qExpectedInput;
bool WriteCallback(const char* const pch, size_t const cch);
void TestPaint(VtEngine& engine, std::function<void()> pfn);
void TestPaintXterm(XtermEngine& engine, std::function<void()> pfn);
Viewport SetUpViewport();
};
@ -173,38 +174,6 @@ void VtRendererTest::TestPaint(VtEngine& engine, std::function<void()> pfn)
VERIFY_SUCCEEDED(engine.EndPaint());
}
// Function Description:
// - Small helper to do a series of testing wrapped by StartPaint/EndPaint calls
// Also expects \x1b[?25l and \x1b[?25h on start/stop, for cursor visibility
// Arguments:
// - engine: the engine to operate on
// - pfn: A function pointer to some test code to run.
// Return Value:
// - <none>
void VtRendererTest::TestPaintXterm(XtermEngine& engine, std::function<void()> pfn)
{
HRESULT hr = engine.StartPaint();
pfn();
// If we didn't have anything to do on this frame, still execute our
// callback, but don't check for the following ?25h
if (hr != S_FALSE)
{
// If the engine has decided that it needs to disble the cursor, it'll
// insert ?25l to the front of the buffer (which won't hit this
// callback) and write ?25h to the end of the frame
if (engine._needToDisableCursor)
{
qExpectedInput.push_back("\x1b[?25h");
}
}
VERIFY_SUCCEEDED(engine.EndPaint());
VERIFY_ARE_EQUAL(qExpectedInput.size(),
static_cast<size_t>(0),
L"Done painting, there shouldn't be any output we're still expecting");
}
void VtRendererTest::VtSequenceHelperTests()
{
wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE);
@ -298,7 +267,7 @@ void VtRendererTest::Xterm256TestInvalidate()
L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences"));
COORD scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one down, only top line is invalid. ----"));
invalid = view.ToExclusive();
@ -314,7 +283,7 @@ void VtRendererTest::Xterm256TestInvalidate()
scrollDelta = { 0, 3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -328,7 +297,7 @@ void VtRendererTest::Xterm256TestInvalidate()
scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one up, only bottom line is invalid. ----"));
invalid = view.ToExclusive();
@ -343,7 +312,7 @@ void VtRendererTest::Xterm256TestInvalidate()
scrollDelta = { 0, -3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three up, only bottom 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -363,7 +332,7 @@ void VtRendererTest::Xterm256TestInvalidate()
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
scrollDelta = { 0, 2 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -557,8 +526,6 @@ void VtRendererTest::Xterm256TestCursor()
L"----move to the start of this line (y stays the same)----"));
qExpectedInput.push_back("\r");
VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 1 }));
qExpectedInput.push_back("\x1b[?25h");
});
TestPaint(*engine, [&]() {
@ -629,7 +596,7 @@ void VtRendererTest::XtermTestInvalidate()
L"Make sure that invalidating anything only invalidates that portion"));
SMALL_RECT invalid = { 1, 1, 1, 1 };
VERIFY_SUCCEEDED(engine->Invalidate(&invalid));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive());
});
@ -637,7 +604,7 @@ void VtRendererTest::XtermTestInvalidate()
L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences"));
COORD scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one down, only top line is invalid. ----"));
invalid = view.ToExclusive();
@ -652,7 +619,7 @@ void VtRendererTest::XtermTestInvalidate()
scrollDelta = { 0, 3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -666,7 +633,7 @@ void VtRendererTest::XtermTestInvalidate()
scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one up, only bottom line is invalid. ----"));
invalid = view.ToExclusive();
@ -681,7 +648,7 @@ void VtRendererTest::XtermTestInvalidate()
scrollDelta = { 0, -3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three up, only bottom 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -701,7 +668,7 @@ void VtRendererTest::XtermTestInvalidate()
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
scrollDelta = { 0, 2 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
invalid = view.ToExclusive();
@ -864,8 +831,6 @@ void VtRendererTest::XtermTestCursor()
L"----move to the start of this line (y stays the same)----"));
qExpectedInput.push_back("\r");
VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 1 }));
qExpectedInput.push_back("\x1b[?25h");
});
TestPaint(*engine, [&]() {
@ -1130,14 +1095,14 @@ void VtRendererTest::TestWrapping()
Viewport view = SetUpViewport();
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"Make sure the cursor is at 0,0"));
qExpectedInput.push_back("\x1b[H");
VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 0 }));
});
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"Painting a line that wrapped, then painting another line, and "
L"making sure we don't manually move the cursor between those paints."));
@ -1196,9 +1161,125 @@ void VtRendererTest::TestResize()
VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive()));
TestPaintXterm(*engine, [&]() {
TestPaint(*engine, [&]() {
VERIFY_ARE_EQUAL(newView, engine->_invalidRect);
VERIFY_IS_FALSE(engine->_firstPaint);
VERIFY_IS_FALSE(engine->_suppressResizeRepaint);
});
}
void VtRendererTest::TestCursorVisibility()
{
Viewport view = SetUpViewport();
wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE);
auto engine = std::make_unique<Xterm256Engine>(std::move(hFile), _shutdownEvent, p, view, g_ColorTable, static_cast<WORD>(COLOR_TABLE_SIZE));
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);
IRenderEngine::CursorOptions options{};
options.coordCursor = origin;
// 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.
TestPaint(*engine, [&]() {
VERIFY_IS_FALSE(engine->_lastCursorIsVisible);
VERIFY_IS_FALSE(engine->_nextCursorIsVisible);
VERIFY_IS_FALSE(engine->_needToDisableCursor);
Log::Comment(NoThrowString().Format(L"Make sure the cursor is at 0,0"));
qExpectedInput.push_back("\x1b[H");
VERIFY_SUCCEEDED(engine->PaintCursor(options));
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_TRUE(engine->_needToDisableCursor);
qExpectedInput.push_back("\x1b[?25h");
});
VERIFY_IS_TRUE(engine->_lastCursorIsVisible);
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_FALSE(engine->_needToDisableCursor);
// Frame 2: Paint the cursor again at the home position. At the end of the
// 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);
Log::Comment(NoThrowString().Format(L"If we just paint the cursor again at the same position, the cursor should not need to be disabled"));
VERIFY_SUCCEEDED(engine->PaintCursor(options));
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_FALSE(engine->_needToDisableCursor);
});
VERIFY_IS_TRUE(engine->_lastCursorIsVisible);
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_FALSE(engine->_needToDisableCursor);
// Frame 3: Paint the cursor at 2,2. At the end of the frame, the cursor
// 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);
Log::Comment(NoThrowString().Format(L"Move the cursor to 2,2"));
qExpectedInput.push_back("\x1b[3;3H");
options.coordCursor = { 2, 2 };
VERIFY_SUCCEEDED(engine->PaintCursor(options));
VERIFY_IS_TRUE(engine->_lastCursorIsVisible);
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_TRUE(engine->_needToDisableCursor);
// Because _needToDisableCursor is true, we'll insert a ?25l at the
// start of the frame. Unfortunately, we can't test to make sure that
// it's there, but we can ensure that the matching ?25h is printed:
qExpectedInput.push_back("\x1b[?25h");
});
VERIFY_IS_TRUE(engine->_lastCursorIsVisible);
VERIFY_IS_TRUE(engine->_nextCursorIsVisible);
VERIFY_IS_FALSE(engine->_needToDisableCursor);
// Frame 4: Don't paint the cursor. At the end of the frame, the cursor
// 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);
}

73
src/renderer/vt/XtermEngine.cpp
View file

@ -22,7 +22,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
_fUseAsciiOnly(fUseAsciiOnly),
_previousLineWrapped(false),
_usingUnderLine(false),
_needToDisableCursor(false)
_needToDisableCursor(false),
_lastCursorIsVisible(false),
_nextCursorIsVisible(true)
{
// Set out initial cursor position to -1, -1. This will force our initial
// paint to manually move the cursor to 0, 0, not just ignore it.
@ -45,6 +47,11 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
_trace.TraceLastText(_lastText);
// Prep us to think that the cursor is not visible this frame. If it _is_
// visible, then PaintCursor will be called, and we'll set this to true
// during the frame.
_nextCursorIsVisible = false;
if (_firstPaint)
{
// MSFT:17815688
@ -73,14 +80,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
if (!_quickReturn)
{
if (!_WillWriteSingleChar())
{
// MSFT:TODO:20331739
// Make sure to match the cursor visibility in the terminal to the console's
// // Turn off cursor
// RETURN_IF_FAILED(_HideCursor());
}
else
if (_WillWriteSingleChar())
{
// Don't re-enable the cursor.
_quickReturn = true;
@ -99,22 +99,38 @@ 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::EndPaint() noexcept
{
// MSFT:TODO:20331739
// Make sure to match the cursor visibility in the terminal to the console's
// if (!_quickReturn)
// {
// // Turn on cursor
// RETURN_IF_FAILED(_ShowCursor());
// }
// If during the frame we determined that the cursor needed to be disabled,
// then insert a cursor off at the start of the buffer, and re-enable
// the cursor here.
if (_needToDisableCursor)
{
_buffer.insert(0, "\x1b[25l");
// If the cursor was previously visible, let's hide it for this frame,
// by prepending a cursor off.
if (_lastCursorIsVisible)
{
_buffer.insert(0, "\x1b[25l");
_lastCursorIsVisible = 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)
{
RETURN_IF_FAILED(_ShowCursor());
}
// 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());
@ -178,6 +194,27 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, colorBackground, isBold, _ColorTable, _cColorTable);
}
// Routine Description:
// - Draws the cursor on the screen
// Arguments:
// - options - Options that affect the presentation of the cursor
// Return Value:
// - S_OK or suitable HRESULT error from writing pipe.
[[nodiscard]] HRESULT XtermEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept
{
// PaintCursor is only called when the cursor is in fact visible in a single
// frame. When this is called, mark _nextCursorIsVisible as true. At the end
// of the frame, we'll decide to either turn the cursor on or not, based
// upon the previous state.
// When this method is not called during a frame, it's because the cursor
// was not visible. In that case, at the end of the frame,
// _nextCursorIsVisible will still be false (from when we set it during
// StartPaint)
_nextCursorIsVisible = true;
return VtEngine::PaintCursor(options);
}
// Routine Description:
// - Write a VT sequence to move the cursor to the specified coordinates. We
// also store the last place we left the cursor for future optimizations.

4
src/renderer/vt/XtermEngine.hpp
View file

@ -40,6 +40,8 @@ namespace Microsoft::Console::Render
[[nodiscard]] HRESULT StartPaint() noexcept override;
[[nodiscard]] HRESULT EndPaint() noexcept override;
[[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override;
[[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground,
const COLORREF colorBackground,
const WORD legacyColorAttribute,
@ -61,6 +63,8 @@ namespace Microsoft::Console::Render
bool _previousLineWrapped;
bool _usingUnderLine;
bool _needToDisableCursor;
bool _lastCursorIsVisible;
bool _nextCursorIsVisible;
[[nodiscard]] HRESULT _MoveCursor(const COORD coord) noexcept override;

2
src/renderer/vt/vtrenderer.hpp
View file

@ -64,7 +64,7 @@ namespace Microsoft::Console::Render
const COORD coordTarget) noexcept override;
[[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override;
[[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override;
[[nodiscard]] virtual HRESULT PaintCursor(const CursorOptions& options) noexcept override;
[[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground,
const COLORREF colorBackground,