Skip to content

Commit

Permalink
Stop committing the whole buffer when determining if it's empty (micr…
Browse files Browse the repository at this point in the history
…osoft#15582)

As a shortcut, GetLastNonSpaceCharacter can start with the last
committed row. It's guaranteed that there isn't anything of worth below
that point, so why bother checking?

Without this, Terminal immediately commits the entire 9031-line buffer
on startup while trying to--get this!--clear the screen!

---------

Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
  • Loading branch information
2 people authored and qqkookie committed Jun 22, 2023
1 parent 1429922 commit 20500ba
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
17 changes: 15 additions & 2 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ ROW& TextBuffer::_getRowByOffsetDirect(size_t offset)
return *reinterpret_cast<ROW*>(row);
}

// Returns the "user-visible" index of the last committed row, which can be used
// to short-circuit some algorithms that try to scan the entire buffer.
// Returns 0 if no rows are committed in.
til::CoordType TextBuffer::_estimateOffsetOfLastCommittedRow() const noexcept
{
const auto lastRowOffset = (_commitWatermark - _buffer.get()) / _bufferRowStride;
// This subtracts 2 from the offset to account for the:
// * scratchpad row at offset 0, whereas regular rows start at offset 1.
// * fact that _commitWatermark points _past_ the last committed row,
// but we want to return an index pointing at the last row.
return std::max(0, gsl::narrow_cast<til::CoordType>(lastRowOffset - 2));
}

// Retrieves a row from the buffer by its offset from the first row of the text buffer
// (what corresponds to the top row of the screen buffer).
const ROW& TextBuffer::GetRowByOffset(const til::CoordType index) const
Expand Down Expand Up @@ -807,7 +820,7 @@ til::point TextBuffer::GetLastNonSpaceCharacter(std::optional<const Microsoft::C

til::point coordEndOfText;
// Search the given viewport by starting at the bottom.
coordEndOfText.y = viewport.BottomInclusive();
coordEndOfText.y = std::min(viewport.BottomInclusive(), _estimateOffsetOfLastCommittedRow());

const auto& currRow = GetRowByOffset(coordEndOfText.y);
// The X position of the end of the valid text is the Right draw boundary (which is one beyond the final valid character)
Expand Down Expand Up @@ -2622,7 +2635,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
// printable char gets reset. See GH #12567
auto newRowY = newCursor.GetPosition().y + 1;
const auto newHeight = newBuffer.GetSize().Height();
const auto oldHeight = oldBuffer.GetSize().Height();
const auto oldHeight = oldBuffer._estimateOffsetOfLastCommittedRow() + 1;
for (;
iOldRow < oldHeight && newRowY < newHeight;
iOldRow++)
Expand Down
1 change: 1 addition & 0 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ class TextBuffer final
void _construct(const std::byte* until) noexcept;
void _destroy() const noexcept;
ROW& _getRowByOffsetDirect(size_t offset);
til::CoordType _estimateOffsetOfLastCommittedRow() const noexcept;

void _SetFirstRowIndex(const til::CoordType FirstRowIndex) noexcept;
til::point _GetPreviousFromCursor() const;
Expand Down

0 comments on commit 20500ba

Please sign in to comment.