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

Reland 2: Multiview Pipeline #47239

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Oct 23, 2023

The last attempt #47234 was reverted because there was another test merged in the meantime that violates the rule added in this PR.

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 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.

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 23, 2023
@auto-submit auto-submit bot merged commit 0ef5caa into flutter:main Oct 23, 2023
@dkwingsmt dkwingsmt deleted the reland2-mv-pipeline branch October 23, 2023 23:47
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 24, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 24, 2023
…137114)

flutter/engine@dd6a653...0ef5caa

2023-10-23 dkwingsmt@users.noreply.github.com Reland 2: Multiview Pipeline  (flutter/engine#47239)

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 jimgraham@google.com,rmistry@google.com,zra@google.com 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 added a commit to dkwingsmt/engine that referenced this pull request Dec 19, 2023
auto-submit bot pushed a commit that referenced this pull request Dec 19, 2023
This reverts commit 0ef5caa.

Internal performance test shows that this PR negatively affects startup time. I'll reland this PR in parts.

b/307872797

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
auto-submit bot pushed a commit that referenced this pull request Dec 20, 2023
This is one of a series of changes to reland #47239.

This PR changes `Animator` so that if `Render` is not called after a `BeginFrame`, this call is ignored.

Note that this is slightly different from #47239. Instead of saying that we should ultimately change this skip to an assertion, this PR aims to keep the skip as the final shape. This is because a while ago we (with @goderbauer and @loic-sharma) decided that `PlatformDispatcher` should contain as little logic as possible to allow testing, and instead serve as a minimal native function binding, which means that we should eventually move the code that validates calling convention to the engine. 

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
dkwingsmt added a commit to dkwingsmt/engine that referenced this pull request Jan 18, 2024
dkwingsmt added a commit to dkwingsmt/engine that referenced this pull request Jan 22, 2024
@dkwingsmt dkwingsmt mentioned this pull request Jan 23, 2024
8 tasks
dkwingsmt added a commit to dkwingsmt/engine that referenced this pull request Jan 24, 2024
dkwingsmt added a commit to dkwingsmt/engine that referenced this pull request Feb 7, 2024
dkwingsmt added a commit that referenced this pull request Feb 23, 2024
This is the 3rd attempt to land multiview pipeline, following
#47239.

The pipeline now properly implements the required logic for
`scheduleWarmUpFrame` to work in a multi-view setup, following the
preparation in flutter/flutter#143290 and
#50570.

## 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 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].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
dkwingsmt pushed a commit that referenced this pull request Feb 23, 2024
Reverts #49950

Initiated by: dkwingsmt

Reason for reverting: Head redness 
```
java.lang.RuntimeException: Timeout waiting for firstFrameLatch to signal
	at dev.flutter.scenarios.ExternalTextureFlutterActivity.waitUntilFlutterRendered(ExternalTextureFlutterActivity.java:98)
	at dev.flutter.scenariosui.ScreenshotUtil.capture(ScreenshotUtil.java:122)
```

Original PR Author: dkwingsmt

Reviewed By: {loic-sharma}

This change reverts the following previous change:
Original Description:
This is the 3rd attempt to land multiview pipeline, following
#47239.

The pipeline now properly implements the required logic for
`scheduleWarmUpFrame` to work in a multi-view setup, following the
preparation in flutter/flutter#143290 and
#50570.

## 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 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].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
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