Skip to content

Commit

Permalink
Limit Concept of TextBuffer's Size in UIA (#4523)
Browse files Browse the repository at this point in the history 
## Summary of the Pull Request
In UIA Providers, update the concept of the size of the text buffer to just go down to the virtual bottom. This significantly increases performance to the point that it can even be used in the Debug build.

## PR Checklist
* [x] Closes #4485 
* [x] CLA signed.

## Detailed Description of the Pull Request / Additional comments
We already actually have this concept exposed to us via the IUiaData. So we're just leveraging that and putting it in a helper function `_getBufferSize()`.

## Validation Steps Performed
Tested word nav on Narrator (previously hung). Now it works on the Debug build. Previously, using the release build was necessary to be able to test this feature.
  • Loading branch information
@carlos-zamora
carlos-zamora authored Feb 10, 2020
1 parent 4f6916c commit 681a0db
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 17 deletions.
52 changes: 39 additions & 13 deletions src/types/UiaTextRangeBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,9 @@ const COORD UiaTextRangeBase::GetEndpoint(TextPatternRangeEndpoint endpoint) con
// - val - the value that it will be set to
// Return Value:
// - true if range is degenerate, false otherwise.
bool UiaTextRangeBase::SetEndpoint(TextPatternRangeEndpoint endpoint, const COORD val)
bool UiaTextRangeBase::SetEndpoint(TextPatternRangeEndpoint endpoint, const COORD val) noexcept
{
const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();
switch (endpoint)
{
case TextPatternRangeEndpoint::TextPatternRangeEndpoint_End:
Expand Down Expand Up @@ -301,7 +301,7 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc
try
{
const auto& buffer = _pData->GetTextBuffer();
const auto bufferSize = buffer.GetSize();
const auto bufferSize = _getBufferSize();
const auto bufferEnd = bufferSize.EndExclusive();

if (unit == TextUnit::TextUnit_Character)
Expand All @@ -314,6 +314,13 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc
// expand to word
_start = buffer.GetWordStart(_start, _wordDelimiters, true);
_end = buffer.GetWordEnd(_start, _wordDelimiters, true);

// GetWordEnd may return the actual end of the TextBuffer.
// If so, just set it to this value of bufferEnd
if (!bufferSize.IsInBounds(_end))
{
_end = bufferEnd;
}
}
else if (unit <= TextUnit::TextUnit_Line)
{
Expand Down Expand Up @@ -359,7 +366,7 @@ try
*ppRetVal = nullptr;

const std::wstring queryText{ text, SysStringLen(text) };
const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();
const auto sensitivity = ignoreCase ? Search::Sensitivity::CaseInsensitive : Search::Sensitivity::CaseSensitive;

auto searchDirection = Search::Direction::Forward;
Expand Down Expand Up @@ -436,7 +443,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_
// set of coords.
std::vector<double> coords;

const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();

// these viewport vars are converted to the buffer coordinate space
const auto viewport = bufferSize.ConvertToOrigin(_pData->GetViewport());
Expand Down Expand Up @@ -970,6 +977,25 @@ const unsigned int UiaTextRangeBase::_getViewportHeight(const SMALL_RECT viewpor
return viewport.Bottom - viewport.Top + 1;
}

// Routine Description:
// - Gets a viewport representing where valid text may be in the TextBuffer
// - Use this instead of `textBuffer.GetSize()`. This improves performance
// because it's a smaller space to have to search through
// Arguments:
// - <none>
// Return Value:
// - A viewport representing the portion of the TextBuffer that has valid text
const Viewport UiaTextRangeBase::_getBufferSize() const noexcept
{
// we need to add 1 to the X/Y of textBufferEnd
// because we want the returned viewport to include this COORD
const auto textBufferEnd = _pData->GetTextBufferEndPosition();
const auto width = base::ClampAdd<short>(1, textBufferEnd.X);
const auto height = base::ClampAdd<short>(1, textBufferEnd.Y);

return Viewport::FromDimensions({ 0, 0 }, width, height);
}

// Routine Description:
// - adds the relevant coordinate points from the row to coords.
// - it is assumed that startAnchor and endAnchor are within the same row
Expand Down Expand Up @@ -1038,7 +1064,7 @@ void UiaTextRangeBase::_getBoundingRect(_In_ const COORD startAnchor, _In_ const
void UiaTextRangeBase::_moveEndpointByUnitCharacter(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd)
_In_ const bool preventBufferEnd) noexcept
{
*pAmountMoved = 0;

Expand All @@ -1049,7 +1075,7 @@ void UiaTextRangeBase::_moveEndpointByUnitCharacter(_In_ const int moveCount,

const bool allowBottomExclusive = !preventBufferEnd;
const MovementDirection moveDirection = (moveCount > 0) ? MovementDirection::Forward : MovementDirection::Backward;
const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();

bool success = true;
auto target = GetEndpoint(endpoint);
Expand Down Expand Up @@ -1106,10 +1132,10 @@ void UiaTextRangeBase::_moveEndpointByUnitWord(_In_ const int moveCount,
const bool allowBottomExclusive = !preventBufferEnd;
const MovementDirection moveDirection = (moveCount > 0) ? MovementDirection::Forward : MovementDirection::Backward;
const auto& buffer = _pData->GetTextBuffer();
const auto bufferSize = buffer.GetSize();
const auto bufferSize = _getBufferSize();
const auto bufferOrigin = bufferSize.Origin();
const auto bufferEnd = bufferSize.EndExclusive();
const auto lastCharPos = buffer.GetLastNonSpaceCharacter();
const auto lastCharPos = buffer.GetLastNonSpaceCharacter(bufferSize);

auto resultPos = GetEndpoint(endpoint);
auto nextPos = resultPos;
Expand Down Expand Up @@ -1180,7 +1206,7 @@ void UiaTextRangeBase::_moveEndpointByUnitWord(_In_ const int moveCount,
void UiaTextRangeBase::_moveEndpointByUnitLine(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd)
_In_ const bool preventBufferEnd) noexcept
{
*pAmountMoved = 0;

Expand All @@ -1191,7 +1217,7 @@ void UiaTextRangeBase::_moveEndpointByUnitLine(_In_ const int moveCount,

const bool allowBottomExclusive = !preventBufferEnd;
const MovementDirection moveDirection = (moveCount > 0) ? MovementDirection::Forward : MovementDirection::Backward;
const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();

bool success = true;
auto resultPos = GetEndpoint(endpoint);
Expand Down Expand Up @@ -1264,7 +1290,7 @@ void UiaTextRangeBase::_moveEndpointByUnitLine(_In_ const int moveCount,
void UiaTextRangeBase::_moveEndpointByUnitDocument(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd)
_In_ const bool preventBufferEnd) noexcept
{
*pAmountMoved = 0;

Expand All @@ -1274,7 +1300,7 @@ void UiaTextRangeBase::_moveEndpointByUnitDocument(_In_ const int moveCount,
}

const MovementDirection moveDirection = (moveCount > 0) ? MovementDirection::Forward : MovementDirection::Backward;
const auto bufferSize = _pData->GetTextBuffer().GetSize();
const auto bufferSize = _getBufferSize();

const auto target = GetEndpoint(endpoint);
switch (moveDirection)
Expand Down
9 changes: 5 additions & 4 deletions src/types/UiaTextRangeBase.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ namespace Microsoft::Console::Types

const IdType GetId() const noexcept;
const COORD GetEndpoint(TextPatternRangeEndpoint endpoint) const noexcept;
bool SetEndpoint(TextPatternRangeEndpoint endpoint, const COORD val);
bool SetEndpoint(TextPatternRangeEndpoint endpoint, const COORD val) noexcept;
const bool IsDegenerate() const noexcept;

// ITextRangeProvider methods
Expand Down Expand Up @@ -159,14 +159,15 @@ namespace Microsoft::Console::Types

virtual const COORD _getScreenFontSize() const;
const unsigned int _getViewportHeight(const SMALL_RECT viewport) const noexcept;
const Viewport _getBufferSize() const noexcept;

void _getBoundingRect(_In_ const COORD startAnchor, _In_ const COORD endAnchor, _Inout_ std::vector<double>& coords) const;

void
_moveEndpointByUnitCharacter(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd = false);
_In_ const bool preventBufferEnd = false) noexcept;

void
_moveEndpointByUnitWord(_In_ const int moveCount,
Expand All @@ -178,13 +179,13 @@ namespace Microsoft::Console::Types
_moveEndpointByUnitLine(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd = false);
_In_ const bool preventBufferEnd = false) noexcept;

void
_moveEndpointByUnitDocument(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd = false);
_In_ const bool preventBufferEnd = false) noexcept;

#ifdef UNIT_TESTING
friend class ::UiaTextRangeTests;
Expand Down

0 comments on commit 681a0db

Please sign in to comment.