Skip to content

Course correct props at SurfaceMountingManager.updateProps()#53589

Closed
zeyap wants to merge 2 commits into
facebook:mainfrom
zeyap:export-D81611823
Closed

Course correct props at SurfaceMountingManager.updateProps()#53589
zeyap wants to merge 2 commits into
facebook:mainfrom
zeyap:export-D81611823

Conversation

@zeyap
Copy link
Copy Markdown
Contributor

@zeyap zeyap commented Sep 3, 2025

Summary:

Changelog:

[Android] [Changed] - [c++ animated] Course correct props at SurfaceMountingManager.updateProps()

Sometimes a React update will try to commit to the same view that native animated modified before via direct manipulation, and after the update host view will use the prop value currently in Fabric. In AnimatedMountingOverrideDelegate there's logic to course correct at ShadowTree mount, but if this update is from JS thread, it takes some time to reach mounting layer, at the same time UI thread can still be doing more direct animation updates, and once the corrected change gets there it's already stale.

In this diff I added mechanism to keep track of direct manipulation props (or "synchronous mount props" to match the naming of java function synchronouslyUpdateView...) and use it to correct what reaches host view. SurfaceMountingManager.updateProps() is called by both regular mount and direct manipulation and it's always called on UI thread, so it could be a good candidate to synchronize these 2 scenarios

Differential Revision: D81611823

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 3, 2025
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@zeyap zeyap force-pushed the export-D81611823 branch 2 times, most recently from 0d9f11b to 985255b Compare September 4, 2025 13:52
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@zeyap zeyap force-pushed the export-D81611823 branch 2 times, most recently from 02f27a9 to 9db781f Compare September 4, 2025 14:51
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

1 similar comment
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

…k#53589)

Summary:
Pull Request resolved: facebook#53589

## Changelog:

[Android] [Changed] - [c++ animated] Course correct props at SurfaceMountingManager.updateProps()

Sometimes a React update will try to commit to the same view that native animated modified before via direct manipulation, and after the update host view will use the prop value currently in Fabric. In `AnimatedMountingOverrideDelegate` there's logic to course correct at ShadowTree mount, but if this update is from JS thread, it takes some time to reach mounting layer, at the same time UI thread can still be doing more direct animation updates, and once the corrected change gets there it's already stale.

In this diff I added mechanism to keep track of direct manipulation props (or "synchronous mount props" to match the naming of java function `synchronouslyUpdateView...`) and use it to correct what reaches host view. `SurfaceMountingManager.updateProps()` is called by both regular mount and direct manipulation and it's always called on UI thread, so it could be a good candidate to synchronize these 2 scenarios

Reviewed By: sammy-SC

Differential Revision: D81611823
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D81611823

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 5, 2025
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in dae2f60.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants