Skip to content

Commit

Permalink
Fix a deadlock and a bounding rects issue in UIA (#5385)
Browse files Browse the repository at this point in the history
The scroll locking rework that landed with the DxRenderer's partial
invalidation change introduced a deadlock. UIA locks the buffer for
reading before asking it to scroll (which now requires a write lock.)

Scrolling is probably _okay_ to have a little bit of torn state that
might arise from us unlocking the read lock early before triggering the
write lock down the line.

While investigating this, I also noticed that our bounding rects stopped
being viewport-relative (and were instead buffer-relative.) That
regressed with the switch to `GetTextRects` in #4991  

## PR Checklist
* [ ] Closes (issues noticed in investigating the DPI changes)
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Requires documentation to be updated
* [x] Core contributor badge

## Validation Steps Performed
Manual pass with inspect.exe
  • Loading branch information
DHowett committed Apr 17, 2020
1 parent f1fe4c6 commit bceeb45
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions src/types/UiaTextRangeBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_

// these viewport vars are converted to the buffer coordinate space
const auto viewport = bufferSize.ConvertToOrigin(_pData->GetViewport());
const auto viewportOrigin = viewport.Origin();
const til::point viewportOrigin = viewport.Origin();
const auto viewportEnd = viewport.EndExclusive();

// startAnchor: the earliest COORD we will get a bounding rect for
Expand Down Expand Up @@ -426,7 +426,9 @@ IFACEMETHODIMP UiaTextRangeBase::GetBoundingRectangles(_Outptr_result_maybenull_

for (const auto& rect : textRects)
{
_getBoundingRect(rect, coords);
til::rectangle r{ rect };
r -= viewportOrigin;
_getBoundingRect(r, coords);
}
}

Expand Down Expand Up @@ -759,6 +761,8 @@ try
FAIL_FAST_IF(!(newViewport.Bottom <= bottomRow));
FAIL_FAST_IF(!(_getViewportHeight(oldViewport) == _getViewportHeight(newViewport)));

Unlock.reset();

_ChangeViewport(newViewport);

UiaTracing::TextRange::ScrollIntoView(alignToTop, *this);
Expand Down

0 comments on commit bceeb45

Please sign in to comment.