Skip to content

Commit

Permalink
WIP: An attempt to fix attributes after circling
Browse files Browse the repository at this point in the history
  • Loading branch information
@lhecker
lhecker committed Jun 2, 2023
1 parent cc14db2 commit 6f1e61e
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 74 deletions.
54 changes: 33 additions & 21 deletions src/buffer/out/Row.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,37 +119,49 @@ bool ROW::IsValid() const noexcept
return _valid;
}

void ROW::Discard() noexcept
void ROW::Discard(const TextAttribute& attr) noexcept
{
_charsHeap.reset();
_chars = { _charsBuffer, _columnCount };
// Constructing and then moving objects into place isn't free.
// Modifying the existing object is _much_ faster.
*_attr.runs().unsafe_shrink_to_size(1) = til::rle_pair{ attr, _columnCount };
_lineRendition = LineRendition::SingleWidth;
_wrapForced = false;
_doubleBytePadded = false;
_valid = false;
}

void ROW::Reset(const TextAttribute& attr) noexcept
void ROW::Initialize() noexcept
{
_init(attr);
std::fill_n(_chars.begin(), _columnCount, UNICODE_SPACE);
// The usage of _charsBuffer instead of _chars is quite intentional,
// as it allows _reset() to first call Initialize() and then Discard().
std::fill_n(_charsBuffer, _columnCount, UNICODE_SPACE);
std::iota(_charOffsets.begin(), _charOffsets.end(), uint16_t{ 0 });
_valid = true;
}

void ROW::Reset(const ROW& whitespaceRow, const TextAttribute& attr) noexcept
void ROW::Initialize(const ROW& whitespaceRow) noexcept
{
_init(attr);
memcpy(_chars.data(), whitespaceRow._chars.data(), std::min(whitespaceRow._chars.size(), _chars.size()) * 2);
memcpy(_charOffsets.data(), whitespaceRow._charOffsets.data(), std::min(whitespaceRow._charOffsets.size(), _charOffsets.size()) * 2);
assert(!_valid);
FAIL_FAST_IF(whitespaceRow.size() != size());
// The usage of _charsBuffer instead of _chars is quite intentional,
// as it allows _reset() to first call Initialize() and then Discard().
memcpy(_charsBuffer, whitespaceRow._charsBuffer, _columnCount * 2);
memcpy(_charOffsets.data(), whitespaceRow._charOffsets.data(), _charOffsets.size() * 2);
_valid = true;
}

void ROW::_init(const TextAttribute& attr) noexcept
// This is somewhat of a weapon of last resort to get the ROW back into a state that doesn't break
// callers too much. It discards the row (= releases the memory) but still ensures that it at least
// contains proper whitespace text to avoid any invalid text from being visible or returned anywhere.
void ROW::_reset() noexcept
{
_charsHeap.reset();
_chars = { _charsBuffer, _columnCount };
// Constructing and then moving objects into place isn't free.
// Modifying the existing object is _much_ faster.
*_attr.runs().unsafe_shrink_to_size(1) = til::rle_pair{ attr, _columnCount };
_lineRendition = LineRendition::SingleWidth;
_wrapForced = false;
_doubleBytePadded = false;
_valid = true;
static constexpr TextAttribute emptyAttributes{};
// By first calling Initialize() and then Discard() we end up in a state
// where _valid is false, but the contents are still sort of valid.
Initialize();
Discard(emptyAttributes);
}

void ROW::TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& attr, til::CoordType newWidth)
Expand Down Expand Up @@ -374,7 +386,7 @@ catch (...)
// Due to this function writing _charOffsets first, then calling _resizeChars (which may throw) and only then finally
// filling in _chars, we might end up in a situation were _charOffsets contains offsets outside of the _chars array.
// --> Restore this row to a known "okay"-state.
Reset(TextAttribute{});
_reset();
throw;
}

Expand Down Expand Up @@ -425,7 +437,7 @@ try
}
catch (...)
{
Reset(TextAttribute{});
_reset();
throw;
}

