Skip to content

Commit

Permalink
Revert "Prevent the virtual viewport bottom being moved up unintentio…
Browse files Browse the repository at this point in the history
…nally (#9770)"

This reverts commit 74909c0.

Reopens #9754
Closes #9872
  • Loading branch information
DHowett committed Apr 28, 2021
1 parent c5fcbed commit 3809bb5
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 96 deletions.
11 changes: 0 additions & 11 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,17 +656,6 @@ bool ConhostInternalGetSet::InsertLines(const size_t count)
return true;
}

// Method Description:
// - Resets the internal "virtual bottom" tracker to the top of the buffer.
// Arguments:
// - <none>
// Return Value:
// - <none>
void ConhostInternalGetSet::ResetBottom()
{
_io.GetActiveOutputBuffer().ResetBottom();
}

// Method Description:
// - Connects the MoveToBottom call directly into our Driver Message servicing
// call inside Conhost.exe
Expand Down
1 change: 0 additions & 1 deletion src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
bool DeleteLines(const size_t count) override;
bool InsertLines(const size_t count) override;

void ResetBottom() override;
bool MoveToBottom() const override;

bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const noexcept override;
Expand Down
24 changes: 3 additions & 21 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,14 @@ SCREEN_INFORMATION::~SCREEN_INFORMATION()
// Set up viewport
pScreen->_viewport = Viewport::FromDimensions({ 0, 0 },
pScreen->_IsInPtyMode() ? coordScreenBufferSize : coordWindowSize);
pScreen->UpdateBottom();

// Set up text buffer
pScreen->_textBuffer = std::make_unique<TextBuffer>(coordScreenBufferSize,
defaultAttributes,
uiCursorSize,
pScreen->_renderTarget);

pScreen->UpdateBottom();

const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
pScreen->_textBuffer->GetCursor().SetColor(gci.GetCursorColor());
pScreen->_textBuffer->GetCursor().SetType(gci.GetCursorType());
Expand Down Expand Up @@ -2529,30 +2528,13 @@ TextBufferCellIterator SCREEN_INFORMATION::GetCellDataAt(const COORD at, const V

// Method Description:
// - Updates our internal "virtual bottom" tracker with wherever the viewport
// currently is. This is only used to move the bottom further down, unless
// the buffer has shrunk, and the viewport needs to be moved back up to
// fit within the new buffer range.
// currently is.
// - <none>
// Return Value:
// - <none>
void SCREEN_INFORMATION::UpdateBottom()
{
// We clamp it so it's at least as low as the current viewport bottom,
// but no lower than the bottom of the buffer.
_virtualBottom = std::clamp(_virtualBottom, _viewport.BottomInclusive(), GetBufferSize().BottomInclusive());
}

// Method Description:
// - Resets the internal "virtual bottom" tracker to the top of the buffer.
// Used when the scrollback buffer has been completely cleared.
// - <none>
// Return Value:
// - <none>
void SCREEN_INFORMATION::ResetBottom()
{
// The virtual bottom points to the last line of the viewport, so at the
// top of the buffer it should be one less than the viewport height.
_virtualBottom = _viewport.Height() - 1;
_virtualBottom = _viewport.BottomInclusive();
}

// Method Description:
Expand Down
1 change: 0 additions & 1 deletion src/host/screenInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
void SetTerminalConnection(_In_ Microsoft::Console::ITerminalOutputConnection* const pTtyConnection);

void UpdateBottom();
void ResetBottom();
void MoveToBottom();

Microsoft::Console::Render::IRenderTarget& GetRenderTarget() noexcept;
Expand Down
53 changes: 0 additions & 53 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ class ScreenBufferTests
auto& currentBuffer = gci.GetActiveOutputBuffer();
// Make sure a test hasn't left us in the alt buffer on accident
VERIFY_IS_FALSE(currentBuffer._IsAltBuffer());
// Make sure the virtual viewport bottom is reset.
currentBuffer._virtualBottom = 0;
VERIFY_SUCCEEDED(currentBuffer.SetViewportOrigin(true, { 0, 0 }, true));
// Make sure the viewport always starts off at the default size.
auto defaultSize = COORD{ CommonState::s_csWindowWidth, CommonState::s_csWindowHeight };
Expand Down Expand Up @@ -217,7 +215,6 @@ class ScreenBufferTests
TEST_METHOD(TestAddHyperlinkCustomIdDifferentUri);

TEST_METHOD(UpdateVirtualBottomWhenCursorMovesBelowIt);
TEST_METHOD(DontUpdateVirtualBottomWhenMovedUp);
TEST_METHOD(RetainHorizontalOffsetWhenMovingToBottom);

TEST_METHOD(TestWriteConsoleVTQuirkMode);
Expand Down Expand Up @@ -1545,7 +1542,6 @@ void ScreenBufferTests::VtNewlineOutsideMargins()
VERIFY_ARE_EQUAL(COORD({ 0, viewportTop + 1 }), si.GetViewport().Origin());

Log::Comment(L"Reset viewport and apply DECSTBM margins");
si._virtualBottom = 0;
VERIFY_SUCCEEDED(si.SetViewportOrigin(true, COORD({ 0, viewportTop }), true));
stateMachine.ProcessString(L"\x1b[1;5r");
// Make sure we clear the margins on exit so they can't break other tests.
Expand Down Expand Up @@ -3345,7 +3341,6 @@ void ScreenBufferTests::ScrollOperations()
const auto bufferHeight = si.GetBufferSize().Height();

// Move the viewport down a few lines, and only cover part of the buffer width.
si._virtualBottom = 0;
si.SetViewport(Viewport::FromDimensions({ 5, 10 }, { bufferWidth - 10, 10 }), true);
const auto viewportStart = si.GetViewport().Top();
const auto viewportEnd = si.GetViewport().BottomExclusive();
Expand Down Expand Up @@ -3870,7 +3865,6 @@ void ScreenBufferTests::EraseTests()
const auto bufferHeight = si.GetBufferSize().Height();

// Move the viewport down a few lines, and only cover part of the buffer width.
si._virtualBottom = 0;
si.SetViewport(Viewport::FromDimensions({ 5, 10 }, { bufferWidth - 10, 10 }), true);

// Fill the entire buffer with Zs. Blue on Green.
Expand Down Expand Up @@ -3991,7 +3985,6 @@ void _CommonScrollingSetup()
const auto oldView = si.GetViewport();
const auto view = Viewport::FromDimensions({ 0, 0 }, { oldView.Width(), 6 });
si.SetViewport(view, true);
si.ResetBottom();
cursor.SetPosition({ 0, 0 });
stateMachine.ProcessString(L"A");
cursor.SetPosition({ 0, 5 });
Expand Down Expand Up @@ -6028,52 +6021,6 @@ void ScreenBufferTests::UpdateVirtualBottomWhenCursorMovesBelowIt()
VERIFY_ARE_EQUAL(newVirtualBottom, si.GetViewport().BottomInclusive());
}

