Skip to content

Commit

Permalink
Improve wide glyph support in UIA (#4946)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
- Added better wide glyph support for UIA. We used to move one _cell_ at a time, so wide glyphs would be read twice.
- Converted a few things to use til::point since I'm already here.
- fixed telemetry for UIA

## PR Checklist
* [x] Closes #1354

## Detailed Description of the Pull Request / Additional comments
The text buffer has a concept of word boundaries, so it makes sense to have a concept of glyph boundaries too.

_start and _end in UiaTextRange are now til::point

## Validation Steps Performed
Verified using Narrator
  • Loading branch information
carlos-zamora authored and DHowett committed Apr 14, 2020
1 parent ad80cff commit c97f336
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 46 deletions.
88 changes: 88 additions & 0 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1260,6 +1260,94 @@ bool TextBuffer::MoveToPreviousWord(COORD& pos, std::wstring_view wordDelimiters
return true;
}

// Method Description:
// - Update pos to be the beginning of the current glyph/character. This is used for accessibility
// Arguments:
// - pos - a COORD on the word you are currently on
// Return Value:
// - pos - The COORD for the first cell of the current glyph (inclusive)
const til::point TextBuffer::GetGlyphStart(const til::point pos) const
{
COORD resultPos = pos;

const auto bufferSize = GetSize();
if (resultPos != bufferSize.EndExclusive() && GetCellDataAt(resultPos)->DbcsAttr().IsTrailing())
{
bufferSize.DecrementInBounds(resultPos, true);
}

return resultPos;
}

// Method Description:
// - Update pos to be the end of the current glyph/character. This is used for accessibility
// Arguments:
// - pos - a COORD on the word you are currently on
// Return Value:
// - pos - The COORD for the last cell of the current glyph (exclusive)
const til::point TextBuffer::GetGlyphEnd(const til::point pos) const
{
COORD resultPos = pos;

const auto bufferSize = GetSize();
if (resultPos != bufferSize.EndExclusive() && GetCellDataAt(resultPos)->DbcsAttr().IsLeading())
{
bufferSize.IncrementInBounds(resultPos, true);
}

// increment one more time to become exclusive
bufferSize.IncrementInBounds(resultPos, true);
return resultPos;
}

// Method Description:
// - Update pos to be the beginning of the next glyph/character. This is used for accessibility
// Arguments:
// - pos - a COORD on the word you are currently on
// - allowBottomExclusive - allow the nonexistent end-of-buffer cell to be encountered
// Return Value:
// - true, if successfully updated pos. False, if we are unable to move (usually due to a buffer boundary)
// - pos - The COORD for the first cell of the current glyph (inclusive)
bool TextBuffer::MoveToNextGlyph(til::point& pos, bool allowBottomExclusive) const
{
COORD resultPos = pos;

// try to move. If we can't, we're done.
const auto bufferSize = GetSize();
const bool success = bufferSize.IncrementInBounds(resultPos, allowBottomExclusive);
if (resultPos != bufferSize.EndExclusive() && GetCellDataAt(resultPos)->DbcsAttr().IsTrailing())
{
bufferSize.IncrementInBounds(resultPos, allowBottomExclusive);
}

pos = resultPos;
return success;
}

// Method Description:
// - Update pos to be the beginning of the previous glyph/character. This is used for accessibility
// Arguments:
// - pos - a COORD on the word you are currently on
// - allowBottomExclusive - allow the nonexistent end-of-buffer cell to be encountered
// Return Value:
// - true, if successfully updated pos. False, if we are unable to move (usually due to a buffer boundary)
// - pos - The COORD for the first cell of the previous glyph (inclusive)
bool TextBuffer::MoveToPreviousGlyph(til::point& pos, bool allowBottomExclusive) const
{
COORD resultPos = pos;

// try to move. If we can't, we're done.
const auto bufferSize = GetSize();
const bool success = bufferSize.DecrementInBounds(resultPos, allowBottomExclusive);
if (resultPos != bufferSize.EndExclusive() && GetCellDataAt(resultPos)->DbcsAttr().IsLeading())
{
bufferSize.DecrementInBounds(resultPos, allowBottomExclusive);
}

pos = resultPos;
return success;
}

// Method Description:
// - Determines the line-by-line rectangles based on two COORDs
// - expands the rectangles to support wide glyphs
Expand Down
5 changes: 5 additions & 0 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ class TextBuffer final
bool MoveToNextWord(COORD& pos, const std::wstring_view wordDelimiters, COORD lastCharPos) const;
bool MoveToPreviousWord(COORD& pos, const std::wstring_view wordDelimiters) const;

const til::point GetGlyphStart(const til::point pos) const;
const til::point GetGlyphEnd(const til::point pos) const;
bool MoveToNextGlyph(til::point& pos, bool allowBottomExclusive = false) const;
bool MoveToPreviousGlyph(til::point& pos, bool allowBottomExclusive = false) const;

const std::vector<SMALL_RECT> GetTextRects(COORD start, COORD end, bool blockSelection = false) const;

class TextAndColor
Expand Down
54 changes: 54 additions & 0 deletions src/host/ut_host/TextBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class TextBufferTests

void WriteLinesToBuffer(const std::vector<std::wstring>& text, TextBuffer& buffer);
TEST_METHOD(GetWordBoundaries);
TEST_METHOD(GetGlyphBoundaries);

TEST_METHOD(GetTextRects);
TEST_METHOD(GetText);
Expand Down Expand Up @@ -2153,6 +2154,59 @@ void TextBufferTests::GetWordBoundaries()
}
}

