From 391c394dac186568a022079056eb2ea40ecd986a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 14 Jul 2020 11:33:19 -0700 Subject: [PATCH 1/3] commit attr runs less frequently. accumulate lengths. --- src/buffer/out/Row.cpp | 121 +++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 46 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 6c82b718fb9..bba10ed8f93 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -160,66 +160,95 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co // If we're given a right-side column limit, use it. Otherwise, the write limit is the final column index available in the char row. const auto finalColumnInRow = limitRight.value_or(_charRow.size() - 1); - while (it && currentIndex <= finalColumnInRow) + if (it) { - // Fill the color if the behavior isn't set to keeping the current color. - if (it->TextAttrBehavior() != TextAttributeBehavior::Current) - { - const TextAttributeRun attrRun{ 1, it->TextAttr() }; - LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &attrRun, 1 }, - currentIndex, - currentIndex, - _charRow.size())); - } + // Accumulate usages of the same color so we can spend less time in InsertAttrRuns rewriting it. + auto currentColor = it->TextAttr(); + size_t colorUses = 0; + size_t colorStarts = index; - // Fill the text if the behavior isn't set to saying there's only a color stored in this iterator. - if (it->TextAttrBehavior() != TextAttributeBehavior::StoredOnly) + while (it && currentIndex <= finalColumnInRow) { - const bool fillingLastColumn = currentIndex == finalColumnInRow; - - // TODO: MSFT: 19452170 - We need to ensure when writing any trailing byte that the one to the left - // is a matching leading byte. Likewise, if we're writing a leading byte, we need to make sure we still have space in this loop - // for the trailing byte coming up before writing it. - - // If we're trying to fill the first cell with a trailing byte, pad it out instead by clearing it. - // Don't increment iterator. We'll advance the index and try again with this value on the next round through the loop. - if (currentIndex == 0 && it->DbcsAttr().IsTrailing()) + // Fill the color if the behavior isn't set to keeping the current color. + if (it->TextAttrBehavior() != TextAttributeBehavior::Current) { - _charRow.ClearCell(currentIndex); + // If the color of this cell is the same as the run we're currently on, + // just increment the counter. + if (currentColor == it->TextAttr()) + { + ++colorUses; + } + else + { + // Otherwise, commit this color into the run and save off the new one. + const TextAttributeRun run{ colorUses, currentColor }; + // Now commit the new color runs into the attr row. + LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &run, 1 }, + index, + currentIndex - 1, + _charRow.size())); + currentColor = it->TextAttr(); + colorUses = 1; + colorStarts = currentIndex; + } } - // If we're trying to fill the last cell with a leading byte, pad it out instead by clearing it. - // Don't increment iterator. We'll exit because we couldn't write a lead at the end of a line. - else if (fillingLastColumn && it->DbcsAttr().IsLeading()) + + // Fill the text if the behavior isn't set to saying there's only a color stored in this iterator. + if (it->TextAttrBehavior() != TextAttributeBehavior::StoredOnly) { - _charRow.ClearCell(currentIndex); - _charRow.SetDoubleBytePadded(true); + const bool fillingLastColumn = currentIndex == finalColumnInRow; + + // TODO: MSFT: 19452170 - We need to ensure when writing any trailing byte that the one to the left + // is a matching leading byte. Likewise, if we're writing a leading byte, we need to make sure we still have space in this loop + // for the trailing byte coming up before writing it. + + // If we're trying to fill the first cell with a trailing byte, pad it out instead by clearing it. + // Don't increment iterator. We'll advance the index and try again with this value on the next round through the loop. + if (currentIndex == 0 && it->DbcsAttr().IsTrailing()) + { + _charRow.ClearCell(currentIndex); + } + // If we're trying to fill the last cell with a leading byte, pad it out instead by clearing it. + // Don't increment iterator. We'll exit because we couldn't write a lead at the end of a line. + else if (fillingLastColumn && it->DbcsAttr().IsLeading()) + { + _charRow.ClearCell(currentIndex); + _charRow.SetDoubleBytePadded(true); + } + // Otherwise, copy the data given and increment the iterator. + else + { + _charRow.DbcsAttrAt(currentIndex) = it->DbcsAttr(); + _charRow.GlyphAt(currentIndex) = it->Chars(); + ++it; + } + + // If we're asked to (un)set the wrap status and we just filled the last column with some text... + // NOTE: + // - wrap = std::nullopt --> don't change the wrap value + // - wrap = true --> we're filling cells as a steam, consider this a wrap + // - wrap = false --> we're filling cells as a block, unwrap + if (wrap.has_value() && fillingLastColumn) + { + // set wrap status on the row to parameter's value. + _charRow.SetWrapForced(wrap.value()); + } } - // Otherwise, copy the data given and increment the iterator. else { - _charRow.DbcsAttrAt(currentIndex) = it->DbcsAttr(); - _charRow.GlyphAt(currentIndex) = it->Chars(); ++it; } - // If we're asked to (un)set the wrap status and we just filled the last column with some text... - // NOTE: - // - wrap = std::nullopt --> don't change the wrap value - // - wrap = true --> we're filling cells as a steam, consider this a wrap - // - wrap = false --> we're filling cells as a block, unwrap - if (wrap.has_value() && fillingLastColumn) - { - // set wrap status on the row to parameter's value. - _charRow.SetWrapForced(wrap.value()); - } - } - else - { - ++it; + // Move to the next cell for the next time through the loop. + ++currentIndex; } - // Move to the next cell for the next time through the loop. - ++currentIndex; + // Now commit the final color into the attr row + const TextAttributeRun run{ colorUses, currentColor }; + LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &run, 1 }, + colorStarts, + currentIndex - 1, + _charRow.size())); } return it; From d38a0f16ee4ffda303ace1b0eea14930868e4f97 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Tue, 14 Jul 2020 16:42:31 -0700 Subject: [PATCH 2/3] Only try to commit the run if it was attempting to change colors at all. --- src/buffer/out/Row.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index bba10ed8f93..5d8395f900b 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -244,11 +244,14 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co } // Now commit the final color into the attr row - const TextAttributeRun run{ colorUses, currentColor }; - LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &run, 1 }, - colorStarts, - currentIndex - 1, - _charRow.size())); + if (colorUses) + { + const TextAttributeRun run{ colorUses, currentColor }; + LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &run, 1 }, + colorStarts, + currentIndex - 1, + _charRow.size())); + } } return it; From d23eb840614b6e4a792db8c649bcccbc0f1933db Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 17 Jul 2020 10:09:49 -0700 Subject: [PATCH 3/3] Use the correct starting index as we move through the row, not just the initial index of the function call. --- src/buffer/out/Row.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 5d8395f900b..2718aa14229 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -184,7 +184,7 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co const TextAttributeRun run{ colorUses, currentColor }; // Now commit the new color runs into the attr row. LOG_IF_FAILED(_attrRow.InsertAttrRuns({ &run, 1 }, - index, + colorStarts, currentIndex - 1, _charRow.size())); currentColor = it->TextAttr();