Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Remove the dummy rasterizer delegate now that flutter_runner is in tree, and cleanup ctor params #20486

Merged
merged 2 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions shell/common/animator_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ TEST_F(ShellTest, VSyncTargetTime) {
std::move(create_vsync_waiter),
ShellTestPlatformView::BackendType::kDefaultBackend, nullptr);
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(
shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
});
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
ASSERT_TRUE(DartVMRef::IsInstanceRunning());

auto configuration = RunConfiguration::InferFromSettings(settings);
Expand Down
45 changes: 15 additions & 30 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,38 +25,18 @@ namespace flutter {
// used within this interval.
static constexpr std::chrono::milliseconds kSkiaCleanupExpiration(15000);

// TODO(dnfield): Remove this once internal embedders have caught up.
static Rasterizer::DummyDelegate dummy_delegate_;
Rasterizer::Rasterizer(
TaskRunners task_runners,
std::unique_ptr<flutter::CompositorContext> compositor_context,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
: Rasterizer(dummy_delegate_,
std::move(task_runners),
std::move(compositor_context),
is_gpu_disabled_sync_switch) {}

Rasterizer::Rasterizer(
Delegate& delegate,
TaskRunners task_runners,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
Rasterizer::Rasterizer(Delegate& delegate)
: Rasterizer(delegate,
std::move(task_runners),
std::make_unique<flutter::CompositorContext>(
delegate.GetFrameBudget()),
is_gpu_disabled_sync_switch) {}
delegate.GetFrameBudget())) {}

Rasterizer::Rasterizer(
Delegate& delegate,
TaskRunners task_runners,
std::unique_ptr<flutter::CompositorContext> compositor_context,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
std::unique_ptr<flutter::CompositorContext> compositor_context)
: delegate_(delegate),
task_runners_(std::move(task_runners)),
compositor_context_(std::move(compositor_context)),
user_override_resource_cache_bytes_(false),
weak_factory_(this),
is_gpu_disabled_sync_switch_(is_gpu_disabled_sync_switch) {
weak_factory_(this) {
FML_DCHECK(compositor_context_);
}

Expand All @@ -83,8 +63,9 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
// support for raster thread merger for Fuchsia.
if (surface_->GetExternalViewEmbedder()) {
const auto platform_id =
task_runners_.GetPlatformTaskRunner()->GetTaskQueueId();
const auto gpu_id = task_runners_.GetRasterTaskRunner()->GetTaskQueueId();
delegate_.GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId();
const auto gpu_id =
delegate_.GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId();
raster_thread_merger_ =
fml::MakeRefCounted<fml::RasterThreadMerger>(platform_id, gpu_id);
}
Expand Down Expand Up @@ -134,7 +115,9 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
// we yield and let this frame be serviced on the right thread.
return;
}
FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());
FML_DCHECK(delegate_.GetTaskRunners()
.GetRasterTaskRunner()
->RunsTasksOnCurrentThread());

