Skip to content

Commit

Permalink
[Rasterizer] Make resubmit information temporary (flutter#42001)
Browse files Browse the repository at this point in the history
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].

<!-- Links -->
[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
  • Loading branch information
dkwingsmt authored May 25, 2023
1 parent 99f2653 commit 9f36408
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 31 deletions.
57 changes: 33 additions & 24 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,19 @@ RasterStatus Rasterizer::Draw(
.GetRasterTaskRunner()
->RunsTasksOnCurrentThread());

RasterStatus raster_status = RasterStatus::kFailed;
DoDrawResult draw_result;
LayerTreePipeline::Consumer consumer =
[&](std::unique_ptr<LayerTreeItem> item) {
[this, &draw_result,
&discard_callback](std::unique_ptr<LayerTreeItem> item) {
std::unique_ptr<LayerTree> layer_tree = std::move(item->layer_tree);
std::unique_ptr<FrameTimingsRecorder> 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);
}
};

Expand All @@ -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<LayerTreeItem>(
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;
}

Expand Down Expand Up @@ -255,7 +253,7 @@ RasterStatus Rasterizer::Draw(
break;
}

return raster_status;
return draw_result.raster_status;
}

bool Rasterizer::ShouldResubmitFrame(const RasterStatus& raster_status) {
Expand Down Expand Up @@ -376,7 +374,7 @@ fml::Milliseconds Rasterizer::GetFrameBudget() const {
return delegate_.GetFrameBudget();
};

RasterStatus Rasterizer::DoDraw(
Rasterizer::DoDrawResult Rasterizer::DoDraw(
std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
std::unique_ptr<flutter::LayerTree> layer_tree,
float device_pixel_ratio) {
Expand All @@ -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();
Expand All @@ -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<LayerTreeItem>(
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() &&
Expand Down Expand Up @@ -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(
Expand Down
21 changes: 14 additions & 7 deletions shell/common/rasterizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<LayerTreeItem> resubmitted_layer_tree_item;
};

// |SnapshotDelegate|
std::unique_ptr<GpuImageResult> MakeSkiaGpuImage(
sk_sp<DisplayList> display_list,
Expand Down Expand Up @@ -554,7 +567,7 @@ class Rasterizer final : public SnapshotDelegate,
GrDirectContext* surface_context,
bool compressed);

RasterStatus DoDraw(
DoDrawResult DoDraw(
std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
std::unique_ptr<flutter::LayerTree> layer_tree,
float device_pixel_ratio);
Expand All @@ -581,12 +594,6 @@ class Rasterizer final : public SnapshotDelegate,
// This is the last successfully rasterized layer tree.
std::unique_ptr<flutter::LayerTree> 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<flutter::LayerTree> resubmitted_layer_tree_;
std::unique_ptr<FrameTimingsRecorder> resubmitted_recorder_;
float resubmitted_pixel_ratio_;
fml::closure next_frame_callback_;
bool user_override_resource_cache_bytes_;
std::optional<size_t> max_cache_bytes_;
Expand Down

0 comments on commit 9f36408

Please sign in to comment.