void ScreenBufferTests::DontUpdateVirtualBottomWhenMovedUp()
{
auto& g = ServiceLocator::LocateGlobals();
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& cursor = si.GetTextBuffer().GetCursor();

Log::Comment(L"Set the initial viewport one page from the top");
const auto initialOrigin = COORD{ 0, si.GetViewport().Top() + si.GetViewport().Height() };
VERIFY_SUCCEEDED(si.SetViewportOrigin(false, initialOrigin, false));
VERIFY_ARE_EQUAL(initialOrigin, si.GetViewport().Origin());

Log::Comment(L"Make sure the initial virtual bottom is at the bottom of the viewport");
si.UpdateBottom();
const auto initialVirtualBottom = si.GetViewport().BottomInclusive();
VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom);

Log::Comment(L"Set the initial cursor position 5 lines above virtual bottom line");
const auto initialCursorPos = COORD{ 0, initialVirtualBottom - 5 };
cursor.SetPosition(initialCursorPos);
VERIFY_ARE_EQUAL(initialCursorPos, cursor.GetPosition());

Log::Comment(L"Pan to the top of the buffer so the the cursor is out of view");
const auto topOfBufferOrigin = COORD{ 0, 0 };
VERIFY_SUCCEEDED(si.SetViewportOrigin(true, topOfBufferOrigin, false));
VERIFY_ARE_EQUAL(topOfBufferOrigin, si.GetViewport().Origin());

Log::Comment(L"Confirm that the virtual bottom has not changed");
VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom);

Log::Comment(L"Now write out some content using WriteCharsLegacy");
const auto content = L"Hello World";
auto numBytes = wcslen(content) * sizeof(wchar_t);
VERIFY_SUCCESS_NTSTATUS(WriteCharsLegacy(si, content, content, content, &numBytes, nullptr, 0, 0, nullptr));

Log::Comment(L"The viewport bottom should now align with the cursor pos");
VERIFY_ARE_EQUAL(initialCursorPos.Y, si.GetViewport().BottomInclusive());

Log::Comment(L"But the virtual bottom should still not have changed");
VERIFY_ARE_EQUAL(initialVirtualBottom, si._virtualBottom);

Log::Comment(L"After MoveToBottom, the viewport should align with the virtual bottom");
si.MoveToBottom();
VERIFY_ARE_EQUAL(initialVirtualBottom, si.GetViewport().BottomInclusive());
}

void ScreenBufferTests::RetainHorizontalOffsetWhenMovingToBottom()
{
auto& g = ServiceLocator::LocateGlobals();
Expand Down
3 changes: 0 additions & 3 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2030,9 +2030,6 @@ bool AdaptDispatch::_EraseScrollback()

if (success)
{
// Reset the virtual viewport bottom to the top of the buffer.
_pConApi->ResetBottom();

// Move the viewport (CAN'T be done in one call with SetConsolescreenBufferInfoEx, because legacy)
SMALL_RECT newViewport;
newViewport.Left = screen.Left;
Expand Down
1 change: 0 additions & 1 deletion src/terminal/adapter/conGetSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ namespace Microsoft::Console::VirtualTerminal
virtual bool DeleteLines(const size_t count) = 0;
virtual bool InsertLines(const size_t count) = 0;

virtual void ResetBottom() = 0;
virtual bool MoveToBottom() const = 0;

virtual bool PrivateGetColorTableEntry(const size_t index, COLORREF& value) const = 0;
Expand Down
5 changes: 0 additions & 5 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -492,11 +492,6 @@ class TestGetSet final : public ConGetSet
return TRUE;
}

void ResetBottom() override
{
Log::Comment(L"ResetBottom MOCK called...");
}

bool MoveToBottom() const override
{
Log::Comment(L"MoveToBottom MOCK called...");
Expand Down

0 comments on commit 3809bb5

Please sign in to comment.