Commit Graph

6 Commits

Author SHA1 Message Date
Joshua Gross 6a3c866b6a LayoutAnimations: work on making index adjustment more robust to flattening/unflattening and many reorders
Summary:
My goal in this diff is to make LayoutAnimations more stable, and more resilient to challenging situations. Namely, LayoutAnimations already works fine with:

1) Reorders
2) Flattening/unflattening
3) Deletion and recreation of the same hierarchy
4) Updates conflicting with an existing animation

However, what if /all/ of those things are combined? Handling update conflicts with multiple ongoing animations, repeatedly flattening/unflattening the same layer of hierarchy and reordering both parents and children, etc .

This diff does not make LayoutAnimations perfect, but it does make LayoutAnimations much more resilient to situations it was not able to handle before.

My primary method of testing was to use two Playground examples: one just repeatedly queues up mutations (some animated, some not) that create, update, and delete in the same hierarchy layer. The second, more complex one, mutates between random view hierarchies that involve a lot of flattening and unflattening as well as reordering, over a depth of 5. It also exercises animations over TextInlineViews, which is more challenging.

LayoutAnimations works best with the new "Flattening Differ" for now, because the Flattening Differ produces a much smaller, nearly minimal set of instructions in cases of flattening-unflattening. I would like that to not be a hard requirement for using LayoutAnimations, but it's a good starting-point for now.

As part of this work, I also developed a lot of debugging and logging mechanisms that are handy for detecting inconsistencies and debugging crashes. Some are included in this diff behind `#define` statements that are disabled by default, and the rest will be published separately and likely cannot be landed permanently, as they're more invasive changes that are only helpful in debugging.

# Followups:

- Automate testing: write a suite of C++ tests that mutates between random diffs and guarantees that all mutations in a StubViewTree are sensible
- Construct a set of minimal repros that catalogues all remaining crashes and inconsistency issues (these seem to be extremely marginal cases and are very hard to repro - so I think it's fine to run this in prod for now, but I will follow-up as soon as I'm back to catalogue and fix all remaining issues)
- This diff focuses on *not crashing*, but it is still possible to construct a sequence of complex mutations that results in (for example) views having some opacity between 0 and 1 if animations are interrupted repeatedly. Although this is easy enough to prevent in product code - the types of scenarios I'm running in tests are very unlikely to ever happen in production - it would be nice to be *sure* that LayoutAnimations will always converge to a sensible View hierarchy with up-to-date props.
- In general, the index adjustment logic is complicated. I don't know if there's a great way around it, so I need to at least catalogue and test all edge-cases as mentioned above.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D23382975

fbshipit-source-id: f379d9aa2a4b9c33fa2ba8fa07870c9e31fad5e7
2020-08-27 19:37:06 -07:00
David Vacca 48e9f0a1c0 Extend 'animations' module to compile in OSS
Summary:
This diff extends the 'animations' module to compile in OSS

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D22963702

fbshipit-source-id: e432e2f98b47a4b0456fd5e3d0f5263631782b29
2020-08-07 11:12:02 -07:00
Joshua Gross 28ff63f669 Optional verbose logging for LayoutAnimations
Summary:
I've used this while debugging LayoutAnimations a few times. It compiles out to nothing when the define is not set.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22962113

fbshipit-source-id: 88b7bb1c20a07a7d804e5c81d31cf871d58bee92
2020-08-05 17:44:06 -07:00
Joshua Gross 1b575e8d1f Simplify and correct LayoutAnimations index adjustment
Summary:
Consolidated a few places where index adjustment was happening with nearly identical code. There was also one remaining case where index adjustment should be happening, but was not previously.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22962115

fbshipit-source-id: 732c026b5a3c60bb0eadb8d49826ffd6367f7f62
2020-08-05 17:44:05 -07:00
Joshua Gross e3302eeeab LayoutAnimations: call onSuccess, onFailure callbacks
Summary:
Hook up onSuccess and onFailure callbacks to LayoutAnimations.

Note that in non-Fabric RN, onSuccess is not known to work in Android. I could not find any uses of onFailure and it's not documented, so for now, it is only called if the setup of the animation fails.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D22889352

fbshipit-source-id: 4306debb350388dd2b7a2cbfe295eb99723988e2
2020-08-02 16:37:03 -07:00
David Vacca 3093010ea5 move fabric to ReactCommon/react/renderer
Summary:
This diff moves fabric C++ code from ReactCommon/fabric to ReactCommon/react/renderer
As part of this diff I also refactored components, codegen and callsites on CatalystApp, FB4A and venice

Script: P137350694

changelog: [internal] internal refactor

Reviewed By: fkgozali

Differential Revision: D22852139

fbshipit-source-id: f85310ba858b6afd81abfd9cbe6d70b28eca7415
2020-07-31 13:34:29 -07:00