Skip to content

Commit

Permalink
Discard wrong size layer tree instead of rendering it (flutter#21179)
Browse files Browse the repository at this point in the history
  • Loading branch information
knopp authored Sep 18, 2020
1 parent fa33c76 commit 5c9dddc
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 8 deletions.
4 changes: 3 additions & 1 deletion flow/compositor_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ enum class RasterStatus {
// only used when thread configuration change occurs.
kEnqueuePipeline,
// Failed to rasterize the frame.
kFailed
kFailed,
// Layer tree was discarded due to LayerTreeDiscardCallback
kDiscarded
};

class CompositorContext {
Expand Down
9 changes: 8 additions & 1 deletion shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ if (enable_unittests) {
]
}

config("shell_test_fixture_sources_config") {
defines = [
# Required for MSVC STL
"_ENABLE_ATOMIC_ALIGNMENT_FIX",
]
}

source_set("shell_test_fixture_sources") {
testonly = true

Expand Down Expand Up @@ -214,7 +221,7 @@ if (enable_unittests) {
"//third_party/skia",
]

public_configs = []
public_configs = [ ":shell_test_fixture_sources_config" ]

# SwiftShader only supports x86/x64_64
if (target_cpu == "x86" || target_cpu == "x64") {
Expand Down
9 changes: 7 additions & 2 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ void Rasterizer::DrawLastLayerTree() {
DrawToSurface(*last_layer_tree_);
}

void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
LayerTreeDiscardCallback discardCallback) {
TRACE_EVENT0("flutter", "GPURasterizer::Draw");
if (raster_thread_merger_ &&
!raster_thread_merger_->IsOnRasterizingThread()) {
Expand All @@ -166,7 +167,11 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
RasterStatus raster_status = RasterStatus::kFailed;
Pipeline<flutter::LayerTree>::Consumer consumer =
[&](std::unique_ptr<LayerTree> layer_tree) {
raster_status = DoDraw(std::move(layer_tree));
if (discardCallback(*layer_tree.get())) {
raster_status = RasterStatus::kDiscarded;
} else {
raster_status = DoDraw(std::move(layer_tree));
}
};

PipelineConsumeResult consume_result = pipeline->Consume(consumer);
Expand Down
9 changes: 8 additions & 1 deletion shell/common/rasterizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ class Rasterizer final : public SnapshotDelegate {
///
flutter::TextureRegistry* GetTextureRegistry();

using LayerTreeDiscardCallback = std::function<bool(flutter::LayerTree&)>;

//----------------------------------------------------------------------------
/// @brief Takes the next item from the layer tree pipeline and executes
/// the raster thread frame workload for that pipeline item to
Expand Down Expand Up @@ -232,8 +234,11 @@ class Rasterizer final : public SnapshotDelegate {
///
/// @param[in] pipeline The layer tree pipeline to take the next layer tree
/// to render from.
/// @param[in] discardCallback if specified and returns true, the layer tree
/// is discarded instead of being rendered
///
void Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline);
void Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
LayerTreeDiscardCallback discardCallback = NoDiscard);

//----------------------------------------------------------------------------
/// @brief The type of the screenshot to obtain of the previously
Expand Down Expand Up @@ -454,6 +459,8 @@ class Rasterizer final : public SnapshotDelegate {

void FireNextFrameCallbackIfPresent();

static bool NoDiscard(const flutter::LayerTree& layer_tree) { return false; }

FML_DISALLOW_COPY_AND_ASSIGN(Rasterizer);
};

Expand Down
18 changes: 15 additions & 3 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,12 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) {
engine->SetViewportMetrics(metrics);
}
});

{
std::scoped_lock<std::mutex> lock(resize_mutex_);
expected_frame_size_ =
SkISize::Make(metrics.physical_width, metrics.physical_height);
}
}

// |PlatformView::Delegate|
Expand Down Expand Up @@ -1021,13 +1027,19 @@ void Shell::OnAnimatorDraw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
}
}

auto discard_callback = [this](flutter::LayerTree& tree) {
std::scoped_lock<std::mutex> lock(resize_mutex_);
return !expected_frame_size_.isEmpty() &&
tree.frame_size() != expected_frame_size_;
};

