From 3b00b85da8f8ca38326ffe3ea2b8047a56355f97 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 25 May 2023 13:11:23 -0700 Subject: [PATCH] [Rasterizer] Make resubmit information temporary (flutter/engine#42001) This PR refactors `Rasterizer` so that the resubmit information is returned as a temporary struct instead of stored permanently as class member. This limits the lifetime of these values and reduces class members. Additionally, this benefits the multi-view project: if the rasterizer is going to store multiple surfaces, what variables should be stored in a map from view IDs? Should all surfaces have a to-be-submitted record? The answer is "no", because these information is only temporary. This PR should not need unit tests since it's only a refactor. ## 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]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [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 --- engine/src/flutter/shell/common/rasterizer.cc | 57 +++++++++++-------- engine/src/flutter/shell/common/rasterizer.h | 21 ++++--- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/engine/src/flutter/shell/common/rasterizer.cc b/engine/src/flutter/shell/common/rasterizer.cc index e5e77dd9b9..cc613dcf89 100644 --- a/engine/src/flutter/shell/common/rasterizer.cc +++ b/engine/src/flutter/shell/common/rasterizer.cc @@ -193,18 +193,19 @@ RasterStatus Rasterizer::Draw( .GetRasterTaskRunner() ->RunsTasksOnCurrentThread()); - RasterStatus raster_status = RasterStatus::kFailed; + DoDrawResult draw_result; LayerTreePipeline::Consumer consumer = - [&](std::unique_ptr item) { + [this, &draw_result, + &discard_callback](std::unique_ptr item) { std::unique_ptr layer_tree = std::move(item->layer_tree); std::unique_ptr frame_timings_recorder = std::move(item->frame_timings_recorder); float device_pixel_ratio = item->device_pixel_ratio; if (discard_callback(*layer_tree.get())) { - raster_status = RasterStatus::kDiscarded; + draw_result.raster_status = RasterStatus::kDiscarded; } else { - raster_status = DoDraw(std::move(frame_timings_recorder), - std::move(layer_tree), device_pixel_ratio); + draw_result = DoDraw(std::move(frame_timings_recorder), + std::move(layer_tree), device_pixel_ratio); } }; @@ -215,18 +216,15 @@ RasterStatus Rasterizer::Draw( // if the raster status is to resubmit the frame, we push the frame to the // front of the queue and also change the consume status to more available. - bool should_resubmit_frame = ShouldResubmitFrame(raster_status); + bool should_resubmit_frame = ShouldResubmitFrame(draw_result.raster_status); if (should_resubmit_frame) { - auto resubmitted_layer_tree_item = std::make_unique( - std::move(resubmitted_layer_tree_), std::move(resubmitted_recorder_), - resubmitted_pixel_ratio_); auto front_continuation = pipeline->ProduceIfEmpty(); - PipelineProduceResult result = - front_continuation.Complete(std::move(resubmitted_layer_tree_item)); - if (result.success) { + PipelineProduceResult pipeline_result = front_continuation.Complete( + std::move(draw_result.resubmitted_layer_tree_item)); + if (pipeline_result.success) { consume_result = PipelineConsumeResult::MoreAvailable; } - } else if (raster_status == RasterStatus::kEnqueuePipeline) { + } else if (draw_result.raster_status == RasterStatus::kEnqueuePipeline) { consume_result = PipelineConsumeResult::MoreAvailable; } @@ -255,7 +253,7 @@ RasterStatus Rasterizer::Draw( break; } - return raster_status; + return draw_result.raster_status; } bool Rasterizer::ShouldResubmitFrame(const RasterStatus& raster_status) { @@ -376,7 +374,7 @@ fml::Milliseconds Rasterizer::GetFrameBudget() const { return delegate_.GetFrameBudget(); }; -RasterStatus Rasterizer::DoDraw( +Rasterizer::DoDrawResult Rasterizer::DoDraw( std::unique_ptr frame_timings_recorder, std::unique_ptr layer_tree, float device_pixel_ratio) { @@ -387,7 +385,9 @@ RasterStatus Rasterizer::DoDraw( ->RunsTasksOnCurrentThread()); if (!layer_tree || !surface_) { - return RasterStatus::kFailed; + return DoDrawResult{ + .raster_status = RasterStatus::kFailed, + }; } PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); @@ -399,13 +399,18 @@ RasterStatus Rasterizer::DoDraw( last_layer_tree_ = std::move(layer_tree); last_device_pixel_ratio_ = device_pixel_ratio; } else if (ShouldResubmitFrame(raster_status)) { - resubmitted_pixel_ratio_ = device_pixel_ratio; - resubmitted_layer_tree_ = std::move(layer_tree); - resubmitted_recorder_ = frame_timings_recorder->CloneUntil( - FrameTimingsRecorder::State::kBuildEnd); - return raster_status; + return DoDrawResult{ + .raster_status = raster_status, + .resubmitted_layer_tree_item = std::make_unique( + std::move(layer_tree), + frame_timings_recorder->CloneUntil( + FrameTimingsRecorder::State::kBuildEnd), + device_pixel_ratio), + }; } else if (raster_status == RasterStatus::kDiscarded) { - return raster_status; + return DoDrawResult{ + .raster_status = raster_status, + }; } if (persistent_cache->IsDumpingSkp() && @@ -473,11 +478,15 @@ RasterStatus Rasterizer::DoDraw( if (raster_thread_merger_) { if (raster_thread_merger_->DecrementLease() == fml::RasterThreadStatus::kUnmergedNow) { - return RasterStatus::kEnqueuePipeline; + return DoDrawResult{ + .raster_status = RasterStatus::kEnqueuePipeline, + }; } } - return raster_status; + return DoDrawResult{ + .raster_status = raster_status, + }; } RasterStatus Rasterizer::DrawToSurface( diff --git a/engine/src/flutter/shell/common/rasterizer.h b/engine/src/flutter/shell/common/rasterizer.h index b17242342c..93747c3f77 100644 --- a/engine/src/flutter/shell/common/rasterizer.h +++ b/engine/src/flutter/shell/common/rasterizer.h @@ -500,6 +500,19 @@ class Rasterizer final : public SnapshotDelegate, void DisableThreadMergerIfNeeded(); private: + // The result of `DoDraw`. + // + // Normally `DoDraw` returns simply a raster status. However, sometimes we + // need to attempt to rasterize the layer tree again. This happens when + // layer_tree has not successfully rasterized due to changes in the thread + // configuration, in which case the resubmitted task will be inserted to the + // front of the pipeline. + struct DoDrawResult { + RasterStatus raster_status = RasterStatus::kFailed; + + std::unique_ptr resubmitted_layer_tree_item; + }; + // |SnapshotDelegate| std::unique_ptr MakeSkiaGpuImage( sk_sp display_list, @@ -554,7 +567,7 @@ class Rasterizer final : public SnapshotDelegate, GrDirectContext* surface_context, bool compressed); - RasterStatus DoDraw( + DoDrawResult DoDraw( std::unique_ptr frame_timings_recorder, std::unique_ptr layer_tree, float device_pixel_ratio); @@ -581,12 +594,6 @@ class Rasterizer final : public SnapshotDelegate, // This is the last successfully rasterized layer tree. std::unique_ptr last_layer_tree_; float last_device_pixel_ratio_; - // Set when we need attempt to rasterize the layer tree again. This layer_tree - // has not successfully rasterized. This can happen due to the change in the - // thread configuration. This will be inserted to the front of the pipeline. - std::unique_ptr resubmitted_layer_tree_; - std::unique_ptr resubmitted_recorder_; - float resubmitted_pixel_ratio_; fml::closure next_frame_callback_; bool user_override_resource_cache_bytes_; std::optional max_cache_bytes_;