Multi-view Rasterizer #45512

dkwingsmt · 2023-09-06T20:02:07Z

This PR refactors Rasterizer so that it's more suitable for multi-view.

Design doc: flutter.dev/go/multi-view-pipeline-and-rasterizer

With this change, Rasterizer::DrawToSurfaces has a structure that can handle drawing to multiple views, although the lack of some functionality still blocks it, mostly related to ExternalViewEmbedder and RasterCache.

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.

…aster-status-rescope

shell/common/rasterizer.cc

dnfield

No major concerns, mostly nits

flow/compositor_context.h

flow/frame_timings.h

shell/common/animator.cc

flow/layers/layer_tree.h

shell/common/rasterizer.cc

dnfield · 2023-09-25T22:04:19Z

shell/common/rasterizer.cc

+  FramePipeline::Consumer consumer = [&draw_result,
+                                      this](std::unique_ptr<FrameItem> item) {
+    draw_result = DoDraw(std::move(item->frame_timings_recorder),
+                         std::move(item->layer_tree_tasks));


Something should probably clear out the tasks after this right?

Otherwise we have a list of tasks with nullptr...

The tasks are moved all the way into DrawToSurfacesUnsafe, where some are discarded, some moved into resubmitted_tasks, and some stored in last_successful_tasks_. Since the vector is moved, there shouldn't be a list of nullptrs.

Oh, I think I see now, this is the last spot they're used.

shell/common/rasterizer.cc

dnfield · 2023-09-25T22:06:57Z

shell/common/rasterizer.cc

+                                     device_pixel_ratio);
+    }
+  }
+  // TODO(dkwingsmt): Pass in all raster caches


File a bug.

What's not clear to me is whether we'd actually want multiple raster caches or just one raster cache per layer tree. There are also ongoing explorations to see if we can just remove the raster cache entirely as we migrate to Impeller.

(@jonahwilliams fyi)

I don't think its possible for the rasterizer to know if it is safe to share any of this information. i.e. if these are from different windows with a different GlContext, or the different windows with the same GlContext, et cetera. The backends are not designed to handle this.

shell/common/rasterizer.cc

…r-real

dkwingsmt · 2023-09-26T23:10:43Z

@loic-sharma @dnfield I should have addressed all review comments.

Here are the non-trivial changes since the last review:

Several methods are renamed to reflect their multi-view nature, such as DrawLastLayerTree is renamed to DrawLastLayerTrees.
Rasterizer's view-specific data are grouped into a ViewRecord struct.
As per requested, the list of LayerTreeTask is now a vector of unique_ptrs.

Can you take another look?

flow/layers/layer_tree.h

dnfield

LGTM

