Skip to content

Commit

Permalink
Improve Word Navigation/Selection Performance (#4797)
Browse files Browse the repository at this point in the history 
## Summary of the Pull Request
1) Improves the performance of word-recognition operations such as word
   navigation in UIA and selection.

2) Fixes a bug where attempting to find the next word in UIA, when none
   exists, would hang

3) TraceLogging code only runs when somebody is listening

## Detailed Description of the Pull Request / Additional comments
- The concept of a delimiter class got moved to the CharRow.
- The buffer iterator used to save a lot more information than we needed
- I missed updating a tracing function after making GetSelection return
  one text range. That is fixed now.


## Validation Steps Performed
Performed Word Navigation under Narrator and NVDA.
NOTE: The release build should be used when testing to optimize
performance

Closes #4703
  • Loading branch information
msftbot[bot] authored Mar 4, 2020
1 parent c6879d7 commit 267deaa
Show file tree
Hide file tree
Showing 8 changed files with 476 additions and 390 deletions.
27 changes: 27 additions & 0 deletions src/buffer/out/CharRow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,33 @@ std::wstring CharRow::GetText() const
return wstr;
}

// Method Description:
// - get delimiter class for a position in the char row
// - used for double click selection and uia word navigation
// Arguments:
// - column: column to get text data for
// - wordDelimiters: the delimiters defined as a part of the DelimiterClass::DelimiterChar
// Return Value:
// - the delimiter class for the given char
const DelimiterClass CharRow::DelimiterClassAt(const size_t column, const std::wstring_view wordDelimiters) const
{
THROW_HR_IF(E_INVALIDARG, column >= _data.size());

const auto glyph = *GlyphAt(column).begin();
if (glyph <= UNICODE_SPACE)
{
return DelimiterClass::ControlChar;
}
else if (wordDelimiters.find(glyph) != std::wstring_view::npos)
{
return DelimiterClass::DelimiterChar;
}
else
{
return DelimiterClass::RegularChar;
}
}

UnicodeStorage& CharRow::GetUnicodeStorage() noexcept
{
return _pParent->GetUnicodeStorage();
Expand Down
9 changes: 9 additions & 0 deletions src/buffer/out/CharRow.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ Revision History:

class ROW;

enum class DelimiterClass
{
ControlChar,
DelimiterChar,
RegularChar
};

// the characters of one row of screen buffer
// we keep the following values so that we don't write
// more pixels to the screen than we have to:
Expand Down Expand Up @@ -64,6 +71,8 @@ class CharRow final
void ClearGlyph(const size_t column);
std::wstring GetText() const;

const DelimiterClass DelimiterClassAt(const size_t column, const std::wstring_view wordDelimiters) const;

// working with glyphs
const reference GlyphAt(const size_t column) const;
reference GlyphAt(const size_t column);
Expand Down
118 changes: 35 additions & 83 deletions src/buffer/out/textBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,19 @@ Microsoft::Console::Render::IRenderTarget& TextBuffer::GetRenderTarget() noexcep
return _renderTarget;
}

// Method Description:
// - get delimiter class for buffer cell position
// - used for double click selection and uia word navigation
// Arguments:
// - pos: the buffer cell under observation
// - wordDelimiters: the delimiters defined as a part of the DelimiterClass::DelimiterChar
// Return Value:
// - the delimiter class for the given char
const DelimiterClass TextBuffer::_GetDelimiterClassAt(const COORD pos, const std::wstring_view wordDelimiters) const
{
return GetRowByOffset(pos.Y).GetCharRow().DelimiterClassAt(pos.X, wordDelimiters);
}

