GetCurrentPoint breaking-change fix

[maxkatz6]

What is the current behavior?

Currently if point cannot be transformed to the relativeTo visual, it returns default point (0,0). Which can cause wrong behavior in the apps. Typical use case, when visual root and relative to element might not have common visual, which might happen if event was raised in the popup/tooltip, but handled in the parent control. Exactly what happens in #8796 when tooltip is enabled.

What is the updated/expected behavior with this PR?

GetPosition and GetCurrentPoint returns null if it can't be transformed. Developer then can decide what to do with this result. Also added public Properties prop to simplify access.

Breaking changes

Yes. Pretty annoying one. @grokys @danwalmsley @kekekeks

Fixed issues

Fixes #8796

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0023819-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[robloo]

This does sound tricky. No idea how to implement this, just high-level comments:

1. I dont think the point position should ever return null if it is over an actual Avalonia control. Only if it was outside the window or something would that potentially make sense. However, I would really avoid nullability if possible here.
2. On fast movement like this, the popup should dissapear BEFORE the pointer even fires for the parent. This means the root issue should never happen because the popup is hidden. That said, of course there might be a fade-out animation. In that situation a mechanism is somehow needed to know the popup is closing and shouldn't be used for hit testing.
3. There should be a way for the pointer even to get below the popup and return a valid position if the popup doesn't handle it.

[grokys]

One option we discussed is returning Point(double.NaN, double.NaN) in this case, this has the advantage of not changing the API but the disadvantage of returning a value one may not expect.

[robloo]

I still think this case should be correctly handled before we get to the point of invalid position coordinates. That would preempt this whole issue. The root cause is not invalid pointer position as much as its pointer position can't properly be calculated in this case. The pointer position should be able to transform to the spectrum to begin with.

I agree that NaN, infinity or even the default is better than null. However, Null and even NaN would cause too many exceptions in the future. I don't think you want those types of crashes to occur in the framework. You will get a lot of bug reports in edge cases I would think (current behavior in the above bug report is better than a crash).

Actually, if we can't calculate things the default should be screen or window position. Would have to read up on UWP a bit more to see what they do as well.

[maxkatz6]

@robloo I am not a huge fan of returning null either, but there should be a better alternative otherwise. Returning NaN might work, but returning a default value is definitely wrong as (0,0) position is absolutely valid position. And it would cause issues https://github.com/AvaloniaUI/Avalonia/issues/8796 once again.

The pointer position should be able to transform to the spectrum to begin with

Not always though. That's the problem. Intuitive solution would be to rely on screen coordinates. I.e. if we see that point event was raised on another root (popup/tooltip), we should transform it to the screen coordinates first and then transform back to the target window. That's how I solved this issue in previous PR but had to revert. The big problem we can't rely on screen coordinates in xplat framework so easily. Wayland on linux for instance does not provide this information and this issue would still exist there.

Also, another use case is when transformation matrix has zero determinant. From my understanding this normally happens when "relativeTo" element is not visible because of specific render transforms. https://github.com/AvaloniaUI/Avalonia/blob/887c57ea18e724e444eb1831295195cf71838e85/src/Avalonia.Base/VisualExtensions.cs#L58

Actually, if we can't calculate things the default should be screen or window position

Which window position? Target window (from relativeTo element, i.e. could be a tooltip) or source window (parent's window).

Would have to read up on UWP a bit more to see what they do as well.

I would expect UWP to transform position from the screen space, what we can't do because of Wayland.

However, Null and even NaN would cause too many exceptions in the future. I don't think you want those types of crashes to occur in the framework

NaN - yes, it would cause such reports. Null - unlikely, that's a struct, compiler would force developers to do null-checks. As for now we are getting reports of unexpected (0,0) pointer position, because Avalonia assumes that (0,0) can be treated as an invalid position. Which is wrong.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0023848-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[robloo]

@maxkatz6 Thanks for the details. Wayland is the missing piece of information I didn't know. If Wayland doesn't support screen coordinates I understand how fixing this in a "more correct" way isn't possible... This is disappointing but I understand your solution now.

Hopefully in the future Wayland will get this support and it can be done differently in another release accepting breaking changes.

That said, it may still be possible to quickly hide the tooltip in the case of the bug report. If the tooltip is never there to intercept the pointer event it would be handled correctly. That said, there are cases you want the pointer to go over the tooltip on purpose so there would likely have to be an additional property to control this.