AvaloniaUI
Fixed positioning of popups attached to adorners
What does the pull request do?
This PR fixes positioning of popups attached to adorners, this is an useful scenario, i.e. ComboxBox attached as an adorner.
What is the current behavior?
Popups attached to adorners are displayed in wrong position - they ignore adorned control position.
What is the updated/expected behavior with this PR?
Popups attached to adorners will be correctly positioned.
How was the solution implemented (if it's not obvious)?
This worked in Avalonia 0.10, because adorners used adorned control's TransformedBounds property to set adorner RenderTransform and later in the popup placement logic, RenderTransform was used to position the popup. However, with Avalonia 11 this property was removed. Using a new method GetTransformedBounds() does help, but because there is no way to observe for changes in this property, this solution is not feasible in Avalonia 11. Therefore we can not update adorner control RenderTransform and a different approach is required. If we can't fix adorner RenderTransform, we can make popup positioning aware of adorners and that's how I implemented it. When a popup wants to be attached to an adorner, it calculates the position from top level to the adorned element and then from the adorner layer to the target.
Checklist
- [x] Added unit tests (if possible)?
- [ ] Added XML documentation to any related classes?
- [ ] Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation
Breaking changes
n/a
Obsoletions / Deprecations
n/a
Fixed issues
Fixes #9845
You can test this PR using the following package version. 11.1.999-cibuild0044293-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0044307-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0044428-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0044430-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0044460-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0045529-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0045719-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.1.999-cibuild0045870-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]
@maxkatz6 @grokys I hate to be this guy (who pings without a reason), but as the master branch is switching to 11.2 soon, is there any chance for this PR to make its way to 11.1? I'd be grateful as it's a breaking bug for me. Cheers
@BAndysc I am not sure if I can review this PR properly, as I am not much familiar with specifics of adorners. From my understanding, TransformToVisual should have already include adorner transform. And if it doesn't - it probably should be fixed instead. But I would leave decision to @grokys.
Thank you for your response.
TransformToVisual works by traversing the visual parents of the control and taking each control Bounds, additionaly if a control has RenderTransform set, then it takes it into account.
The reason why TransformToVisual does not include the transform of an adorned element is as follows:
- Adorned elements are not on the path from the visual root to the adorner, since that's how adorners work - adorners in the visual tree belong to a separate control
- Having that in mind, given the current implementation of
TransformToVisual, the only way to ensure it returned the correct value would be to set an appropriateRenderTransformto the adorner.
And in fact, that's how this used to work in Avalonia 0.10. In that version, adorners had a RenderTransform set based on the position of the adorned element, allowing TransformToVisual to operate as expected. The RenderTransform would update whenever the TransformedBounds of the adorned element changed:
https://github.com/AvaloniaUI/Avalonia/blob/7739bd9ca3a20a8cb2b10c4ee2ca31337b227f4e/src/Avalonia.Controls/Primitives/AdornerLayer.cs#L183-L187
https://github.com/AvaloniaUI/Avalonia/blob/7739bd9ca3a20a8cb2b10c4ee2ca31337b227f4e/src/Avalonia.Controls/Primitives/AdornerLayer.cs#L89-L99
This has changed with the compositor changes. TransformedBounds property is now gone. It can be calculated, but it can't be observed
https://github.com/AvaloniaUI/Avalonia/blob/2ecfbb978337ef7937821717c01421cdf60cd504/src/Avalonia.Controls/Primitives/AdornerLayer.cs#L321-L325
The info.bounds now only includes size, not position. Although TransformedBounds can still be calculated (with the correct postion), it's no longer observable, so RenderTransform wouldn't update with movements of the adorned element.
You might wonder: if adorners no longer have a RenderTransform, how are they rendered correctly? The answer is simple: the compositor is aware of the concept of adorners:
https://github.com/AvaloniaUI/Avalonia/blob/4dbb165a7bcb338ef3324690549fd3012d3f8a92/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositionVisual.cs#L146-L148
(I am not 100% sure those two lines are responsible for rendering the adorner in the correct place, but it doesn't really matter - the only important thing is that compositor knows adorners exist and knows how to handle them).
With all the info, I don't see how adorners could have RenderTransform set correctly. Additionally, it's challenging to make TransformToVisual aware of adorners since TransformToVisual is part of Avalonia.Base, which doesn't have information about adorners.
If RenderTransform can't be set and TransformToVisual can't be changed - the only way (that I see; only way without overhauling a lot of the code) to make popups attached to adorners work is by making the popup placement logic aware of adorners, just like compositor is.
Yeah, it's clear enough for me now. Now it feels like we should have a TransformToVisual/GetTransformToVisual that works on top of compositor tree snapshot, since render transform doesn't represent full reality anymore. But that's outside of this PR scope for sure.
You can test this PR using the following package version. 11.1.999-cibuild0045906-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]