// Method Description:
// - Get the COORD for the beginning of the word you are on
// Arguments:
Expand Down Expand Up @@ -1001,16 +1014,11 @@ const COORD TextBuffer::_GetWordStartForAccessibility(const COORD target, const
COORD result = target;
const auto bufferSize = GetSize();
bool stayAtOrigin = false;
auto bufferIterator = GetTextDataAt(result);

// ignore left boundary. Continue until readable text found
while (_GetDelimiterClass(*bufferIterator, wordDelimiters) != DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
{
if (bufferSize.DecrementInBounds(result))
{
--bufferIterator;
}
else
if (!bufferSize.DecrementInBounds(result))
{
// first char in buffer is a DelimiterChar or ControlChar
// we can't move any further back
Expand All @@ -1020,13 +1028,9 @@ const COORD TextBuffer::_GetWordStartForAccessibility(const COORD target, const
}

// make sure we expand to the left boundary or the beginning of the word
while (_GetDelimiterClass(*bufferIterator, wordDelimiters) == DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(result, wordDelimiters) == DelimiterClass::RegularChar)
{
if (bufferSize.DecrementInBounds(result))
{
--bufferIterator;
}
else
if (!bufferSize.DecrementInBounds(result))
{
// first char in buffer is a RegularChar
// we can't move any further back
Expand All @@ -1035,7 +1039,7 @@ const COORD TextBuffer::_GetWordStartForAccessibility(const COORD target, const
}

// move off of delimiter and onto word start
if (!stayAtOrigin && _GetDelimiterClass(*bufferIterator, wordDelimiters) != DelimiterClass::RegularChar)
if (!stayAtOrigin && _GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
{
bufferSize.IncrementInBounds(result);
}
Expand All @@ -1054,17 +1058,16 @@ const COORD TextBuffer::_GetWordStartForSelection(const COORD target, const std:
{
COORD result = target;
const auto bufferSize = GetSize();
auto bufferIterator = GetTextDataAt(result);
const auto initialDelimiter = _GetDelimiterClass(*bufferIterator, wordDelimiters);

const auto initialDelimiter = _GetDelimiterClassAt(result, wordDelimiters);

// expand left until we hit the left boundary or a different delimiter class
while (result.X > bufferSize.Left() && (_GetDelimiterClass(*bufferIterator, wordDelimiters) == initialDelimiter))
while (result.X > bufferSize.Left() && (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter))
{
bufferSize.DecrementInBounds(result);
--bufferIterator;
}

if (_GetDelimiterClass(*bufferIterator, wordDelimiters) != initialDelimiter)
if (_GetDelimiterClassAt(result, wordDelimiters) != initialDelimiter)
{
// move off of delimiter
bufferSize.IncrementInBounds(result);
Expand Down Expand Up @@ -1116,31 +1119,20 @@ const COORD TextBuffer::_GetWordEndForAccessibility(const COORD target, const st
{
const auto bufferSize = GetSize();
COORD result = target;
auto bufferIterator = GetTextDataAt(result);

// ignore right boundary. Continue through readable text found
while (_GetDelimiterClass(*bufferIterator, wordDelimiters) == DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(result, wordDelimiters) == DelimiterClass::RegularChar)
{
if (bufferSize.IncrementInBounds(result, true))
if (!bufferSize.IncrementInBounds(result, true))
{
++bufferIterator;
}
else
{
// last char in buffer is a RegularChar
// we can't move any further forward
break;
}
}

// make sure we expand to the beginning of the NEXT word
while (_GetDelimiterClass(*bufferIterator, wordDelimiters) != DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(result, wordDelimiters) != DelimiterClass::RegularChar)
{
if (bufferSize.IncrementInBounds(result, true))
{
++bufferIterator;
}
else
if (!bufferSize.IncrementInBounds(result, true))
{
// we are at the EndInclusive COORD
// this signifies that we must include the last char in the buffer
Expand All @@ -1162,25 +1154,23 @@ const COORD TextBuffer::_GetWordEndForAccessibility(const COORD target, const st
const COORD TextBuffer::_GetWordEndForSelection(const COORD target, const std::wstring_view wordDelimiters) const
{
const auto bufferSize = GetSize();
COORD result = target;
auto bufferIterator = GetTextDataAt(result);

// can't expand right
if (target.X == bufferSize.RightInclusive())
{
return result;
return target;
}

const auto initialDelimiter = _GetDelimiterClass(*bufferIterator, wordDelimiters);
COORD result = target;
const auto initialDelimiter = _GetDelimiterClassAt(result, wordDelimiters);

// expand right until we hit the right boundary or a different delimiter class
while (result.X < bufferSize.RightInclusive() && (_GetDelimiterClass(*bufferIterator, wordDelimiters) == initialDelimiter))
while (result.X < bufferSize.RightInclusive() && (_GetDelimiterClassAt(result, wordDelimiters) == initialDelimiter))
{
bufferSize.IncrementInBounds(result);
++bufferIterator;
}

if (_GetDelimiterClass(*bufferIterator, wordDelimiters) != initialDelimiter)
if (_GetDelimiterClassAt(result, wordDelimiters) != initialDelimiter)
{
// move off of delimiter
bufferSize.DecrementInBounds(result);
Expand All @@ -1203,20 +1193,15 @@ bool TextBuffer::MoveToNextWord(COORD& pos, const std::wstring_view wordDelimite
auto copy = pos;
const auto bufferSize = GetSize();

auto text = GetTextDataAt(copy)->data();
auto delimiterClass = _GetDelimiterClass(text, wordDelimiters);

// started on a word, continue until the end of the word
while (delimiterClass == DelimiterClass::RegularChar)
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;
}
text = GetTextDataAt(copy)->data();
delimiterClass = _GetDelimiterClass(text, wordDelimiters);
}

// we are already on/past the last RegularChar
Expand All @@ -1226,16 +1211,14 @@ bool TextBuffer::MoveToNextWord(COORD& pos, const std::wstring_view wordDelimite
}

// on whitespace, continue until the beginning of the next word
while (delimiterClass != DelimiterClass::RegularChar)
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;
}
text = GetTextDataAt(copy)->data();
delimiterClass = _GetDelimiterClass(text, wordDelimiters);
}

// successful move, copy result out
Expand All @@ -1256,64 +1239,33 @@ bool TextBuffer::MoveToPreviousWord(COORD& pos, std::wstring_view wordDelimiters
auto copy = pos;
auto bufferSize = GetSize();

auto text = GetTextDataAt(copy)->data();
auto delimiterClass = _GetDelimiterClass(text, wordDelimiters);

// started on whitespace/delimiter, continue until the end of the previous word
while (delimiterClass != DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(copy, wordDelimiters) != DelimiterClass::RegularChar)
{
if (!bufferSize.DecrementInBounds(copy))
{
// first char in buffer is a DelimiterChar or ControlChar
// there is no previous word
return false;
}
text = GetTextDataAt(copy)->data();
delimiterClass = _GetDelimiterClass(text, wordDelimiters);
}

// on a word, continue until the beginning of the word
while (delimiterClass == DelimiterClass::RegularChar)
while (_GetDelimiterClassAt(copy, wordDelimiters) == DelimiterClass::RegularChar)
{
if (!bufferSize.DecrementInBounds(copy))
{
// first char in buffer is a RegularChar
// there is no previous word
return false;
}
text = GetTextDataAt(copy)->data();
delimiterClass = _GetDelimiterClass(text, wordDelimiters);
}

// successful move, copy result out
pos = copy;
return true;
}

// Method Description:
// - get delimiter class for buffer cell data
// - used for double click selection and uia word navigation
// Arguments:
// - cellChar: the char saved to the buffer cell under observation
// - wordDelimiters: the delimiters defined as a part of the DelimiterClass::DelimiterChar
// Return Value:
// - the delimiter class for the given char
TextBuffer::DelimiterClass TextBuffer::_GetDelimiterClass(const std::wstring_view cellChar, const std::wstring_view wordDelimiters) const noexcept
{
if (cellChar.at(0) <= UNICODE_SPACE)
{
return DelimiterClass::ControlChar;
}
else if (wordDelimiters.find(cellChar) != std::wstring_view::npos)
{
return DelimiterClass::DelimiterChar;
}
else
{
return DelimiterClass::RegularChar;
}
}

// Method Description:
// - Determines the line-by-line rectangles based on two COORDs
// - expands the rectangles to support wide glyphs
Expand Down
8 changes: 1 addition & 7 deletions src/buffer/out/textBuffer.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,7 @@ class TextBuffer final

void _ExpandTextRow(SMALL_RECT& selectionRow) const;

enum class DelimiterClass
{
ControlChar,
DelimiterChar,
RegularChar
};
DelimiterClass _GetDelimiterClass(const std::wstring_view cellChar, const std::wstring_view wordDelimiters) const noexcept;
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;
Expand Down
2 changes: 1 addition & 1 deletion src/types/ScreenInfoUiaProviderBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_
return hr;
}

UiaTracing::TextProvider::GetSelection(*this);
UiaTracing::TextProvider::GetSelection(*this, *range.Get());
return S_OK;
}

Expand Down
4 changes: 4 additions & 0 deletions src/types/UiaTextRangeBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,10 @@ void UiaTextRangeBase::_moveEndpointByUnitWord(_In_ const int moveCount,
resultPos = bufferEnd;
(*pAmountMoved)++;
}
else
{
success = false;
}
break;
}
case MovementDirection::Backward:
Expand Down
Loading

0 comments on commit 267deaa

Please sign in to comment.