task_runners_.GetRasterTaskRunner()->PostTask(
[&waiting_for_first_frame = waiting_for_first_frame_,
&waiting_for_first_frame_condition = waiting_for_first_frame_condition_,
rasterizer = rasterizer_->GetWeakPtr(),
pipeline = std::move(pipeline)]() {
rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline),
discard_callback = std::move(discard_callback)]() {
if (rasterizer) {
rasterizer->Draw(pipeline);
rasterizer->Draw(pipeline, std::move(discard_callback));

if (waiting_for_first_frame.load()) {
waiting_for_first_frame.store(false);
Expand Down
7 changes: 7 additions & 0 deletions shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,13 @@ class Shell final : public PlatformView::Delegate,
// and read from the raster thread.
std::atomic<float> display_refresh_rate_ = 0.0f;

// protects expected_frame_size_ which is set on platform thread and read on
// raster thread
std::mutex resize_mutex_;

// used to discard wrong size layer tree produced during interactive resizing
SkISize expected_frame_size_ = SkISize::MakeEmpty();

// How many frames have been timed since last report.
size_t UnreportedFramesCount() const;

Expand Down
10 changes: 10 additions & 0 deletions shell/common/shell_test_external_view_embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ int ShellTestExternalViewEmbedder::GetSubmittedFrameCount() {
return submitted_frame_count_;
}

SkISize ShellTestExternalViewEmbedder::GetLastSubmittedFrameSize() {
return last_submitted_frame_size_;
}

// |ExternalViewEmbedder|
void ShellTestExternalViewEmbedder::CancelFrame() {}

Expand Down Expand Up @@ -61,6 +65,12 @@ void ShellTestExternalViewEmbedder::SubmitFrame(
GrDirectContext* context,
std::unique_ptr<SurfaceFrame> frame) {
frame->Submit();
if (frame && frame->SkiaSurface()) {
last_submitted_frame_size_ = SkISize::Make(frame->SkiaSurface()->width(),
frame->SkiaSurface()->height());
} else {
last_submitted_frame_size_ = SkISize::MakeEmpty();
}
submitted_frame_count_++;
}

Expand Down
4 changes: 4 additions & 0 deletions shell/common/shell_test_external_view_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
// the external view embedder.
int GetSubmittedFrameCount();

// Returns the size of last submitted frame surface
SkISize GetLastSubmittedFrameSize();

private:
// |ExternalViewEmbedder|
void CancelFrame() override;
Expand Down Expand Up @@ -80,6 +83,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
bool support_thread_merging_;

std::atomic<int> submitted_frame_count_;
std::atomic<SkISize> last_submitted_frame_size_;

FML_DISALLOW_COPY_AND_ASSIGN(ShellTestExternalViewEmbedder);
};
Expand Down
55 changes: 55 additions & 0 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2011,5 +2011,60 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) {
DestroyShell(std::move(shell));
}

TEST_F(ShellTest, DiscardLayerTreeOnResize) {
auto settings = CreateSettingsForFixture();

SkISize wrong_size = SkISize::Make(400, 100);
SkISize expected_size = SkISize::Make(400, 200);

fml::AutoResetWaitableEvent end_frame_latch;

auto end_frame_callback = [&](bool, fml::RefPtr<fml::RasterThreadMerger>) {
end_frame_latch.Signal();
};

std::shared_ptr<ShellTestExternalViewEmbedder> external_view_embedder =
std::make_shared<ShellTestExternalViewEmbedder>(
std::move(end_frame_callback), PostPrerollResult::kSuccess, true);

std::unique_ptr<Shell> shell = CreateShell(
settings, GetTaskRunnersForFixture(), false, external_view_embedder);

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(),
[&shell, &expected_size]() {
shell->GetPlatformView()->SetViewportMetrics(
{1.0, static_cast<double>(expected_size.width()),
static_cast<double>(expected_size.height())});
});

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));

fml::WeakPtr<RuntimeDelegate> runtime_delegate = shell->GetEngine();

PumpOneFrame(shell.get(), static_cast<double>(wrong_size.width()),
static_cast<double>(wrong_size.height()));

end_frame_latch.Wait();

ASSERT_EQ(0, external_view_embedder->GetSubmittedFrameCount());

PumpOneFrame(shell.get(), static_cast<double>(expected_size.width()),
static_cast<double>(expected_size.height()));

end_frame_latch.Wait();

ASSERT_EQ(1, external_view_embedder->GetSubmittedFrameCount());
ASSERT_EQ(expected_size, external_view_embedder->GetLastSubmittedFrameSize());

DestroyShell(std::move(shell));
}

} // namespace testing
} // namespace flutter

0 comments on commit 5c9dddc

Please sign in to comment.