Skip to content

Commit

Permalink
Move AdaptDispatch::_FillRect into TextBuffer (#15541)
Browse files Browse the repository at this point in the history
This commit makes 2 changes:
* Expose dirty-range information from `ROW::CopyTextFrom`
  This will allow us to call `TriggerRedraw`, which is an aspect
  I haven't previously considered as something this API needs.
* Add a `FillRect` API to `TextBuffer` and refactor `AdeptDispatch`
  to use that API. Even if we determine that the new text APIs are
  unfit (for instance too difficult to use), this will make it simpler
  to write efficient implementations right inside `TextBuffer`.

Since the new `FillRect` API lacks bounds checks the way `WriteLine`
has them, it breaks `AdaptDispatch::_EraseAll` which failed to adjust
the bottom parameter after scrolling the contents. This would result
in more rows being erased than intended.

## Validation Steps Performed
* `chcp 65001`
* Launch `pwsh`
* ``"`e[29483`$x"`` fills the viewport with cats ✅
* `ResizeTraditional` still doesn't work any worse than it used to ✅
  • Loading branch information
lhecker authored Jun 14, 2023
1 parent 8f8c79f commit c183d12
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 71 deletions.
57 changes: 31 additions & 26 deletions src/buffer/out/Row.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ void ROW::TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& a

void ROW::CopyFrom(const ROW& source)
{
til::CoordType begin = 0;
CopyTextFrom(0, til::CoordTypeMax, source, begin, til::CoordTypeMax);
RowCopyTextFromState state{ .source = source };
CopyTextFrom(state);
TransferAttributes(source.Attributes(), _columnCount);
_lineRendition = source._lineRendition;
_wrapForced = source._wrapForced;
Expand Down Expand Up @@ -451,46 +451,51 @@ catch (...)
charsConsumed = ch - chBeg;
}

til::CoordType ROW::CopyTextFrom(til::CoordType columnBegin, til::CoordType columnLimit, const ROW& other, til::CoordType& otherBegin, til::CoordType otherLimit)
void ROW::CopyTextFrom(RowCopyTextFromState& state)
try
{
const auto otherColBeg = other._clampedColumnInclusive(otherBegin);
const auto otherColLimit = other._clampedColumnInclusive(otherLimit);
std::span<uint16_t> charOffsets;
auto& source = state.source;
const auto sourceColBeg = source._clampedColumnInclusive(state.sourceColumnBegin);
const auto sourceColLimit = source._clampedColumnInclusive(state.sourceColumnLimit);
std::span<const uint16_t> charOffsets;
std::wstring_view chars;

if (otherColBeg < otherColLimit)
if (sourceColBeg < sourceColLimit)
{
charOffsets = other._charOffsets.subspan(otherColBeg, static_cast<size_t>(otherColLimit) - otherColBeg + 1);
charOffsets = source._charOffsets.subspan(sourceColBeg, static_cast<size_t>(sourceColLimit) - sourceColBeg + 1);
const auto charsOffset = charOffsets.front() & CharOffsetsMask;
// We _are_ using span. But C++ decided that string_view and span aren't convertible.
// _chars is a std::span for performance and because it refers to raw, shared memory.
#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead (bounds.1).
chars = { other._chars.data() + charsOffset, other._chars.size() - charsOffset };
chars = { source._chars.data() + charsOffset, source._chars.size() - charsOffset };
}

WriteHelper h{ *this, columnBegin, columnLimit, chars };
// If we were to copy text from ourselves, we'd overwrite
// our _charOffsets and break Finish() which reads from it.
if (!h.IsValid() || this == &other)
{
assert(false); // You probably shouldn't call this function in the first place.
return h.colBeg;
}
// Any valid charOffsets array is at least 2 elements long (the 1st element is the start offset and the 2nd
// element is the length of the first glyph) and begins/ends with a non-trailer offset. We don't really
// need to test for the end offset, since `WriteHelper::WriteWithOffsets` already takes care of that.
if (charOffsets.size() < 2 || WI_IsFlagSet(charOffsets.front(), CharOffsetsTrailer))
WriteHelper h{ *this, state.columnBegin, state.columnLimit, chars };

if (!h.IsValid() ||
// If we were to copy text from ourselves, we'd overwrite
// our _charOffsets and break Finish() which reads from it.
this == &state.source ||
// Any valid charOffsets array is at least 2 elements long (the 1st element is the start offset and the 2nd
// element is the length of the first glyph) and begins/ends with a non-trailer offset. We don't really
// need to test for the end offset, since `WriteHelper::WriteWithOffsets` already takes care of that.
charOffsets.size() < 2 || WI_IsFlagSet(charOffsets.front(), CharOffsetsTrailer))
{
assert(false);
otherBegin = other.size();
return h.colBeg;
state.columnEnd = h.colBeg;
state.columnBeginDirty = h.colBeg;
state.columnEndDirty = h.colBeg;
state.sourceColumnEnd = source._columnCount;
return;
}

h.CopyTextFrom(charOffsets);
h.Finish();

otherBegin += h.colEnd - h.colBeg;
return h.colEndDirty;
// state.columnEnd is computed identical to ROW::ReplaceText. Check it out for more information.
state.columnEnd = h.charsConsumed == chars.size() ? h.colEnd : h.colLimit;
state.columnBeginDirty = h.colBegDirty;
state.columnEndDirty = h.colEndDirty;
state.sourceColumnEnd = sourceColBeg + h.colEnd - h.colBeg;
}
catch (...)
{
Expand Down
29 changes: 27 additions & 2 deletions src/buffer/out/Row.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Revision History:
#include "OutputCell.hpp"
#include "OutputCellIterator.hpp"

class ROW;
class TextBuffer;

enum class DelimiterClass
Expand All @@ -43,7 +44,7 @@ struct RowWriteState
// The column at which to start writing.
til::CoordType columnBegin = 0; // IN
// The first column which should not be written to anymore.
til::CoordType columnLimit = 0; // IN
til::CoordType columnLimit = til::CoordTypeMax; // IN

// The column 1 past the last glyph that was successfully written into the row. If you need to call
// ReplaceAttributes() to colorize the written range, etc., this is the columnEnd parameter you want.
Expand All @@ -57,6 +58,30 @@ struct RowWriteState
til::CoordType columnEndDirty = 0; // OUT
};

struct RowCopyTextFromState
{
// The row to copy text from.
const ROW& source; // IN
// The column at which to start writing.
til::CoordType columnBegin = 0; // IN
// The first column which should not be written to anymore.
til::CoordType columnLimit = til::CoordTypeMax; // IN
// The column at which to start reading from source.
til::CoordType sourceColumnBegin = 0; // IN
// The first column which should not be read from anymore.
til::CoordType sourceColumnLimit = til::CoordTypeMax; // IN

til::CoordType columnEnd = 0; // OUT
// The first column that got modified by this write operation. In case that the first glyph we write overwrites
// the trailing half of a wide glyph, leadingSpaces will be 1 and this value will be 1 less than colBeg.
til::CoordType columnBeginDirty = 0; // OUT
// This is 1 past the last column that was modified and will be 1 past columnEnd if we overwrote
// the leading half of a wide glyph and had to fill the trailing half with whitespace.
til::CoordType columnEndDirty = 0; // OUT
// This is 1 past the last column that was read from.
til::CoordType sourceColumnEnd = 0; // OUT
};

class ROW final
{
public:
Expand Down Expand Up @@ -109,7 +134,7 @@ class ROW final
void ReplaceAttributes(til::CoordType beginIndex, til::CoordType endIndex, const TextAttribute& newAttr);
void ReplaceCharacters(til::CoordType columnBegin, til::CoordType width, const std::wstring_view& chars);
void ReplaceText(RowWriteState& state);
til::CoordType CopyTextFrom(til::CoordType columnBegin, til::CoordType columnLimit, const ROW& other, til::CoordType& otherBegin, til::CoordType otherLimit);
void CopyTextFrom(RowCopyTextFromState& state);

til::small_rle<TextAttribute, uint16_t, 1>& Attributes() noexcept;
const til::small_rle<TextAttribute, uint16_t, 1>& Attributes() const noexcept;
Expand Down
71 changes: 66 additions & 5 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,15 @@ ROW& TextBuffer::GetRowByOffset(const til::CoordType index)
// Returns a row filled with whitespace and the current attributes, for you to freely use.
ROW& TextBuffer::GetScratchpadRow()
{
return _getRowByOffsetDirect(0);
return GetScratchpadRow(_currentAttributes);
}

// Returns a row filled with whitespace and the given attributes, for you to freely use.
ROW& TextBuffer::GetScratchpadRow(const TextAttribute& attributes)
{
auto& r = _getRowByOffsetDirect(0);
r.Reset(attributes);
return r;
}

#pragma warning(pop)
Expand Down Expand Up @@ -429,10 +437,8 @@ void TextBuffer::ConsumeGrapheme(std::wstring_view& chars) noexcept
chars = til::utf16_pop(chars);
}

// This function is intended for writing regular "lines" of text and only the `state.text` and`state.columnBegin`
// fields are being used, whereas `state.columnLimit` is automatically overwritten by the line width of the given row.
// This allows this function to automatically set the wrap-forced field of the row, which is also the return value.
// The return value indicates to the caller whether the cursor should be moved to the next line.
// This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row.
// You can continue calling the function on the same row as long as state.columnEnd < state.columnLimit.
void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state)
{
auto& r = GetRowByOffset(row);
Expand All @@ -448,6 +454,61 @@ void TextBuffer::WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribu
TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, state.columnEndDirty, row + 1 }));
}