RasterStatus raster_status = RasterStatus::kFailed;
Pipeline<flutter::LayerTree>::Consumer consumer =
Expand Down Expand Up @@ -168,7 +151,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
// between successive tries.
switch (consume_result) {
case PipelineConsumeResult::MoreAvailable: {
task_runners_.GetRasterTaskRunner()->PostTask(
delegate_.GetTaskRunners().GetRasterTaskRunner()->PostTask(
[weak_this = weak_factory_.GetWeakPtr(), pipeline]() {
if (weak_this) {
weak_this->Draw(pipeline);
Expand Down Expand Up @@ -226,7 +209,7 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
result = DrawSnapshot(surface, draw_callback);
} else {
is_gpu_disabled_sync_switch_->Execute(
delegate_.GetIsGpuDisabledSyncSwitch()->Execute(
fml::SyncSwitch::Handlers()
.SetIfTrue([&] {
sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
Expand Down Expand Up @@ -282,7 +265,9 @@ sk_sp<SkImage> Rasterizer::ConvertToRasterImage(sk_sp<SkImage> image) {

RasterStatus Rasterizer::DoDraw(
std::unique_ptr<flutter::LayerTree> layer_tree) {
FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());
FML_DCHECK(delegate_.GetTaskRunners()
.GetRasterTaskRunner()
->RunsTasksOnCurrentThread());

if (!layer_tree || !surface_) {
return RasterStatus::kFailed;
Expand Down
67 changes: 12 additions & 55 deletions shell/common/rasterizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,82 +74,41 @@ class Rasterizer final : public SnapshotDelegate {
/// Target time for the latest frame. See also `Shell::OnAnimatorBeginFrame`
/// for when this time gets updated.
virtual fml::TimePoint GetLatestFrameTargetTime() const = 0;
};

// TODO(dnfield): remove once embedders have caught up.
class DummyDelegate : public Delegate {
void OnFrameRasterized(const FrameTiming&) override {}
fml::Milliseconds GetFrameBudget() override {
return fml::kDefaultFrameBudget;
}
// Returning a time in the past so we don't add additional trace
// events when exceeding the frame budget for other embedders.
fml::TimePoint GetLatestFrameTargetTime() const override {
return fml::TimePoint::FromEpochDelta(fml::TimeDelta::Zero());
}
};
/// Task runners used by the shell.
virtual const TaskRunners& GetTaskRunners() const = 0;

//----------------------------------------------------------------------------
/// @brief Creates a new instance of a rasterizer. Rasterizers may only
/// be created on the GPU task runner. Rasterizers are currently
/// only created by the shell. Usually, the shell also sets itself
/// up as the rasterizer delegate. But, this constructor sets up a
/// dummy rasterizer delegate.
///
// TODO(chinmaygarde): The rasterizer does not use the task runners for
// anything other than thread checks. Remove the same as an argument.
///
/// @param[in] task_runners The task runners used by the shell.
/// @param[in] compositor_context The compositor context used to hold all
/// the GPU state used by the rasterizer.
/// @param[in] is_gpu_disabled_sync_switch
/// A `SyncSwitch` for handling disabling of the GPU (typically happens
/// when an app is backgrounded)
///
Rasterizer(TaskRunners task_runners,
std::unique_ptr<flutter::CompositorContext> compositor_context,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
/// Accessor for the shell's GPU sync switch, which determines whether GPU
/// operations are allowed on the current thread.
///
/// For example, on some platforms when the application is backgrounded it
/// is critical that GPU operations are not processed.
virtual std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch()
const = 0;
};

//----------------------------------------------------------------------------
/// @brief Creates a new instance of a rasterizer. Rasterizers may only
/// be created on the GPU task runner. Rasterizers are currently
/// only created by the shell (which also sets itself up as the
/// rasterizer delegate).
///
// TODO(chinmaygarde): The rasterizer does not use the task runners for
// anything other than thread checks. Remove the same as an argument.
///
/// @param[in] delegate The rasterizer delegate.
/// @param[in] task_runners The task runners used by the shell.
/// @param[in] is_gpu_disabled_sync_switch
/// A `SyncSwitch` for handling disabling of the GPU (typically happens
/// when an app is backgrounded)
///
Rasterizer(Delegate& delegate,
TaskRunners task_runners,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
Rasterizer(Delegate& delegate);

//----------------------------------------------------------------------------
/// @brief Creates a new instance of a rasterizer. Rasterizers may only
/// be created on the GPU task runner. Rasterizers are currently
/// only created by the shell (which also sets itself up as the
/// rasterizer delegate).
///
// TODO(chinmaygarde): The rasterizer does not use the task runners for
// anything other than thread checks. Remove the same as an argument.
///
/// @param[in] delegate The rasterizer delegate.
/// @param[in] task_runners The task runners used by the shell.
/// @param[in] compositor_context The compositor context used to hold all
/// the GPU state used by the rasterizer.
/// @param[in] is_gpu_disabled_sync_switch
/// A `SyncSwitch` for handling disabling of the GPU (typically happens
/// when an app is backgrounded)
///
Rasterizer(Delegate& delegate,
TaskRunners task_runners,
std::unique_ptr<flutter::CompositorContext> compositor_context,
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
std::unique_ptr<flutter::CompositorContext> compositor_context);

//----------------------------------------------------------------------------
/// @brief Destroys the rasterizer. This must happen on the GPU task
Expand Down Expand Up @@ -432,7 +391,6 @@ class Rasterizer final : public SnapshotDelegate {

private:
Delegate& delegate_;
TaskRunners task_runners_;
std::unique_ptr<Surface> surface_;
std::unique_ptr<flutter::CompositorContext> compositor_context_;
// This is the last successfully rasterized layer tree.
Expand All @@ -446,7 +404,6 @@ class Rasterizer final : public SnapshotDelegate {
std::optional<size_t> max_cache_bytes_;
fml::TaskRunnerAffineWeakPtrFactory<Rasterizer> weak_factory_;
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch_;

// |SnapshotDelegate|
sk_sp<SkImage> MakeRasterSnapshot(sk_sp<SkPicture> picture,
Expand Down
4 changes: 2 additions & 2 deletions shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ class Shell final : public PlatformView::Delegate,
///
/// @return The task runners current in use by the shell.
///
const TaskRunners& GetTaskRunners() const;
const TaskRunners& GetTaskRunners() const override;

//----------------------------------------------------------------------------
/// @brief Rasterizers may only be accessed on the GPU task runner.
Expand Down Expand Up @@ -352,7 +352,7 @@ class Shell final : public PlatformView::Delegate,

//----------------------------------------------------------------------------
/// @brief Accessor for the disable GPU SyncSwitch
std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch() const;
std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch() const override;

//----------------------------------------------------------------------------
/// @brief Get a pointer to the Dart VM used by this running shell
Expand Down
6 changes: 1 addition & 5 deletions shell/common/shell_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ static void StartupAndShutdownShell(benchmark::State& state,
[](Shell& shell) {
return std::make_unique<PlatformView>(shell, shell.GetTaskRunners());
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(
shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
});
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
}

FML_CHECK(shell);
Expand Down
5 changes: 1 addition & 4 deletions shell/common/shell_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,7 @@ std::unique_ptr<Shell> ShellTest::CreateShell(
ShellTestPlatformView::BackendType::kDefaultBackend,
shell_test_external_view_embedder);
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
});
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
}
void ShellTest::DestroyShell(std::unique_ptr<Shell> shell) {
DestroyShell(std::move(shell), GetTaskRunnersForFixture());
Expand Down
5 changes: 1 addition & 4 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,7 @@ TEST_F(ShellTest,
},
ShellTestPlatformView::BackendType::kDefaultBackend, nullptr);
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
});
[](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
ASSERT_TRUE(ValidateShell(shell.get()));
ASSERT_TRUE(DartVMRef::IsInstanceRunning());
DestroyShell(std::move(shell), std::move(task_runners));
Expand Down
3 changes: 1 addition & 2 deletions shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ AndroidShellHolder::AndroidShellHolder(
};

Shell::CreateCallback<Rasterizer> on_create_rasterizer = [](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
return std::make_unique<Rasterizer>(shell);
};

// The current thread will be used as the platform thread. Ensure that the
Expand Down
5 changes: 1 addition & 4 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,7 @@ - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
};

flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
[](flutter::Shell& shell) {
return std::make_unique<flutter::Rasterizer>(shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
};
[](flutter::Shell& shell) { return std::make_unique<flutter::Rasterizer>(shell); };

if (flutter::IsIosEmbeddedViewsPreviewEnabled()) {
// Embedded views requires the gpu and the platform views to be the same.
Expand Down
3 changes: 1 addition & 2 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -991,8 +991,7 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,

flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
[](flutter::Shell& shell) {
return std::make_unique<flutter::Rasterizer>(
shell, shell.GetTaskRunners(), shell.GetIsGpuDisabledSyncSwitch());
return std::make_unique<flutter::Rasterizer>(shell);
};

// TODO(chinmaygarde): This is the wrong spot for this. It belongs in the
Expand Down
4 changes: 1 addition & 3 deletions shell/platform/fuchsia/flutter/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,7 @@ Engine::Engine(Delegate& delegate,
}

return std::make_unique<flutter::Rasterizer>(
/*task_runners=*/shell.GetTaskRunners(),
/*compositor_context=*/std::move(compositor_context),
/*is_gpu_disabled_sync_switch=*/shell.GetIsGpuDisabledSyncSwitch());
shell, std::move(compositor_context));
});

UpdateNativeThreadLabelNames(thread_label_, task_runners);
Expand Down
3 changes: 1 addition & 2 deletions shell/testing/tester_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ int RunTester(const flutter::Settings& settings,
};

Shell::CreateCallback<Rasterizer> on_create_rasterizer = [](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
shell.GetIsGpuDisabledSyncSwitch());
return std::make_unique<Rasterizer>(shell);
};

auto shell = Shell::Create(task_runners, //
Expand Down