Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] Include Adorners in matrix calculations in VisualExtensions #15484

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BAndysc
Copy link
Contributor

@BAndysc BAndysc commented Apr 23, 2024

What does the pull request do?

This PR changes VisualExtensions::TransformToVisual to support adorners.

What is the current behavior?

Currently VisualExtensions::TransformToVisual doesn't work correctly when one of the controls is inside an Adorner Layer. Adorned element is not on a path to root from adorner element. So it is not included in calculations. Adorners are rendered correctly, because the renderer has special handling for adorners. Hence TransformToVisual needs some special handling too.

What is the updated/expected behavior with this PR?

VisualExtensions::TransformToVisual should correctly take into account adorners.

How was the solution implemented (if it's not obvious)?

Unfortunately, adorners are defined in Avalonia.Controls while VisualExtensions is in Avalonia.Base, so VisualExtensions doesn't know what adorners are. Therefore, this PR adds AdornerLayerBase with AdornedElementProperty and an empty IAdornerLayer to Avalonia.Base so that this information is available. This is far from perfect, but I don't know how to do it in a cleaner way.

Besides, the obvious downside of the solution is additional computation complexity in VisualExtensions::TransformToVisual. Each time this method is called, an adorned element needs to be found via calling GetValue(AdornedElementProperty) on each element on path to root.

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #15480

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0047604-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added the bug label Apr 25, 2024
@maxkatz6
Copy link
Member

As per @kekekeks, whole situation with "adorner transformation processed on composition thread" should be changed to be done on UI thread. So, I am not sure how we should process with this PR (and previous already merged).

@BAndysc
Copy link
Contributor Author

BAndysc commented Apr 25, 2024

I understand. However, until it is properly fixed, I would appreciate if this PR could remain open, as I use adorners a lot in my app (and it worked fine in Avalonia 0.10), thus for now I can use nightly builds produced by this PR.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0047725-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@timunie
Copy link
Contributor

timunie commented Apr 25, 2024

@BAndysc we can leave it open, but I'd like to mark it as draft and won't merged for now, just in case

@timunie timunie marked this pull request as draft April 25, 2024 11:15
@timunie timunie changed the title Include Adorners in matrix calculations in VisualExtensions [DO NOT MERGE] Include Adorners in matrix calculations in VisualExtensions Apr 25, 2024
@BAndysc BAndysc force-pushed the adorners_transform_visual_fix branch from 25bd806 to e54d25f Compare May 7, 2024 09:42
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0048166-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text selection works incorrectly for TextBoxes in adorner layer
4 participants