Expand Down Expand Up @@ -555,7 +567,7 @@ try
}
catch (...)
{
Reset(TextAttribute{});
_reset();
throw;
}

Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/Row.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ class ROW final
LineRendition GetLineRendition() const noexcept;

bool IsValid() const noexcept;
void Discard() noexcept;
void Reset(const TextAttribute& attr) noexcept;
void Reset(const ROW& whitespaceRow, const TextAttribute& attr) noexcept;
void Discard(const TextAttribute& attr) noexcept;
void Initialize() noexcept;
void Initialize(const ROW& whitespaceRow) noexcept;
void TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& attr, til::CoordType newWidth);

til::CoordType NavigateToPrevious(til::CoordType column) const noexcept;
Expand Down Expand Up @@ -182,7 +182,7 @@ class ROW final
uint16_t _uncheckedCharOffset(size_t col) const noexcept;
bool _uncheckedIsTrailer(size_t col) const noexcept;

void _init(const TextAttribute& attr) noexcept;
void _reset() noexcept;
void _resizeChars(uint16_t colEndDirty, uint16_t chBegDirty, size_t chEndDirty, uint16_t chEndDirtyOld);

// These fields are a bit "wasteful", but it makes all this a bit more robust against
Expand Down
78 changes: 41 additions & 37 deletions src/buffer/out/textBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,16 @@ til::CoordType TextBuffer::TotalRowCount() const noexcept
// Return Value:
// - const reference to the requested row. Asserts if out of bounds.
const ROW& TextBuffer::GetRowByOffset(const til::CoordType index) const noexcept
{
const auto& row = _getRowByOffset(index);
return row.IsValid() ? row : _whitespaceRow;
}

const ROW& TextBuffer::_getRowByOffset(const til::CoordType index) const noexcept
{
// Rows are stored circularly, so the index you ask for is offset by the start position and mod the total of rows.
const auto offsetIndex = gsl::narrow_cast<size_t>(_firstRow + index) % _storage.size();
const auto& row = til::at(_storage, offsetIndex);
return row.IsValid() ? row : _whitespaceRow;
return til::at(_storage, offsetIndex);
}

// Routine Description:
Expand All @@ -91,20 +96,28 @@ const ROW& TextBuffer::GetRowByOffset(const til::CoordType index) const noexcept
// - Number of rows down from the first row of the buffer.
// Return Value:
// - reference to the requested row. Asserts if out of bounds.
ROW& TextBuffer::GetRowByOffset(const til::CoordType index) noexcept
ROW& TextBuffer::GetMutableRowByOffset(const til::CoordType index) noexcept
{
// Rows are stored circularly, so the index you ask for is offset by the start position and mod the total of rows.
const auto offsetIndex = gsl::narrow_cast<size_t>(_firstRow + index) % _storage.size();
auto& row = til::at(_storage, offsetIndex);

auto& row = _getRowByOffset(index);
if (!row.IsValid())
{
row.Reset(_whitespaceRow, _currentAttributes);
row.Initialize(_whitespaceRow);
}

return row;
}

ROW& TextBuffer::_getRowByOffset(const til::CoordType index) noexcept
{
// Rows are stored circularly, so the index you ask for is offset by the start position and mod the total of rows.
const auto offsetIndex = gsl::narrow_cast<size_t>(_firstRow + index) % _storage.size();
return til::at(_storage, offsetIndex);
}

void TextBuffer::DiscardRowByOffset(const til::CoordType index, const TextAttribute& attr) noexcept
{
_getRowByOffset(index).Discard(attr);
}

