Skip to content

Commit

Permalink
Add optimization to get a11y next word (#7789)
Browse files Browse the repository at this point in the history
This performs a minor refactor on `TextBuffer::MoveToNextWord` that
relies more heavily on `TextBuffer::GetWordEnd`. Now, the logic is
simplified and looks more like `MoveToPreviousWord`.

This refactor required me to move the `lastCharPos` optimization down to
`GetWordEnd`. So word expansion gets this optimization for free now.

### WPR Traces
The percentages below represent the weight that a function call had. The
test scenario included moving by word on the CMD welcome message until
the last word was reached. Inspect.exe was used to limit any additional
calls that are generally performed by a screen reader.

| function   | current | branch |
| --         | --      | --     |
| `UIA:Move` | 34.55%  | 29.52% |

There is an improvement of about 5% in a release build of ConHost.

NOTE: `UIA::Move` already calls `Expand` after a move operation is
performed. I'm using this data to represent a performance improvement
across both functions.

Contributes to #5243
  • Loading branch information
carlos-zamora authored Sep 30, 2020
1 parent da4ca86 commit 386ae04
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 36 deletions.
56 changes: 21 additions & 35 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,8 @@ const COORD TextBuffer::GetWordEnd(const COORD target, const std::wstring_view w

if (accessibilityMode)
{
return _GetWordEndForAccessibility(target, wordDelimiters);
const auto lastCharPos{ GetLastNonSpaceCharacter() };
return _GetWordEndForAccessibility(target, wordDelimiters, lastCharPos);
}
else
{
Expand All @@ -1139,13 +1140,20 @@ const COORD TextBuffer::GetWordEnd(const COORD target, const std::wstring_view w
// Arguments:
// - target - a COORD on the word you are currently on
// - wordDelimiters - what characters are we considering for the separation of words
// - lastCharPos - the position of the last nonspace character in the text buffer (to improve performance)
// Return Value:
// - The COORD for the first character of the next readable "word". If no next word, return one past the end of the buffer
const COORD TextBuffer::_GetWordEndForAccessibility(const COORD target, const std::wstring_view wordDelimiters) const
const COORD TextBuffer::_GetWordEndForAccessibility(const COORD target, const std::wstring_view wordDelimiters, const COORD lastCharPos) const
{
const auto bufferSize = GetSize();
COORD result = target;

// Check if we're already on/past the last RegularChar
if (bufferSize.CompareInBounds(result, lastCharPos, true) >= 0)
{
return bufferSize.EndExclusive();
}

// ignore right boundary. Continue through readable text found
while (_GetDelimiterClassAt(result, wordDelimiters) == DelimiterClass::RegularChar)
{
Expand All @@ -1155,6 +1163,12 @@ const COORD TextBuffer::_GetWordEndForAccessibility(const COORD target, const st
}
}

// we are already on/past the last RegularChar
if (bufferSize.CompareInBounds(result, lastCharPos, true) >= 0)
{
return bufferSize.EndExclusive();
}

// make sure we expand to the beginning of the NEXT word
while (_GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
{
Expand Down Expand Up @@ -1256,44 +1270,16 @@ void TextBuffer::_PruneHyperlinks()
// - pos - The COORD for the first character on the "word" (inclusive)
bool TextBuffer::MoveToNextWord(COORD& pos, const std::wstring_view wordDelimiters, COORD lastCharPos) const
{
auto copy = pos;
const auto bufferSize = GetSize();
// move to the beginning of the next word
// NOTE: _GetWordEnd...() returns the exclusive position of the "end of the word"
// This is also the inclusive start of the next word.
auto copy{ _GetWordEndForAccessibility(pos, wordDelimiters, lastCharPos) };

// Already at the end. Can't move forward.
if (pos == bufferSize.EndExclusive())
if (copy == GetSize().EndExclusive())
{
return false;
}

// started on a word, continue until the end of the word
while (_GetDelimiterClassAt(copy, wordDelimiters) == DelimiterClass::RegularChar)
{
if (!bufferSize.IncrementInBounds(copy))
{
// last char in buffer is a RegularChar
// thus there is no next word
return false;
}
}

// we are already on/past the last RegularChar
if (bufferSize.CompareInBounds(copy, lastCharPos) >= 0)
{
return false;
}

// on whitespace, continue until the beginning of the next word
while (_GetDelimiterClassAt(copy, wordDelimiters) != DelimiterClass::RegularChar)
{
if (!bufferSize.IncrementInBounds(copy))
{
// last char in buffer is a DelimiterChar or ControlChar
// there is no next word
return false;
}
}

// successful move, copy result out
pos = copy;
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class TextBuffer final
const DelimiterClass _GetDelimiterClassAt(const COORD pos, const std::wstring_view wordDelimiters) const;
const COORD _GetWordStartForAccessibility(const COORD target, const std::wstring_view wordDelimiters) const;
const COORD _GetWordStartForSelection(const COORD target, const std::wstring_view wordDelimiters) const;
const COORD _GetWordEndForAccessibility(const COORD target, const std::wstring_view wordDelimiters) const;
const COORD _GetWordEndForAccessibility(const COORD target, const std::wstring_view wordDelimiters, const COORD lastCharPos) const;
const COORD _GetWordEndForSelection(const COORD target, const std::wstring_view wordDelimiters) const;

void _PruneHyperlinks();
Expand Down

0 comments on commit 386ae04

Please sign in to comment.