flutter/engine@48973d7...d203e34 2023-09-29 [email protected] Multi-view Rasterizer (flutter/engine#45512) 2023-09-29 [email protected] Roll Dart SDK from 955a3a964621 to 0931bb8a43c8 (1 revision) (flutter/engine#46423) 2023-09-29 [email protected] [Impeller] Redo simplify invert colors. (flutter/engine#46416) 2023-09-29 [email protected] Use `dart:_wasm` constructs to avoid dependence on `WebAssembly.Function` (flutter/engine#46388) 2023-09-29 [email protected] Roll Skia from 2d4045f55fd5 to a063eaeaf1e0 (6 revisions) (flutter/engine#46420) 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…5776) flutter/engine@48973d7...d203e34 2023-09-29 [email protected] Multi-view Rasterizer (flutter/engine#45512) 2023-09-29 [email protected] Roll Dart SDK from 955a3a964621 to 0931bb8a43c8 (1 revision) (flutter/engine#46423) 2023-09-29 [email protected] [Impeller] Redo simplify invert colors. (flutter/engine#46416) 2023-09-29 [email protected] Use `dart:_wasm` constructs to avoid dependence on `WebAssembly.Function` (flutter/engine#46388) 2023-09-29 [email protected] Roll Skia from 2d4045f55fd5 to a063eaeaf1e0 (6 revisions) (flutter/engine#46420) 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This PR refactors `Rasterizer` so that it's more suitable for multi-view. Design doc: [flutter.dev/go/multi-view-pipeline-and-rasterizer](http://flutter.dev/go/multi-view-pipeline-and-rasterizer) With this change, `Rasterizer::DrawToSurfaces` has a structure that can handle drawing to multiple views, although the lack of some functionality still blocks it, mostly related to `ExternalViewEmbedder` and `RasterCache`. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

dkwingsmt added 30 commits August 15, 2023 13:08

Split FrameStatus of CompositorContext

e084a92

Extract DrawSurfaceStatus

f25f09a

Simplify DrawStatus

3977e16

DoDrawStatus, and rename back to RasterStatus

0b9483d

Combine retry

1138ada

Merge remote-tracking branch 'origin/main' into raster-status-rescope

cbaa530

Split status, found one

6826818

Add doc

6d64314

Split gpu unavailable

07dfb2f

Remove DrawSurfaceStatus discarded

4375281

Merge remote-tracking branch 'origin/main' into raster-status-rescope

c5d40d0

Merge remote-tracking branch 'origin/main' into raster-status-rescope

990a9da

Move the discard callback to surface

1b980ad

Merge remote-tracking branch 'origin/main' into raster-status-rescope

ac9ee88

Move up discarding

4618ad7

Better success handling

7d1706a

Merge remote-tracking branch 'origin/main' into raster-status-rescope

17f269e

Revert the discard movement

209ebee

Merge remote-tracking branch 'origin/main' into raster-status-rescope

fda0e7d

Merge remote-tracking branch 'dkwingsmt/raster-status-rescope' into r…

7b0f468

…aster-status-rescope

Revert some changes

9b3c12f

Revert more changes

69ea367

Rename to status

f2cd6e3

lint

d0d66ef

Merge remote-tracking branch 'origin/main' into mv-raster-real

94aac45

Convert to LayerTreeTask

3d94b8e

Merge remote-tracking branch 'origin/main' into raster-status-rescope

791bc60

Fix retry conversion

9445595

Merge branch 'raster-status-rescope' into mv-raster-real

06c284d

Multiview DoDraw and DrawToSurface

174f6d7

loic-sharma reviewed Sep 25, 2023

View reviewed changes

shell/common/rasterizer.cc Outdated Show resolved Hide resolved

loic-sharma reviewed Sep 25, 2023

View reviewed changes

shell/common/rasterizer.cc Outdated Show resolved Hide resolved

loic-sharma reviewed Sep 25, 2023

View reviewed changes

shell/common/rasterizer.cc Show resolved Hide resolved

dnfield reviewed Sep 25, 2023

View reviewed changes

dkwingsmt added 5 commits September 26, 2023 12:51

Change to vector unique_ptr

797a88c

Merge remote-tracking branch 'dkwingsmt/mv-raster-real' into mv-raste…

9a837be

…r-real

Doc fix

f752d0a

Merge remote-tracking branch 'origin/main' into mv-raster-real

44b24c5

Merge to ViewRecord

69550b3

dkwingsmt mentioned this pull request Sep 26, 2023

Support multi-view rendering flutter/flutter#135530

Closed

5 tasks

dkwingsmt added 2 commits September 26, 2023 16:10

Add issue links

0eb5694

Merge remote-tracking branch 'origin/main' into mv-raster-real

9b8c4cf

loic-sharma reviewed Sep 26, 2023

View reviewed changes

flow/layers/layer_tree.h Outdated Show resolved Hide resolved

dkwingsmt requested a review from dnfield September 28, 2023 17:25

dnfield reviewed Sep 29, 2023

View reviewed changes

flow/layers/layer_tree.h Outdated Show resolved Hide resolved

dkwingsmt added 2 commits September 29, 2023 12:57

Fix comment

0540cad

Merge remote-tracking branch 'origin/main' into mv-raster-real

af7ae28

dkwingsmt requested a review from dnfield September 29, 2023 20:26

Remove unnecessary include

6b9c423

dnfield approved these changes Sep 29, 2023

View reviewed changes

dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 29, 2023

auto-submit bot merged commit d203e34 into flutter:main Sep 29, 2023

engine-flutter-autoroll mentioned this pull request Sep 29, 2023

Roll Flutter Engine from 48973d7f9beb to d203e349781c (5 revisions) flutter/flutter#135776

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 29, 2023

d203e34 Multi-view Rasterizer (flutter/engine#45512)

1359a4a

dkwingsmt deleted the mv-raster-real branch September 29, 2023 23:05

Multi-view Rasterizer #45512

Multi-view Rasterizer #45512

Uh oh!

Conversation

dkwingsmt commented Sep 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnfield Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

dkwingsmt Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

dnfield Sep 29, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dnfield Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

dnfield Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkwingsmt commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dnfield left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dkwingsmt commented Sep 6, 2023 •

edited

Loading

dkwingsmt commented Sep 26, 2023 •

edited

Loading