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

Conversation

@blasten
Copy link

@blasten blasten commented Sep 9, 2020

Description

As it turns out, the Android external view embedder had infrastructure built to work around the fact that the rasterizer submits a frame that should be resubmitted. We can simplify the current implementation by just skipping these frames.

This also fixes flutter/flutter#65404

Related Issues

flutter/flutter#65404

Tests

I added the following tests: Unit tests

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM % nits

@blasten blasten changed the title Don't submit a frame if the render status is kResubmit Add a new raster status kSkipAndRetry frame Sep 11, 2020
kSuccess,
// Frame needs to be resubmitted for rasterization. This is
// currently only called when thread configuration change occurs.
// Frame is submitted twice. This is currently only used when
Copy link
Contributor

Choose a reason for hiding this comment

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

What does frame with the same layer tree mean and how is it different from the same frame being submitted twice? It might be worth clarifying the difference here.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@blasten blasten added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 11, 2020
@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 11, 2020
@flutter flutter deleted a comment from fluttergithubbot Sep 11, 2020
@blasten blasten added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 11, 2020
@blasten blasten merged commit 85a7afa into flutter:master Sep 11, 2020
@blasten blasten deleted the cleanup_rasterizer branch September 11, 2020 23:29
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 12, 2020
pcsosinski pushed a commit to pcsosinski/engine that referenced this pull request Sep 15, 2020
pcsosinski pushed a commit that referenced this pull request Sep 15, 2020
* Update 1.22 engine to use Dart 2.10.0-110.3.beta

* Add a new raster status `kSkipAndRetry` frame (#21059)

* update license file for Dart 2.10.0-110.3.beta

* Account for current open image in FlutterImageView (#21191)

Co-authored-by: Emmanuel Garcia <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-android platform-ios waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External view embedder shouldn't submit frame if threads aren't merged on iOS

5 participants