Animate filter without forcing a Fabric commit every frame - #58071
Open
dennytosp wants to merge 1 commit into
Open
Animate filter without forcing a Fabric commit every frame#58071dennytosp wants to merge 1 commit into
dennytosp wants to merge 1 commit into
Conversation
The C++ direct-manipulation allowlist is missing 'filter', which NativeAnimatedAllowlist.js lists as an animatable style. Anything absent from that set is classified as a layout update by StyleAnimatedNode, so filter animations are committed through the shadow tree each frame instead of taking the direct-manipulation path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
NativeAnimatedAllowlist.hcarries this instruction:It has drifted by one entry.
SUPPORTED_STYLESin the JS file listsfilterbetweenopacityandtransform; the C++ set goes straight from"opacity"to"transform". Every other member of the JS list's non-layout half is present, so this is a missed sync rather than a deliberate exclusion.The set is not advisory — it is what separates layout props from paint props:
and that verdict picks the transport:
So animating
filterruns a shadow-tree commit on every frame instead of taking the direct-manipulation path, for a property that cannot affect layout —filterlives inBaseViewProps(std::vector<FilterFunction> filter{}), not inYogaStylableProps, and nothing inreact/renderer/animated/treats it specially.Note the C++ set is deliberately not a mirror of the whole JS
SUPPORTED_STYLES: the entries the JS file adds underuseSharedAnimatedBackend()(width,height,margin,padding,flex,gap, …) are genuine layout props and must stay out so they keep going through Fabric. Only the non-layout half has to match, andfilterbelongs to it.Changelog:
[GENERAL] [FIXED] - Animating
filterno longer forces a Fabric commit on every frameTest Plan:
Added
directManipulationAllowlistCoversNonLayoutStylestoAnimatedNodeTests. It asserts the allowlist contains every non-layout style the JS file supports, and — so the test cannot be satisfied by simply widening the set — that the layout styles are still absent.The allowlist header is dependency-free, so the invariant can also be checked directly:
getDirectManipulationAllowlistappears in the C++ API snapshots, but only by signature — this changes the contents of the static set, not the declaration, soscripts/cxx-apiis unaffected.The gtest itself was neither executed locally nor built by the public CI.
react/renderer/animated/testsis excluded from the iOS build (React-Fabric.podspec:ss.exclude_files = "react/renderer/animated/tests") and is not in the Android CMake glob (react/renderer/animated/CMakeLists.txtglobs*.cpp drivers/*.cpp event_drivers/*.cpp internal/*.cpp nodes/*.cpp), so it builds only in the internal build reached at import time. What is verified here: the test body compiles clean against the real header under-std=c++20 -Wall -Wextra, and the standalone parity check above exercises the same assertions and fails without the one-line change.