Skip to content

Commit

Permalink
Reduce integer type casts in VtEngine (#13097)
Browse files Browse the repository at this point in the history
This commit is one of the more difficult rewrites that were necessary as part
of #4015, but still simple enough that it can be done as a separate commit.
The search for the `lastNonSpace` was replaced with a simpler
`std::string_view::find_last_not_of`.

## Validation Steps Performed
ConPTY appears to work ✅
  • Loading branch information
lhecker authored May 16, 2022
1 parent 6ffc3dc commit 003cbe7
Showing 1 changed file with 17 additions and 28 deletions.
45 changes: 17 additions & 28 deletions src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,17 +348,17 @@ using namespace Microsoft::Console::Types;
_bufferLine.clear();
_bufferLine.reserve(clusters.size());

short totalWidth = 0;
size_t totalWidth = 0;
for (const auto& cluster : clusters)
{
_bufferLine.append(cluster.GetText());
RETURN_IF_FAILED(ShortAdd(totalWidth, gsl::narrow<short>(cluster.GetColumns()), &totalWidth));
totalWidth += cluster.GetColumns();
}

RETURN_IF_FAILED(VtEngine::_WriteTerminalAscii(_bufferLine));

// Update our internal tracker of the cursor's position
_lastText.X += totalWidth;
_lastText.X += gsl::narrow<short>(totalWidth);

return S_OK;
}
Expand All @@ -384,38 +384,29 @@ using namespace Microsoft::Console::Types;

_bufferLine.clear();
_bufferLine.reserve(clusters.size());
short totalWidth = 0;
size_t totalWidth = 0;
for (const auto& cluster : clusters)
{
_bufferLine.append(cluster.GetText());
RETURN_IF_FAILED(ShortAdd(totalWidth, static_cast<short>(cluster.GetColumns()), &totalWidth));
totalWidth += cluster.GetColumns();
}
const auto cchLine = _bufferLine.size();

auto foundNonspace = false;
size_t lastNonSpace = 0;
for (size_t i = 0; i < cchLine; i++)
{
if (_bufferLine.at(i) != L'\x20')
{
lastNonSpace = i;
foundNonspace = true;
}
}
const auto spaceIndex = _bufferLine.find_last_not_of(L' ');
const auto foundNonspace = spaceIndex != decltype(_bufferLine)::npos;
const auto nonSpaceLength = foundNonspace ? spaceIndex + 1 : 0;

// Examples:
// - " ":
// cch = 2, lastNonSpace = 0, foundNonSpace = false
// cch-lastNonSpace = 2 -> good
// cch-lastNonSpace-(0) = 2 -> good
// cch = 2, spaceIndex = 0, foundNonSpace = false
// cch-nonSpaceLength = 2
// - "A "
// cch = 2, lastNonSpace = 0, foundNonSpace = true
// cch-lastNonSpace = 2 -> bad
// cch-lastNonSpace-(1) = 1 -> good
// cch = 2, spaceIndex = 0, foundNonSpace = true
// cch-nonSpaceLength = 1
// - "AA"
// cch = 2, lastNonSpace = 1, foundNonSpace = true
// cch-lastNonSpace = 1 -> bad
// cch-lastNonSpace-(1) = 0 -> good
const auto numSpaces = cchLine - lastNonSpace - (foundNonspace ? 1 : 0);
// cch = 2, spaceIndex = 1, foundNonSpace = true
// cch-nonSpaceLength = 0
const auto numSpaces = cchLine - nonSpaceLength;

// Optimizations:
// If there are lots of spaces at the end of the line, we can try to Erase
Expand Down Expand Up @@ -460,9 +451,7 @@ using namespace Microsoft::Console::Types;
const auto removeSpaces = !lineWrapped && (useEraseChar ||
_clearedAllThisFrame ||
(_newBottomLine && printingBottomLine && bgMatched));
const auto cchActual = removeSpaces ?
(cchLine - numSpaces) :
cchLine;
const auto cchActual = removeSpaces ? nonSpaceLength : cchLine;

const auto columnsActual = removeSpaces ?
(totalWidth - numSpaces) :
Expand Down

0 comments on commit 003cbe7

Please sign in to comment.