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

Skip invalidation on propagation #25652

Merged
merged 4 commits into from
Nov 5, 2024
Merged

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Nov 4, 2024

Description

The main purpose of #23052 was to propagate the MeasureInvalidated event up the tree. The problem with doing this is that the Page and the Compatibility.Layout will fire InvalidateMeasure when that propagation gets to them.

This PR shift the needle back a little bit and only fires those paths if the invalidation is fired from a direct child. We still want the event to propagate up the tree for listeners to act on but we don't want anything beyond the immediate parent to invalidate

@PureWeen PureWeen requested a review from a team as a code owner November 4, 2024 01:00
@PureWeen PureWeen force-pushed the skip_invalidation_on_propagation branch from 56246f4 to 0afbdca Compare November 4, 2024 09:57
Copy link
Contributor

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand. there's a special case when depth <=1 and we only ++depth (and never decrement)

src/Controls/src/Core/LegacyLayouts/Layout.cs Outdated Show resolved Hide resolved
@PureWeen
Copy link
Member Author

PureWeen commented Nov 4, 2024

not sure I understand. there's a special case when depth <=1 and we only ++depth (and never decrement)

The depth is just there so each call knows how far down the tree the element that triggered the invalidation is and then it can make decisions about it

Each depth is a throwaway value that just starts over when a new invalidation path is called.

The motivation here is that if an element is 4 levels down from a scrollview we don't want that bubbling up to a forced invalidation of the platform element. It's just meant to be informative and then scroll view can decide if it wants to invalidate or not. Thats basically how it's worked in Maui up until 8.0.90 and then that regressed slightly and this pr fixes that regression

@PureWeen PureWeen added this to the .NET 9 SR1 milestone Nov 4, 2024
@PureWeen
Copy link
Member Author

PureWeen commented Nov 5, 2024

@PureWeen PureWeen merged commit 1412b38 into main Nov 5, 2024
100 of 108 checks passed
@PureWeen PureWeen deleted the skip_invalidation_on_propagation branch November 5, 2024 17:46
PureWeen added a commit that referenced this pull request Nov 5, 2024
* Skip Invalidation unless you're my immediate child

* - fix override on test

* Fix Legacy Layouts Invalidation Propagation

* - use invalidation args to propagate depth
# Conflicts:
#	src/Controls/src/Core/Page/Page.cs
#	src/Controls/src/Core/VisualElement/VisualElement.cs
PureWeen added a commit that referenced this pull request Nov 6, 2024
* Skip invalidation on propagation (#25652)

* Skip Invalidation unless you're my immediate child

* - fix override on test

* Fix Legacy Layouts Invalidation Propagation

* - use invalidation args to propagate depth
# Conflicts:
#	src/Controls/src/Core/Page/Page.cs
#	src/Controls/src/Core/VisualElement/VisualElement.cs

* - fix up UseLegacyMeasureInvalidatedBehaviorEnabled

* - fix missing NS

* - add test

* - add screen shots

* - snapshots

* - final snapshots
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants