Fix UIA RangeFromPoint API (#17695)

## Summary of the Pull Request
Fixes the `RangeFromPoint` API such that we're now properly locking when
we attempt to retrieve the viewport data. This also corrects the
conversion from `UiaPoint` (screen position) to buffer coordinates
(buffer cell).

Closes #17579 

## Detailed Description of the Pull Request / Additional comments
- `UiaTextRangeBase::Initialize(UiaPoint)`:
- reordered logic to clamp to client area first, then begin conversion
to buffer coordinates
   - properly lock when retrieving the viewport data
- updated `_TranslatePointToScreen` and `_TranslatePointFromScreen` to
use `&` instead of `*`
   - we weren't properly updating the parameter before
- `TermControlUiaTextRange::_TranslatePointFromScreen()`
- `includeOffsets` was basically copied over from
`_TranslatePointToScreen`. The math itself was straight up wrong since
we had to do it backwards.

## Validation Steps Performed
 Moved WT to top-left of monitor, then used inspect.exe to call
`RangeFromPoint` API when mouse cursor is on top-left buffer cell (also
meticulously stepped through the two functions ensuring everything was
correct).
This commit is contained in:
Carlos Zamora 2024-08-20 15:58:05 -07:00 committed by GitHub
parent 249fe2aca1
commit 7b39d24913
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 37 additions and 45 deletions

View File

@ -69,14 +69,14 @@ IFACEMETHODIMP UiaTextRange::Clone(_Outptr_result_maybenull_ ITextRangeProvider*
return S_OK; return S_OK;
} }
void UiaTextRange::_TranslatePointToScreen(til::point* clientPoint) const void UiaTextRange::_TranslatePointToScreen(til::point& clientPoint) const
{ {
ClientToScreen(_getWindowHandle(), clientPoint->as_win32_point()); ClientToScreen(_getWindowHandle(), clientPoint.as_win32_point());
} }
void UiaTextRange::_TranslatePointFromScreen(til::point* screenPoint) const void UiaTextRange::_TranslatePointFromScreen(til::point& screenPoint) const
{ {
ScreenToClient(_getWindowHandle(), screenPoint->as_win32_point()); ScreenToClient(_getWindowHandle(), screenPoint.as_win32_point());
} }
HWND UiaTextRange::_getWindowHandle() const HWND UiaTextRange::_getWindowHandle() const

View File

@ -56,8 +56,8 @@ namespace Microsoft::Console::Interactivity::Win32
IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override; IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override;
protected: protected:
void _TranslatePointToScreen(til::point* clientPoint) const override; void _TranslatePointToScreen(til::point& clientPoint) const override;
void _TranslatePointFromScreen(til::point* screenPoint) const override; void _TranslatePointFromScreen(til::point& screenPoint) const override;
private: private:
HWND _getWindowHandle() const; HWND _getWindowHandle() const;

View File

@ -73,7 +73,7 @@ IFACEMETHODIMP TermControlUiaTextRange::Clone(_Outptr_result_maybenull_ ITextRan
// (0,0) is the top-left of the app window // (0,0) is the top-left of the app window
// Return Value: // Return Value:
// - <none> // - <none>
void TermControlUiaTextRange::_TranslatePointToScreen(til::point* clientPoint) const void TermControlUiaTextRange::_TranslatePointToScreen(til::point& clientPoint) const
{ {
const gsl::not_null<TermControlUiaProvider*> provider = static_cast<TermControlUiaProvider*>(_pProvider); const gsl::not_null<TermControlUiaProvider*> provider = static_cast<TermControlUiaProvider*>(_pProvider);
@ -96,8 +96,8 @@ void TermControlUiaTextRange::_TranslatePointToScreen(til::point* clientPoint) c
// Get scale factor for display // Get scale factor for display
const auto scaleFactor = provider->GetScaleFactor(); const auto scaleFactor = provider->GetScaleFactor();
clientPoint->x = includeOffsets(clientPoint->x, boundingRect.left, padding.left, scaleFactor); clientPoint.x = includeOffsets(clientPoint.x, boundingRect.left, padding.left, scaleFactor);
clientPoint->y = includeOffsets(clientPoint->y, boundingRect.top, padding.top, scaleFactor); clientPoint.y = includeOffsets(clientPoint.y, boundingRect.top, padding.top, scaleFactor);
} }
// Method Description: // Method Description:
@ -107,15 +107,13 @@ void TermControlUiaTextRange::_TranslatePointToScreen(til::point* clientPoint) c
// (0,0) is the top-left of the screen // (0,0) is the top-left of the screen
// Return Value: // Return Value:
// - <none> // - <none>
void TermControlUiaTextRange::_TranslatePointFromScreen(til::point* screenPoint) const void TermControlUiaTextRange::_TranslatePointFromScreen(til::point& screenPoint) const
{ {
const gsl::not_null<TermControlUiaProvider*> provider = static_cast<TermControlUiaProvider*>(_pProvider); const gsl::not_null<TermControlUiaProvider*> provider = static_cast<TermControlUiaProvider*>(_pProvider);
const auto includeOffsets = [](long screenPos, double termControlPos, double padding, double scaleFactor) { const auto includeOffsets = [](long screenPos, double termControlPos, double padding, double scaleFactor) {
auto result = base::ClampedNumeric<double>(padding); auto result = base::ClampedNumeric<til::CoordType>(screenPos);
// only the padding is in DIPs now result -= padding / scaleFactor;
result /= scaleFactor;
result -= screenPos;
result -= termControlPos; result -= termControlPos;
return result; return result;
}; };
@ -130,8 +128,8 @@ void TermControlUiaTextRange::_TranslatePointFromScreen(til::point* screenPoint)
// Get scale factor for display // Get scale factor for display
const auto scaleFactor = provider->GetScaleFactor(); const auto scaleFactor = provider->GetScaleFactor();
screenPoint->x = includeOffsets(screenPoint->x, boundingRect.left, padding.left, scaleFactor); screenPoint.x = includeOffsets(screenPoint.x, boundingRect.left, padding.left, scaleFactor);
screenPoint->y = includeOffsets(screenPoint->y, boundingRect.top, padding.top, scaleFactor); screenPoint.y = includeOffsets(screenPoint.y, boundingRect.top, padding.top, scaleFactor);
} }
til::size TermControlUiaTextRange::_getScreenFontSize() const noexcept til::size TermControlUiaTextRange::_getScreenFontSize() const noexcept

View File

@ -56,8 +56,8 @@ namespace Microsoft::Terminal
IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override; IFACEMETHODIMP Clone(_Outptr_result_maybenull_ ITextRangeProvider** ppRetVal) override;
protected: protected:
void _TranslatePointToScreen(til::point* clientPoint) const override; void _TranslatePointToScreen(til::point& clientPoint) const override;
void _TranslatePointFromScreen(til::point* screenPoint) const override; void _TranslatePointFromScreen(til::point& screenPoint) const override;
til::size _getScreenFontSize() const noexcept override; til::size _getScreenFontSize() const noexcept override;
}; };
} }

