[xStrom]

The old transform_changed was busted. It was internally documented as tracking transform and scroll_translation, but in practice it was also tracking origin, but then exposed in MutateCtx::transform_has_changed with the claim that it only tracks transform -- and then Xilem was using it with that assumption.

This PR cleans that up by removing MutateCtx::transform_has_changed and properly documenting transform_changed as tracking everything which affects the full transform that the compose pass calculates. Currently that is transform, scroll_translation, and origin.

What's more, the transform_changed flag isn't even merged up the tree, yet the compose pass checks it to decide whether to traverse into a branch. Thus, the compose pass was implicitly depending on unrelated code paths to set the needs_compose flag.

This PR sets the needs_compose flag explicitly, to ensure things remain working even when that flag isn't being set elsewhere.

The composing logic was duplicated in both the compose and accessibility passes. Yet it is critical for correct accessibility behavior for that to stay in sync with the compose results.

This PR introduces WidgetState::compose_local_transform as a helper method, which will calculate the correct local transform. This will help this logic stay in sync. This is especially relevant as the logic gets more fragile, i.e. with the pixel snapping of #1803.

Finally, this PR adds a new test which makes sure that scroll translation doesn't trigger a layout, is respected by hit testing, and that the regular and accessibility bounds are the same.

[PoignardAzur]

I'm not certain about this PR. It seems to add complexity (you now have to remember the difference between mark_compose_transform_changed and mark_transform_changed) to Masonry for the sake of papering over one of Xilem's idiosyncrasies.

Could we instead use something like ViewCtx::prop_has_changed

xilem/xilem_masonry/src/view/prop.rs

Line 61 in 19c3360

let prop_has_changed = ctx.prop_has_changed::<P>(element.id());

and remove MutateCtx::transform_changed entirely?

[xStrom]

Yes, it's significant complexity cost for a minor Xilem quirk. I created #1820 which moves the tracking to Xilem and would allow us to greatly reduce the complexity of Masonry's tracking.

[PoignardAzur]

Should we close this, then?

[xStrom]

No, this PR is still valid. It is now simplified and no longer splits the transform dirty tracking.

[PoignardAzur]

LGTM.

[PoignardAzur]

Seems like assert_all would give more info in case of an error.

[xStrom]

Perhaps, but it requires a new dependency, right? Doesn't seem important enough to add a whole new dep.

[PoignardAzur]

No, I mean the helper function we define in masonry_testing.