automotive-design-compose icon indicating copy to clipboard operation
automotive-design-compose copied to clipboard

Fix #1052: rewrite squoosh animation tree building to not mutate trees.

Open iamralpht opened this issue 1 year ago • 1 comments

While attempting to fix a bug in squoosh's animation support where the root frames of components were not considered for animation, I kept running into layout assertions, which ultimately were because the animation implementation performs destructive mutations on the base layout tree AND on the "transition layout tree".

This change means that all SquooshResolvedNode trees are now immutable after construction (except for layout and style values which can change after the tree was built).

Finally, because the source trees are immutable, we can run layout on the root and target root trees whenever layout needs to change. This means animations are uninterrupted by relayouts, and we can relayout as many times as we like without generating new trees.

iamralpht avatar Apr 28 '24 03:04 iamralpht

Snapshot diff report vs base branch: main Last updated: Mon Jun 17 12:28:28 PDT 2024, Sha: 1afc9e3ac99ea1102ca6527e3e90f8f953fa3843

File name Image
VariantAnimation-Mid
Point_compare.png

github-actions[bot] avatar Apr 28 '24 03:04 github-actions[bot]

I noticed (a few months ago, when last I worked on this!) a regression in the "Same Name" animation test. I've made that test into a unit test as well as an interactive one, and fixed the bug. I think this is ready to merge, but I haven't gone through @rylin8 's most recent comments.

iamralpht avatar Jun 17 '24 19:06 iamralpht

The midpoint diff should be a bit more visible when it gets rendered -- I changed the midpoint time we sample to get something that was more "middle" than before, so that test is expected to be quite different (but I manually inspected the new expectation and believe it's correct).

iamralpht avatar Jun 17 '24 19:06 iamralpht