View File

@ -97,30 +97,24 @@ CATCH_RETURN();
void UiaTextRangeBase::Initialize(_In_ const UiaPoint point) void UiaTextRangeBase::Initialize(_In_ const UiaPoint point)
{ {
til::point clientPoint; til::point clientPoint{ static_cast<til::CoordType>(point.x), static_cast<til::CoordType>(point.y) };
clientPoint.x = static_cast<LONG>(point.x);
clientPoint.y = static_cast<LONG>(point.y);
// get row that point resides in
const auto windowRect = _getTerminalRect();
const auto viewport = _pData->GetViewport().ToInclusive();
til::CoordType row = 0;
if (clientPoint.y <= windowRect.top)
{
row = viewport.top;
}
else if (clientPoint.y >= windowRect.bottom)
{
row = viewport.bottom;
}
else
{
// change point coords to pixels relative to window
_TranslatePointFromScreen(&clientPoint);
const auto currentFontSize = _getScreenFontSize(); // clamp the point to be within the client area
row = clientPoint.y / currentFontSize.height + viewport.top; const auto windowRect = _getTerminalRect();
} clientPoint.x = std::clamp(clientPoint.x, windowRect.left, windowRect.right);
_start = { 0, row }; clientPoint.y = std::clamp(clientPoint.y, windowRect.top, windowRect.bottom);
// convert point to be relative to client area
_TranslatePointFromScreen(clientPoint);
// convert the point to screen buffer coordinates
_pData->LockConsole();
const auto currentFontSize = _getScreenFontSize();
const auto viewport = _pData->GetViewport().ToInclusive();
_pData->UnlockConsole();
_start = { clientPoint.x / currentFontSize.width + viewport.left,
clientPoint.y / currentFontSize.height + viewport.top };
_end = _start; _end = _start;
} }
@ -1381,8 +1375,8 @@ void UiaTextRangeBase::_getBoundingRect(const til::rect& textRect, _Inout_ std::
// convert the coords to be relative to the screen instead of // convert the coords to be relative to the screen instead of
// the client window // the client window
_TranslatePointToScreen(&topLeft); _TranslatePointToScreen(topLeft);
_TranslatePointToScreen(&bottomRight); _TranslatePointToScreen(bottomRight);
const long width = bottomRight.x - topLeft.x; const long width = bottomRight.x - topLeft.x;
const long height = bottomRight.y - topLeft.y; const long height = bottomRight.y - topLeft.y;

View File

@ -123,8 +123,8 @@ namespace Microsoft::Console::Types
std::wstring _wordDelimiters{}; std::wstring _wordDelimiters{};
::Search _searcher; ::Search _searcher;
virtual void _TranslatePointToScreen(til::point* clientPoint) const = 0; virtual void _TranslatePointToScreen(til::point& clientPoint) const = 0;
virtual void _TranslatePointFromScreen(til::point* screenPoint) const = 0; virtual void _TranslatePointFromScreen(til::point& screenPoint) const = 0;
void Initialize(_In_ const UiaPoint point); void Initialize(_In_ const UiaPoint point);