// Routine Description:
// - Retrieves read-only text iterator at the given buffer location
// Arguments:
Expand Down Expand Up @@ -196,7 +209,7 @@ bool TextBuffer::_AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribut
{
// To figure out if the sequence is valid, we have to look at the character that comes before the current one
const auto coordPrevPosition = _GetPreviousFromCursor();
auto& prevRow = GetRowByOffset(coordPrevPosition.y);
auto& prevRow = GetMutableRowByOffset(coordPrevPosition.y);
DbcsAttribute prevDbcsAttr = DbcsAttribute::Single;
try
{
Expand Down Expand Up @@ -292,7 +305,7 @@ bool TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute
if (cursorPosition.x == lineWidth - 1)
{
// set that we're wrapping for double byte reasons
auto& row = GetRowByOffset(cursorPosition.y);
auto& row = GetMutableRowByOffset(cursorPosition.y);
row.SetDoubleBytePadded(true);

// then move the cursor forward and onto the next row
Expand All @@ -315,7 +328,7 @@ void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept
// The return value indicates to the caller whether the cursor should be moved to the next line.
void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state)
{
auto& r = GetRowByOffset(row);
auto& r = GetMutableRowByOffset(row);

r.ReplaceText(state);
r.ReplaceAttributes(state.columnBegin, state.columnEnd, attributes);
Expand Down Expand Up @@ -401,7 +414,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
}

// Get the row and write the cells
auto& row = GetRowByOffset(target.y);
auto& row = GetMutableRowByOffset(target.y);
const auto newIt = row.WriteCells(givenIt, target.x, wrap, limitRight);

// Take the cell distance written and notify that it needs to be repainted.
Expand Down Expand Up @@ -435,7 +448,7 @@ bool TextBuffer::InsertCharacter(const std::wstring_view chars,
const auto iCol = GetCursor().GetPosition().x; // column logical and array positions are equal.

// Get the row associated with the given logical position
auto& Row = GetRowByOffset(iRow);
auto& Row = GetMutableRowByOffset(iRow);

// Store character and double byte data
try
Expand Down Expand Up @@ -509,7 +522,7 @@ void TextBuffer::_AdjustWrapOnCurrentRow(const bool fSet) noexcept
const auto uiCurrentRowOffset = GetCursor().GetPosition().y;

// Set the wrap status as appropriate
GetRowByOffset(uiCurrentRowOffset).SetWrapForced(fSet);
GetMutableRowByOffset(uiCurrentRowOffset).SetWrapForced(fSet);
}

//Routine Description:
Expand Down Expand Up @@ -600,7 +613,7 @@ bool TextBuffer::IncrementCircularBuffer(const bool inVtMode)
// the current background color, but with no meta attributes set.
fillAttributes.SetStandardErase();
}
GetRowByOffset(0).Discard();
DiscardRowByOffset(0, fillAttributes);
{
// Now proceed to increment.
// Incrementing it will cause the next line down to become the new "top" of the window (the new "0" in logical coordinates)
Expand Down Expand Up @@ -737,7 +750,7 @@ TextBuffer::VirtualAllocation TextBuffer::_allocateBuffer(til::size sz, const Te
data += rowStride;
}

whitespaceRow.Reset(attributes);
whitespaceRow.Initialize();
return { std::move(buffer), allocSize };
}

Expand Down Expand Up @@ -872,7 +885,7 @@ void TextBuffer::SetCurrentLineRendition(const LineRendition lineRendition)
{
const auto cursorPosition = GetCursor().GetPosition();
const auto rowIndex = cursorPosition.y;
auto& row = GetRowByOffset(rowIndex);
auto& row = GetMutableRowByOffset(rowIndex);
if (row.GetLineRendition() != lineRendition)
{
row.SetLineRendition(lineRendition);
Expand All @@ -899,7 +912,7 @@ void TextBuffer::ResetLineRenditionRange(const til::CoordType startRow, const ti
{
for (auto row = startRow; row < endRow; row++)
{
GetRowByOffset(row).SetLineRendition(LineRendition::SingleWidth);
GetMutableRowByOffset(row).SetLineRendition(LineRendition::SingleWidth);
}
}

Expand Down Expand Up @@ -945,13 +958,15 @@ til::point TextBuffer::BufferToScreenPosition(const til::point position) const n
// and the default current color attributes
void TextBuffer::Reset()
{
VirtualAlloc(_buffer.ptr.get(), _buffer.size, MEM_RESET, PAGE_READWRITE);

_whitespaceRow.Discard(_currentAttributes);
_whitespaceRow.Initialize();

for (auto& row : _storage)
{
row.Discard();
row.Discard(_currentAttributes);
}

VirtualAlloc(_buffer.ptr.get(), _buffer.size, MEM_RESET, PAGE_READWRITE);
_whitespaceRow.Reset(_currentAttributes);
}

// Routine Description:
Expand Down Expand Up @@ -1088,17 +1103,6 @@ void TextBuffer::TriggerNewTextNotification(const std::wstring_view newText)
}
}

// Routine Description:
// - Retrieves the first row from the underlying buffer.
// Arguments:
// - <none>
// Return Value:
// - reference to the first row.
ROW& TextBuffer::_GetFirstRow() noexcept
{
return GetRowByOffset(0);
}

// Method Description:
// - get delimiter class for buffer cell position
// - used for double click selection and uia word navigation
Expand Down Expand Up @@ -2371,7 +2375,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
const auto newBufferPos = newCursor.GetPosition();
if (newBufferPos.x == 0)
{
auto& newRow = newBuffer.GetRowByOffset(newBufferPos.y);
auto& newRow = newBuffer.GetMutableRowByOffset(newBufferPos.y);
newRow.SetLineRendition(row.GetLineRendition());
}

Expand Down Expand Up @@ -2449,7 +2453,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
// copy attributes from the old row till the end of the new row, and
// move on.
const auto newRowY = newCursor.GetPosition().y;
auto& newRow = newBuffer.GetRowByOffset(newRowY);
auto& newRow = newBuffer.GetMutableRowByOffset(newRowY);
auto newAttrColumn = newCursor.GetPosition().x;
const auto newWidth = newBuffer.GetLineWidth(newRowY);
// Stop when we get to the end of the buffer width, or the new position
Expand Down Expand Up @@ -2574,7 +2578,7 @@ HRESULT TextBuffer::Reflow(TextBuffer& oldBuffer,
// into the new one, and resize the row to match. We'll rely on the
// behavior of ATTR_ROW::Resize to trim down when narrower, or extend
// the last attr when wider.
auto& newRow = newBuffer.GetRowByOffset(newRowY);
auto& newRow = newBuffer.GetMutableRowByOffset(newRowY);
const auto newWidth = newBuffer.GetLineWidth(newRowY);
newRow.TransferAttributes(row.Attributes(), newWidth);

Expand Down
8 changes: 5 additions & 3 deletions src/buffer/out/textBuffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ class TextBuffer final
void CopyProperties(const TextBuffer& OtherBuffer) noexcept;

// row manipulation
const ROW& GetRowByOffset(const til::CoordType index) const noexcept;
ROW& GetRowByOffset(const til::CoordType index) noexcept;
const ROW& GetRowByOffset(til::CoordType index) const noexcept;
ROW& GetMutableRowByOffset(til::CoordType index) noexcept;
void DiscardRowByOffset(til::CoordType index, const TextAttribute& attr) noexcept;

TextBufferCellIterator GetCellDataAt(const til::point at) const;
TextBufferCellIterator GetCellLineDataAt(const til::point at) const;
Expand Down Expand Up @@ -227,6 +228,8 @@ class TextBuffer final

static VirtualAllocation _allocateBuffer(til::size sz, const TextAttribute& attributes, ROW& whitespaceRow, std::vector<ROW>& rows);

const ROW& _getRowByOffset(til::CoordType index) const noexcept;
ROW& _getRowByOffset(til::CoordType index) noexcept;
void _UpdateSize();
void _SetFirstRowIndex(const til::CoordType FirstRowIndex) noexcept;
til::point _GetPreviousFromCursor() const noexcept;
Expand All @@ -235,7 +238,6 @@ class TextBuffer final
// Assist with maintaining proper buffer state for Double Byte character sequences
bool _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute);
bool _AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribute);
ROW& _GetFirstRow() noexcept;
void _ExpandTextRow(til::inclusive_rect& selectionRow) const;
DelimiterClass _GetDelimiterClassAt(const til::point pos, const std::wstring_view wordDelimiters) const noexcept;
til::point _GetWordStartForAccessibility(const til::point target, const std::wstring_view wordDelimiters) const noexcept;
Expand Down
Loading

0 comments on commit 6f1e61e

Please sign in to comment.