Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

Multiview pipeline #44473

Merged
merged 106 commits into from
Oct 20, 2023
Merged

Multiview pipeline #44473

merged 106 commits into from
Oct 20, 2023

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Aug 7, 2023

This PR makes Animator able to handle multiple views, and updates unit tests accordingly.

Before:
image

After:
image

Now Animator::Render must be called during Animator::BeginFrame, which is split into BeginFrame and EndFrame. This requirement is made possible by #45555. The reason to split is to allow ShellTest::PumpOneFrame to insert a render from C++ code.

ShellTest::PumpOneFrame is also refactored to allow pumping a frame without any views.

A few unit tests are tweaked to resolve racing condition.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@dnfield
Copy link
Contributor

dnfield commented Oct 16, 2023

This LGTM but I'll look at it again once it's up to date.

@chinmaygarde
Copy link
Member

xref the re-land #47095

@dkwingsmt dkwingsmt requested a review from dnfield October 19, 2023 23:09
@dkwingsmt
Copy link
Contributor Author

The code that this PR relies on has been relanded in #47062, which I've checked has no performance regressions. @dnfield Can you take a final look?

@dkwingsmt dkwingsmt added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Oct 20, 2023
@auto-submit auto-submit bot merged commit d9f2453 into flutter:main Oct 20, 2023
@dkwingsmt dkwingsmt added the revert Label used to revert changes in a closed and merged pull request. label Oct 20, 2023
@auto-submit auto-submit bot mentioned this pull request Oct 20, 2023
8 tasks
auto-submit bot pushed a commit that referenced this pull request Oct 20, 2023
@auto-submit auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Oct 20, 2023
auto-submit bot added a commit that referenced this pull request Oct 20, 2023
Reverts #44473
Initiated by: dkwingsmt
This change reverts the following previous change:
Original Description:
This PR makes `Animator` able to handle multiple views, and updates unit tests accordingly.

Before:
<img width="543" alt="image" src="https://github.com/flutter/engine/assets/1596656/f7d0e0e4-cc85-4a6e-b516-1896ac3c1b35">

After:
<img width="614" alt="image" src="https://github.com/flutter/engine/assets/1596656/68106301-66ef-4cd1-aeaf-d9c6127ccec2">

Now `Animator::Render` must be called during `Animator::BeginFrame`, which is split into `BeginFrame` and `EndFrame`. This requirement is made possible by #45555. The reason to split is to allow `ShellTest::PumpOneFrame` to insert a render from C++ code.

`ShellTest::PumpOneFrame` is also refactored to allow pumping a frame without any views.

A few unit tests are tweaked to resolve racing condition.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 20, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 20, 2023
…136987)

flutter/engine@b27e1b3...4a65910

2023-10-20 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Multiview pipeline" (flutter/engine#47174)
2023-10-20 [email protected] Multiview pipeline (flutter/engine#44473)
2023-10-20 [email protected] Roll Skia from 9ffd5ef9a9ed to ca69b04f7dd2 (1 revision) (flutter/engine#47171)
2023-10-20 [email protected] Lower the severity of a log message (flutter/engine#47172)
2023-10-20 [email protected] [web] Remove workaround for safely removing slots on Safari (flutter/engine#47169)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@dkwingsmt dkwingsmt mentioned this pull request Oct 23, 2023
8 tasks
auto-submit bot pushed a commit that referenced this pull request Oct 23, 2023
This PR relands #44473.

The previous PR was immediately reverted after merging because we found that the PR could cause illegal renders to be skipped on debug builds but crash the app on release builds. This PR makes the `Animator::Render` skip illegal renders as well. This should not be the final shape of this feature, and thus a TODO is added.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
This PR makes `Animator` able to handle multiple views, and updates unit tests accordingly.

Before:
<img width="543" alt="image" src="https://github.com/flutter/engine/assets/1596656/f7d0e0e4-cc85-4a6e-b516-1896ac3c1b35">

After:
<img width="614" alt="image" src="https://github.com/flutter/engine/assets/1596656/68106301-66ef-4cd1-aeaf-d9c6127ccec2">

Now `Animator::Render` must be called during `Animator::BeginFrame`, which is split into `BeginFrame` and `EndFrame`. This requirement is made possible by #45555. The reason to split is to allow `ShellTest::PumpOneFrame` to insert a render from C++ code.

`ShellTest::PumpOneFrame` is also refactored to allow pumping a frame without any views.

A few unit tests are tweaked to resolve racing condition.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
Reverts #44473
Initiated by: dkwingsmt
This change reverts the following previous change:
Original Description:
This PR makes `Animator` able to handle multiple views, and updates unit tests accordingly.

Before:
<img width="543" alt="image" src="https://github.com/flutter/engine/assets/1596656/f7d0e0e4-cc85-4a6e-b516-1896ac3c1b35">

After:
<img width="614" alt="image" src="https://github.com/flutter/engine/assets/1596656/68106301-66ef-4cd1-aeaf-d9c6127ccec2">

Now `Animator::Render` must be called during `Animator::BeginFrame`, which is split into `BeginFrame` and `EndFrame`. This requirement is made possible by #45555. The reason to split is to allow `ShellTest::PumpOneFrame` to insert a render from C++ code.

`ShellTest::PumpOneFrame` is also refactored to allow pumping a frame without any views.

A few unit tests are tweaked to resolve racing condition.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
This PR relands #44473.

The previous PR was immediately reverted after merging because we found that the PR could cause illegal renders to be skipped on debug builds but crash the app on release builds. This PR makes the `Animator::Render` skip illegal renders as well. This should not be the final shape of this feature, and thus a TODO is added.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller platform-fuchsia
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants