Skip to content

Commit

Permalink
[Impeller] Disable raster stats service protocol. (#47206)
Browse files Browse the repository at this point in the history
As far as I can tell, this functionality has _always_ been falling back to software rendering with Skia when Impeller was enabled. This is at best extremely misleading. Since I started putting Impeller specific objects in the display list it introduced crashes, and must be disabled.

Fixes flutter/flutter#136847
  • Loading branch information
jonahwilliams authored Oct 23, 2023
1 parent ef568c3 commit e2d006d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 0 deletions.
2 changes: 2 additions & 0 deletions flow/layers/layer_tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class LayerTree {
/// When `Paint` is called, if leaf layer tracing is enabled, additional
/// metadata around raterization of leaf layers is collected.
///
/// This is not supported in the Impeller backend.
///
/// See: `LayerSnapshotStore`
void enable_leaf_layer_tracing(bool enable) {
enable_leaf_layer_tracing_ = enable;
Expand Down
7 changes: 7 additions & 0 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,13 @@ bool Shell::OnServiceProtocolRenderFrameWithRasterStats(
rapidjson::Document* response) {
FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());

// Impeller does not support this protocol method.
if (io_manager_->GetImpellerContext()) {
const char* error = "Raster status not supported on Impeller backend.";
ServiceProtocolFailureError(response, error);
return false;
}

// TODO(dkwingsmt): This method only handles view #0, including the snapshot
// and the frame size. We need to adapt this method to multi-view.
// https://github.com/flutter/flutter/issues/131892
Expand Down
41 changes: 41 additions & 0 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2709,6 +2709,47 @@ TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsWorks) {
DestroyShell(std::move(shell));
}

#if defined(FML_OS_MACOSX)
TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsDisableImpeller) {
auto settings = CreateSettingsForFixture();
settings.enable_impeller = true;
std::unique_ptr<Shell> shell = CreateShell({
.settings = settings,
.platform_view_create_callback = ShellTestPlatformViewBuilder({
.rendering_backend =
ShellTestPlatformView::BackendType::kMetalBackend,
}),
});

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

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

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

ServiceProtocol::Handler::ServiceProtocolMap empty_params;
rapidjson::Document document;
OnServiceProtocol(
shell.get(), ServiceProtocolEnum::kRenderFrameWithRasterStats,
shell->GetTaskRunners().GetRasterTaskRunner(), empty_params, &document);
rapidjson::StringBuffer buffer;
rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
document.Accept(writer);
std::string actual_json = buffer.GetString();
std::string expected_json =
"{\"code\":-32000,\"message\":\"Raster status not supported on Impeller "
"backend.\"}";

ASSERT_EQ(actual_json, expected_json);

PlatformViewNotifyDestroyed(shell.get());
DestroyShell(std::move(shell));
}
#endif // FML_OS_MACOSX

// TODO(https://github.com/flutter/flutter/issues/100273): Disabled due to
// flakiness.
// TODO(https://github.com/flutter/flutter/issues/100299): Fix it when
Expand Down

0 comments on commit e2d006d

Please sign in to comment.