diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 011df3aa4b676..ccad16a902d50 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -860,26 +860,36 @@ void Engine::WarmupSkps( SkISize size, std::shared_ptr asset_manager, std::optional> skp_names, - std::optional> completion_callback) { - // We use a raw pointer here because we want to keep this alive until all gpu - // work is done and the callbacks skia takes for this are function pointers - // so we are unable to use a lambda that captures the smart pointer. - SurfaceProducerSurface* skp_warmup_surface = - surface_producer->ProduceOffscreenSurface(size).release(); - if (!skp_warmup_surface) { - FML_LOG(ERROR) << "Failed to create offscreen warmup surface"; - // Tell client that zero shaders were warmed up because warmup failed. - if (completion_callback.has_value() && completion_callback.value()) { - completion_callback.value()(0); + std::optional> maybe_completion_callback, + bool synchronous) { + // Wrap the optional validity checks up in a lambda to simplify the various + // callsites below + auto completion_callback = [maybe_completion_callback](uint32_t skp_count) { + if (maybe_completion_callback.has_value() && + maybe_completion_callback.value()) { + maybe_completion_callback.value()(skp_count); } - return; - } + }; + + // We use this bizzare raw pointer to a smart pointer thing here because we + // want to keep the surface alive until all gpu work is done and the + // callbacks skia takes for this are function pointers so we are unable to + // use a lambda that captures the smart pointer. We need two levels of + // indirection because it needs to be the same across all invocations of the + // raster task lambda from a single invocation of WarmupSkps, but be + // different across different invocations of WarmupSkps (so we cant + // statically initialialize it in the lambda itself). Basically the result + // of a mashup of wierd call dynamics, multithreading, and lifecycle + // management with C style Skia callbacks. + std::unique_ptr* skp_warmup_surface = + new std::unique_ptr(nullptr); // tell concurrent task runner to deserialize all skps available from // the asset manager - concurrent_task_runner->PostTask([raster_task_runner, skp_warmup_surface, - surface_producer, asset_manager, skp_names, - completion_callback]() { + concurrent_task_runner->PostTask([raster_task_runner, size, + skp_warmup_surface, surface_producer, + asset_manager, skp_names, + completion_callback, synchronous]() { TRACE_DURATION("flutter", "DeserializeSkps"); std::vector> skp_mappings; if (skp_names) { @@ -895,6 +905,13 @@ void Engine::WarmupSkps( skp_mappings = asset_manager->GetAsMappings(".*\\.skp$", "shaders"); } + if (skp_mappings.size() == 0) { + FML_LOG(WARNING) + << "Engine::WarmupSkps got zero SKP mappings, returning early"; + completion_callback(0); + return; + } + size_t total_size = 0; for (auto& mapping : skp_mappings) { total_size += mapping->GetSize(); @@ -920,20 +937,35 @@ void Engine::WarmupSkps( // Tell raster task runner to warmup have the compositor // context warm up the newly deserialized picture - raster_task_runner->PostTask([skp_warmup_surface, picture, + raster_task_runner->PostTask([picture, skp_warmup_surface, size, surface_producer, completion_callback, i, - count = skp_mappings.size()] { + count = skp_mappings.size(), synchronous] { TRACE_DURATION("flutter", "WarmupSkp"); - skp_warmup_surface->GetSkiaSurface()->getCanvas()->drawPicture(picture); + if (*skp_warmup_surface == nullptr) { + skp_warmup_surface->reset( + surface_producer->ProduceOffscreenSurface(size).release()); + + if (*skp_warmup_surface == nullptr) { + FML_LOG(ERROR) << "Failed to create offscreen warmup surface"; + // Tell client that zero shaders were warmed up because warmup + // failed. + completion_callback(0); + return; + } + } + + // Do the actual warmup + (*skp_warmup_surface) + ->GetSkiaSurface() + ->getCanvas() + ->drawPicture(picture); if (i == count - 1) { // We call this here instead of inside fFinishedProc below because - // we want to unblock the dart animation code as soon as the raster - // thread is free to enque work, rather than waiting for the GPU work - // itself to finish. - if (completion_callback.has_value() && completion_callback.value()) { - completion_callback.value()(count); - } + // we want to unblock the dart animation code as soon as the + // raster thread is free to enque work, rather than waiting for + // the GPU work itself to finish. + completion_callback(count); } if (surface_producer->gr_context()) { @@ -946,11 +978,12 @@ void Engine::WarmupSkps( struct GrFlushInfo flush_info; flush_info.fFinishedContext = skp_warmup_surface; flush_info.fFinishedProc = [](void* skp_warmup_surface) { - delete static_cast(skp_warmup_surface); + delete static_cast*>( + skp_warmup_surface); }; surface_producer->gr_context()->flush(flush_info); - surface_producer->gr_context()->submit(); + surface_producer->gr_context()->submit(synchronous); } } else { if (i == count - 1) { diff --git a/shell/platform/fuchsia/flutter/engine.h b/shell/platform/fuchsia/flutter/engine.h index 76e4caa16ce35..e11c41e878c14 100644 --- a/shell/platform/fuchsia/flutter/engine.h +++ b/shell/platform/fuchsia/flutter/engine.h @@ -108,7 +108,8 @@ class Engine final : public fuchsia::memorypressure::Watcher { SkISize size, std::shared_ptr asset_manager, std::optional> skp_names, - std::optional> completion_callback); + std::optional> completion_callback, + bool synchronous = false); void OnMainIsolateStart(); diff --git a/shell/platform/fuchsia/flutter/tests/engine_unittests.cc b/shell/platform/fuchsia/flutter/tests/engine_unittests.cc index 10afd5fae5706..ef88ca17e9b6b 100644 --- a/shell/platform/fuchsia/flutter/tests/engine_unittests.cc +++ b/shell/platform/fuchsia/flutter/tests/engine_unittests.cc @@ -66,10 +66,11 @@ class EngineTest : public ::testing::Test { uint64_t height, std::shared_ptr asset_manager, std::optional> skp_names, - std::optional> completion_callback) { + std::optional> completion_callback, + bool synchronous = true) { // Have to create a message loop so default async dispatcher gets set, // otherwise we segfault creating the VulkanSurfaceProducer - auto loop = fml::MessageLoopImpl::Create(); + loop_ = fml::MessageLoopImpl::Create(); context_ = sys::ComponentContext::CreateAndServeOutgoingDirectory(); scenic_ = context_->svc()->Connect(); @@ -79,10 +80,12 @@ class EngineTest : public ::testing::Test { Engine::WarmupSkps(&concurrent_task_runner_, &raster_task_runner_, surface_producer_, SkISize::Make(width, height), - asset_manager, std::nullopt, std::nullopt); + asset_manager, skp_names, completion_callback, + synchronous); } protected: + fml::RefPtr loop_; MockTaskRunner concurrent_task_runner_; MockTaskRunner raster_task_runner_; std::shared_ptr surface_producer_; @@ -163,7 +166,7 @@ TEST_F(EngineTest, SkpWarmup) { std::make_unique(std::move(asset_dir_fd), false)); WarmupSkps(draw_size.width(), draw_size.height(), asset_manager, std::nullopt, - std::nullopt); + [](uint32_t count) { EXPECT_EQ(1u, count); }); concurrent_task_runner_.Run(); raster_task_runner_.Run(); @@ -196,8 +199,9 @@ TEST_F(EngineTest, SkpWarmupAsync) { fml::ScopedTemporaryDirectory asset_dir; fml::UniqueFD asset_dir_fd = fml::OpenDirectory( asset_dir.path().c_str(), false, fml::FilePermission::kRead); - fml::UniqueFD subdir_fd = fml::OpenDirectory(asset_dir_fd, "shaders", true, - fml::FilePermission::kReadWrite); + std::string subdir_name = "shaders"; + fml::UniqueFD subdir_fd = fml::OpenDirectory( + asset_dir_fd, subdir_name.c_str(), true, fml::FilePermission::kReadWrite); std::string skp_name = "test.skp"; bool success = fml::WriteAtomically(subdir_fd, skp_name.c_str(), mapping); @@ -207,7 +211,7 @@ TEST_F(EngineTest, SkpWarmupAsync) { asset_manager->PushBack( std::make_unique(std::move(asset_dir_fd), false)); - std::vector skp_names = {skp_name}; + std::vector skp_names = {subdir_name + "/" + skp_name}; WarmupSkps(draw_size.width(), draw_size.height(), asset_manager, skp_names, [](uint32_t count) { EXPECT_EQ(1u, count); });