[Animation] Add granular frame forcing to animations #173862
Conversation
github-actions
bot
added
the
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces forceFramesEnabled to SchedulerBinding, allowing apps to continue rendering when hidden or paused. The implementation is clear, well-documented, and includes a comprehensive set of tests. The logic for enabling/disabling frames and scheduling a new frame when needed appears correct.
I've found a small improvement opportunity in the new tests to ensure better test isolation by using the corresponding change that resets the _rootElement. Additionally, the change to WidgetsBinding.resetInternalState is not mentioned in the PR description, which would be good to add for clarity.
Overall, this is a great addition that addresses a valid use case.
tguerin
force-pushed
the
allow_rendering_when_hidden
branch
5 times, most recently
from
01bfbc5 to
2dc92da
Compare
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
tguerin
force-pushed
the
allow_rendering_when_hidden
branch
4 times, most recently
from
7e77bb3 to
a81175b
Compare
tguerin
force-pushed
the
allow_rendering_when_hidden
branch
from
a81175b to
174e9ef
Compare
|
We should make sure we are very certain that we want to support this use case before we add something like this due to the ability to shoot yourself in the foot here. @loic-sharma do you know if this is a valid concern for multiwindow/desktop apps? |
I don't know, however, I suspect that there's a better fix for #133533. @chinmaygarde Would you have any thoughts on this change for this use case:
|
|
The fix was not originally made for this specific issue, but to address a problem we’re facing in our app. We are building a Desktop game, and when the window is completely covered, frame rendering stops, which prevents our animations from running. To work around this, we currently have to intercept the hidden state and replace it with inactive, which is really hacky. The key requirement for us is to keep animations running even when the window is not visible. I’m open to different approaches to achieve this, as long as we can meet that requirement. |
|
@tguerin Can you explain why you'd like the animation to keep running at background? What is broken by missing these animation frames? It would be great if you can open a new issue for this so we can discuss it and track it more clearly. |
|
Here is the issue #174356 |
|
Does this comment in the linked issue provide what is needed here? #174356 (comment) |
|
I will rework the PR to include the suggested approach. |
tguerin
force-pushed
the
allow_rendering_when_hidden
branch
from
174e9ef to
0a731ca
Compare
tguerin
changed the title
tguerin
changed the title
tguerin
force-pushed
the
allow_rendering_when_hidden
branch
3 times, most recently
from
244f1af to
639b7ff
Compare
This change adds opt-in, subtree-scoped frame forcing for Flutter animations as described in flutter#174356 (comment) Fixes flutter#174356 This change could help fix flutter#133533. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
This change adds opt-in, subtree-scoped frame forcing for Flutter animations as described in flutter#174356 (comment) Fixes flutter#174356 This change could help fix flutter#133533. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
This rolls the latest DevTools in customer tests, and rolls the latest customer tests in Flutter. This unblocks: flutter#173862. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This change adds opt-in, subtree-scoped frame forcing for Flutter animations as described in flutter#174356 (comment) Fixes flutter#174356 This change could help fix flutter#133533. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
5 participants
This change adds opt-in, subtree-scoped frame forcing for Flutter animations as described in #174356 (comment)
Fixes #174356
This change could help fix #133533.
Pre-launch Checklist
///).