Outstanding cleanup for PR

This commit is contained in:
Mike Griese
2020-01-24 14:15:40 -06:00
parent be668296c1
commit 715014d10f
9 changed files with 24 additions and 91 deletions

View File

@@ -1554,13 +1554,9 @@ std::string TextBuffer::GenRTF(const TextAndColor& rows, const int fontHeightPoi
// Arguments:
// - oldBuffer - the text buffer to copy the contents FROM
// - newBuffer - the text buffer to copy the contents TO
// - padTop - If true, and we're increasing the height of the buffer, we should
// start by inserting new lines at the top of the buffer, so the contents stay
// in the relatively same position. This is only ever set to `true` in the
// console by conpty mode.
// Return Value:
// - S_OK if we successfully copied the contents to the new buffer, otherwise an appropriate HRESULT.
HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const bool padTop, const bool preserveSpaceAtBottom)
HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer)
{
Cursor& oldCursor = oldBuffer.GetCursor();
Cursor& newCursor = newBuffer.GetCursor();
@@ -1572,29 +1568,14 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const b
// place the new cursor back on the equivalent character in
// the new buffer.
const COORD cOldCursorPos = oldCursor.GetPosition();
// const COORD cOldLastChar = preserveSpaceAtBottom ? oldBuffer.GetSize().Dimensions() : oldBuffer.GetLastNonSpaceCharacter();
const COORD cOldLastChar = oldBuffer.GetLastNonSpaceCharacter();
// short const cOldRowsTotal = cOldLastChar.Y + 1;
short const cOldRowsTotal = preserveSpaceAtBottom ?
oldBuffer.GetSize().Dimensions().Y :
cOldLastChar.Y + 1;
short const cOldRowsTotal = cOldLastChar.Y + 1;
short const cOldColsTotal = oldBuffer.GetSize().Width();
COORD cNewCursorPos = { 0 };
bool fFoundCursorPos = false;
// If we increased the height of the buffer, pad the top of the new buffer
// with newlines, so that the contents stay in the same relative location.
if (padTop && oldBuffer.GetSize().Height() < newBuffer.GetSize().Height())
{
const auto diff = newBuffer.GetSize().Height() - oldBuffer.GetSize().Height();
for (int i = 0; i < diff; i++)
{
newBuffer.NewlineCursor();
}
}
HRESULT hr = S_OK;
// Loop through all the rows of the old buffer and reprint them into the new buffer
for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++)

View File

@@ -158,7 +158,7 @@ public:
const std::wstring_view fontFaceName,
const COLORREF backgroundColor);
static HRESULT Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const bool padTop, const bool preserveSpaceAtBottom);
static HRESULT Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer);
private:
std::deque<ROW> _storage;

View File

@@ -189,6 +189,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
// there were blank lines at the bottom, those lines will get trimmed.
// If there's not blank lines, then the top will get "shifted down",
// moving the top line into scrollback.
// See GH#3490 for more details.
// If the final position in the buffer is on the bottom row of the new
// viewport, then we're going to need to move the top down. Otherwise,

View File

@@ -449,7 +449,7 @@ void ConptyRoundtripTests::TestResizeHeight()
VERIFY_ARE_EQUAL(TerminalViewHeight, initialTermView.BottomExclusive());
Log::Comment(NoThrowString().Format(
L"Print 50 lines of output, which will scroll the viewport"));
L"Print %d lines of output, which will scroll the viewport", printedRows));
for (auto i = 0; i < printedRows; i++)
{
@@ -492,7 +492,7 @@ void ConptyRoundtripTests::TestResizeHeight()
for (short row = 0; row < rowsWithText; row++)
{
// SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
SetVerifyOutput settings(VerifyOutputSettings::LogOnlyFailures);
auto iter = termTb.GetCellDataAt({ 0, row });
const wchar_t expectedChar = static_cast<wchar_t>((row + numLostRows) % 93) + 33;

View File

@@ -446,11 +446,3 @@ void VtIo::EndResize()
_pVtRenderEngine->EndResizeRequest();
}
}
void VtIo::SetVirtualTop(const short virtualTop) noexcept
{
if (_pVtRenderEngine)
{
_pVtRenderEngine->SetVirtualTop(virtualTop);
}
}

View File

@@ -38,7 +38,6 @@ namespace Microsoft::Console::VirtualTerminal
void BeginResize();
void EndResize();
void SetVirtualTop(const short virtualTop) noexcept;
#ifdef UNIT_TESTING
void EnableConptyModeForTests();

View File

@@ -1427,29 +1427,11 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const bool isConpty = gci.IsInVtIoMode();
HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get(), false, false);
HRESULT hr = TextBuffer::Reflow(*_textBuffer.get(), *newTextBuffer.get());
const bool widthChanged = coordNewScreenSize.X != _textBuffer->GetSize().Width();
if (SUCCEEDED(hr))
{
// GH#3490 - In conpty mode, we want to pad new lines into to the top of
// the buffer when we increase the buffer height. Terminals typically
// keep the buffer contents "stuck" to the bottom of the viewport when
// they increase in height, moving lines from the scrollback into the
// viewport. We don't have a scrollback at all in conpty mode, but we
// can still keep the content we have at the bottom by inserting new
// lines at the top.
//
// When that happens, make sure to move our relative cursor position
// down, so that it stays in the same position relative to the bottom
// that it was before.
// if (isConpty && _textBuffer->GetSize().Height() < newTextBuffer->GetSize().Height())
// {
// const short diff = newTextBuffer->GetSize().Height() - _textBuffer->GetSize().Height();
// cursorHeightInViewportBefore += diff;
// gci.GetVtIo()->SetVirtualTop(diff);
// }
Cursor& newCursor = newTextBuffer->GetCursor();
// Adjust the viewport so the cursor doesn't wildly fly off up or down.
const short cursorHeightInViewportAfter = newCursor.GetPosition().Y - _viewport.Top();
@@ -1460,13 +1442,18 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
_textBuffer.swap(newTextBuffer);
}
if (isConpty)
// GH#3490 - In conpty mode, We want to invalidate all of the viewport that
// might have been below any wrapped lines, up until the last character of
// the buffer. Lines that were wrapped may have been re-wrapped during this
// resize, so we want them repainted to the terminal. We don't want to just
// invalidate everything though - if there were blank lines at the bottom,
// those can just be ignored.
if (isConpty && widthChanged)
{
// Loop through all the rows of the old buffer and reprint them into the new buffer
// for (short iOldRow = 0; iOldRow < cOldRowsTotal; iOldRow++)
auto bottom = std::max(_textBuffer->GetCursor().GetPosition().Y,
std::min(_viewport.BottomInclusive(),
_textBuffer->GetLastNonSpaceCharacter().Y));
const auto bottom = std::max(_textBuffer->GetCursor().GetPosition().Y,
std::min(_viewport.BottomInclusive(),
_textBuffer->GetLastNonSpaceCharacter().Y));
bool foundWrappedLine = false;
for (short y = _viewport.Top(); y <= bottom; y++)
{

View File

@@ -277,33 +277,12 @@ VtEngine::VtEngine(_In_ wil::unique_hfile pipe,
// lead to the first _actual_ resize being suppressed.
_suppressResizeRepaint = false;
if (SUCCEEDED(hr))
{
// if (oldView.Width() != newView.Width())
// {
// // Viewport is a different width now - just update it all. We may
// // have re-wrapped the buffer contents.
// hr = InvalidateAll();
// }
// else if (oldView.Height() < newView.Height())
// {
// // // We grew in height. We inserted empty lines at the top of the
// // // buffer. The cursor is now _actually_ lower (larger Y/row value)
// // // than it was before.
// // _lastText.Y += gsl::narrow_cast<short>(newView.Height() - oldView.Height());
// // // The text content will try and stay "stuck" at the bottom of the
// // // viewport of the terminal, and invalidating the bottom here can
// // // cause unnecessary lines to get written to the terminal. See
// // // GH#3490.
// }
// else if (oldView.Height() > newView.Height())
// {
// // We shrunk in height. We don't really need to do anything here.
// // Shrinking in height will remove lines from the top of the buffer
// // (pushing them into scrollback in the terminal).
// // The cursor will stay in the same relative position.
// }
}
// GH#3490 - When the viewport width changed, don't do anything extra here.
// If the buffer had areas that were invalid due to the resize, then the
// buffer will have triggered it's own invalidations for what it knows is
// invalid. Previously, we'd invalidate everything if the width changed,
// because we couldn't be sure if lines were reflowed.
_resized = true;
return hr;
}
@@ -403,11 +382,6 @@ bool VtEngine::_AllIsInvalid() const
return S_OK;
}
void VtEngine::SetVirtualTop(const short virtualTop) noexcept
{
_virtualTop = std::max(_virtualTop, virtualTop);
}
void VtEngine::SetTerminalOwner(Microsoft::Console::ITerminalOwner* const terminalOwner)
{
_terminalOwner = terminalOwner;

View File

@@ -104,7 +104,6 @@ namespace Microsoft::Console::Render
void SetTerminalOwner(Microsoft::Console::ITerminalOwner* const terminalOwner);
void BeginResizeRequest();
void EndResizeRequest();
void SetVirtualTop(const short virtualTop) noexcept;
protected:
wil::unique_hfile _hFile;