From 476d3c3732c5ce72d689dfa2342da9e7ee51a223 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 1 Jun 2023 21:50:47 -0700 Subject: [PATCH] Revert "[Rasterizer] Make resubmit information temporary" (#42455) Reverts flutter/engine#42001 due to being the likely culprit to flakiness https://github.com/flutter/flutter/issues/127936 . --- shell/common/rasterizer.cc | 57 ++++++++++++++++---------------------- shell/common/rasterizer.h | 21 +++++--------- 2 files changed, 31 insertions(+), 47 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 40aea5b7481d0..963c5005571c5 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -197,19 +197,18 @@ RasterStatus Rasterizer::Draw( .GetRasterTaskRunner() ->RunsTasksOnCurrentThread()); - DoDrawResult draw_result; + RasterStatus raster_status = RasterStatus::kFailed; LayerTreePipeline::Consumer consumer = - [this, &draw_result, - &discard_callback](std::unique_ptr item) { + [&](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())) { - draw_result.raster_status = RasterStatus::kDiscarded; + raster_status = RasterStatus::kDiscarded; } else { - draw_result = DoDraw(std::move(frame_timings_recorder), - std::move(layer_tree), device_pixel_ratio); + raster_status = DoDraw(std::move(frame_timings_recorder), + std::move(layer_tree), device_pixel_ratio); } }; @@ -220,15 +219,18 @@ 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(draw_result.raster_status); + bool should_resubmit_frame = ShouldResubmitFrame(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 pipeline_result = front_continuation.Complete( - std::move(draw_result.resubmitted_layer_tree_item)); - if (pipeline_result.success) { + PipelineProduceResult result = + front_continuation.Complete(std::move(resubmitted_layer_tree_item)); + if (result.success) { consume_result = PipelineConsumeResult::MoreAvailable; } - } else if (draw_result.raster_status == RasterStatus::kEnqueuePipeline) { + } else if (raster_status == RasterStatus::kEnqueuePipeline) { consume_result = PipelineConsumeResult::MoreAvailable; } @@ -257,7 +259,7 @@ RasterStatus Rasterizer::Draw( break; } - return draw_result.raster_status; + return raster_status; } bool Rasterizer::ShouldResubmitFrame(const RasterStatus& raster_status) { @@ -378,7 +380,7 @@ fml::Milliseconds Rasterizer::GetFrameBudget() const { return delegate_.GetFrameBudget(); }; -Rasterizer::DoDrawResult Rasterizer::DoDraw( +RasterStatus Rasterizer::DoDraw( std::unique_ptr frame_timings_recorder, std::unique_ptr layer_tree, float device_pixel_ratio) { @@ -389,9 +391,7 @@ Rasterizer::DoDrawResult Rasterizer::DoDraw( ->RunsTasksOnCurrentThread()); if (!layer_tree || !surface_) { - return DoDrawResult{ - .raster_status = RasterStatus::kFailed, - }; + return RasterStatus::kFailed; } PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); @@ -403,18 +403,13 @@ Rasterizer::DoDrawResult Rasterizer::DoDraw( last_layer_tree_ = std::move(layer_tree); last_device_pixel_ratio_ = device_pixel_ratio; } else if (ShouldResubmitFrame(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), - }; + 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; } else if (raster_status == RasterStatus::kDiscarded) { - return DoDrawResult{ - .raster_status = raster_status, - }; + return raster_status; } if (persistent_cache->IsDumpingSkp() && @@ -482,15 +477,11 @@ Rasterizer::DoDrawResult Rasterizer::DoDraw( if (raster_thread_merger_) { if (raster_thread_merger_->DecrementLease() == fml::RasterThreadStatus::kUnmergedNow) { - return DoDrawResult{ - .raster_status = RasterStatus::kEnqueuePipeline, - }; + return RasterStatus::kEnqueuePipeline; } } - return DoDrawResult{ - .raster_status = raster_status, - }; + return raster_status; } RasterStatus Rasterizer::DrawToSurface( diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 93747c3f77505..b17242342c084 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -500,19 +500,6 @@ 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, @@ -567,7 +554,7 @@ class Rasterizer final : public SnapshotDelegate, GrDirectContext* surface_context, bool compressed); - DoDrawResult DoDraw( + RasterStatus DoDraw( std::unique_ptr frame_timings_recorder, std::unique_ptr layer_tree, float device_pixel_ratio); @@ -594,6 +581,12 @@ 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_;