// Fills an area of the buffer with a given fill character(s) and attributes.
void TextBuffer::FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes)
{
if (!rect || fill.empty())
{
return;
}

auto& scratchpad = GetScratchpadRow(attributes);

// The scratchpad row gets reset to whitespace by default, so there's no need to
// initialize it again. Filling with whitespace is the most common operation by far.
if (fill != L" ")
{
RowWriteState state{
.columnLimit = rect.right,
.columnEnd = rect.left,
};

// Fill the scratchpad row with consecutive copies of "fill" up to the amount we need.
//
// We don't just create a single string with N copies of "fill" and write that at once,
// because that might join neighboring combining marks unintentionally.
//
// Building the buffer this way is very wasteful and slow, but it's still 3x
// faster than what we had before and no one complained about that either.
// It's seldom used code and probably not worth optimizing for.
while (state.columnEnd < rect.right)
{
state.columnBegin = state.columnEnd;
state.text = fill;
scratchpad.ReplaceText(state);
}
}

// Fill the given rows with copies of the scratchpad row. That's a little
// slower when filling just a single row, but will be much faster for >1 rows.
{
RowCopyTextFromState state{
.source = scratchpad,
.columnBegin = rect.left,
.columnLimit = rect.right,
.sourceColumnBegin = rect.left,
};

for (auto y = rect.top; y < rect.bottom; ++y)
{
auto& r = GetRowByOffset(y);
r.CopyTextFrom(state);
r.ReplaceAttributes(rect.left, rect.right, attributes);
TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, y, state.columnEndDirty, y + 1 }));
}
}
}