void TextBufferTests::GetGlyphBoundaries()
{
struct ExpectedResult
{
std::wstring name;
til::point start;
til::point wideGlyphEnd;
til::point normalEnd;
};

// clang-format off
const std::vector<ExpectedResult> expected = {
{ L"Buffer Start", { 0, 0 }, { 2, 0 }, { 1, 0 } },
{ L"Line Start", { 0, 1 }, { 2, 1 }, { 1, 1 } },
{ L"General Case", { 1, 1 }, { 3, 1 }, { 2, 1 } },
{ L"Line End", { 9, 1 }, { 0, 2 }, { 0, 2 } },
{ L"Buffer End", { 9, 9 }, { 0, 10 }, { 0, 10 } },
};
// clang-format on

BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:wideGlyph", L"{false, true}")
END_TEST_METHOD_PROPERTIES();

bool wideGlyph;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"wideGlyph", wideGlyph), L"Get wide glyph variant");

COORD bufferSize{ 10, 10 };
UINT cursorSize = 12;
TextAttribute attr{ 0x7f };
auto _buffer = std::make_unique<TextBuffer>(bufferSize, attr, cursorSize, _renderTarget);

// This is the burrito emoji: 🌯
// It's encoded in UTF-16, as needed by the buffer.
const auto burrito = L"\xD83C\xDF2F";
const wchar_t* const output = wideGlyph ? burrito : L"X";

const OutputCellIterator iter{ output };

for (const auto& test : expected)
{
Log::Comment(test.name.c_str());
auto target = test.start;
_buffer->Write(iter, target);

auto start = _buffer->GetGlyphStart(target);
auto end = _buffer->GetGlyphEnd(target);

VERIFY_ARE_EQUAL(test.start, start);
VERIFY_ARE_EQUAL(wideGlyph ? test.wideGlyphEnd : test.normalEnd, end);
}
}

void TextBufferTests::GetTextRects()
{
// GetTextRects() is used to...
Expand Down
3 changes: 2 additions & 1 deletion src/types/ScreenInfoUiaProviderBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_
return hr;
}

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

LONG currentIndex = 0;
hr = SafeArrayPutElement(*ppRetVal, &currentIndex, range.Detach());
if (FAILED(hr))
Expand All @@ -265,7 +267,6 @@ IFACEMETHODIMP ScreenInfoUiaProviderBase::GetSelection(_Outptr_result_maybenull_
return hr;
}

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

Expand Down
22 changes: 11 additions & 11 deletions src/types/UiaTextRangeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ IFACEMETHODIMP UiaTextRangeBase::ExpandToEnclosingUnit(_In_ TextUnit unit) noexc

if (unit == TextUnit_Character)
{
_end = _start;
bufferSize.IncrementInBounds(_end, true);
_start = buffer.GetGlyphStart(_start);
_end = buffer.GetGlyphEnd(_start);
}
else if (unit <= TextUnit_Word)
{
Expand Down Expand Up @@ -526,7 +526,7 @@ try
const auto bufferSize = buffer.GetSize();

// convert _end to be inclusive
auto inclusiveEnd{ _end };
auto inclusiveEnd = _end;
bufferSize.DecrementInBounds(inclusiveEnd, true);

const auto textRects = buffer.GetTextRects(_start, inclusiveEnd, _blockRange);
Expand Down Expand Up @@ -687,9 +687,9 @@ try
}
else
{
auto temp = _end;
_pData->GetTextBuffer().GetSize().DecrementInBounds(temp);
_pData->SelectNewRegion(_start, temp);
auto inclusiveEnd = _end;
_pData->GetTextBuffer().GetSize().DecrementInBounds(inclusiveEnd);
_pData->SelectNewRegion(_start, inclusiveEnd);
}

UiaTracing::TextRange::Select(*this);
Expand Down Expand Up @@ -897,7 +897,7 @@ void UiaTextRangeBase::_getBoundingRect(const til::rectangle textRect, _Inout_ s
void UiaTextRangeBase::_moveEndpointByUnitCharacter(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
_Out_ gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd) noexcept
_In_ const bool preventBufferEnd)
{
*pAmountMoved = 0;

Expand All @@ -908,23 +908,23 @@ void UiaTextRangeBase::_moveEndpointByUnitCharacter(_In_ const int moveCount,

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

bool success = true;
auto target = GetEndpoint(endpoint);
til::point target = GetEndpoint(endpoint);
while (std::abs(*pAmountMoved) < std::abs(moveCount) && success)
{
switch (moveDirection)
{
case MovementDirection::Forward:
success = bufferSize.IncrementInBounds(target, allowBottomExclusive);
success = buffer.MoveToNextGlyph(target, allowBottomExclusive);
if (success)
{
(*pAmountMoved)++;
}
break;
case MovementDirection::Backward:
success = bufferSize.DecrementInBounds(target, allowBottomExclusive);
success = buffer.MoveToPreviousGlyph(target, allowBottomExclusive);
if (success)
{
(*pAmountMoved)--;
Expand Down
2 changes: 1 addition & 1 deletion src/types/UiaTextRangeBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ namespace Microsoft::Console::Types
_moveEndpointByUnitCharacter(_In_ const int moveCount,
_In_ const TextPatternRangeEndpoint endpoint,
gsl::not_null<int*> const pAmountMoved,
_In_ const bool preventBufferEnd = false) noexcept;
_In_ const bool preventBufferEnd = false);

void
_moveEndpointByUnitWord(_In_ const int moveCount,
Expand Down
Loading

0 comments on commit c97f336

Please sign in to comment.