// Routine Description:
// - Writes cells to the output buffer. Writes at the cursor.
// Arguments:
Expand Down
2 changes: 2 additions & 0 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class TextBuffer final

// row manipulation
ROW& GetScratchpadRow();
ROW& GetScratchpadRow(const TextAttribute& attributes);
const ROW& GetRowByOffset(til::CoordType index) const;
ROW& GetRowByOffset(til::CoordType index);

Expand All @@ -98,6 +99,7 @@ class TextBuffer final
// Text insertion functions
static void ConsumeGrapheme(std::wstring_view& chars) noexcept;
void WriteLine(til::CoordType row, bool wrapAtEOL, const TextAttribute& attributes, RowWriteState& state);
void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes);

OutputCellIterator Write(const OutputCellIterator givenIt);

Expand Down
2 changes: 1 addition & 1 deletion src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ bool ConhostInternalGetSet::IsVtInputEnabled() const
void ConhostInternalGetSet::NotifyAccessibilityChange(const til::rect& changedRect)
{
auto& screenInfo = _io.GetActiveOutputBuffer();
if (screenInfo.HasAccessibilityEventing())
if (screenInfo.HasAccessibilityEventing() && changedRect)
{
screenInfo.NotifyAccessibilityEventing(
changedRect.left,
Expand Down
Loading

0 comments on commit c183d12

Please sign in to comment.