github/flutter
7
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2025-06-03 00:51:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Impeller] acquire the gpu sync switch when flush stored GPU tasks. (#169596)

Browse Source 
Potential fix for https://github.com/flutter/flutter/issues/166668

See:

*
https://github.com/flutter/flutter/blob/master/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm#L429
*
5d013c73ba/engine/src/flutter/fml/synchronization/sync_switch.cc (L41)

We don't hold the sync switch lock when we flush tasks. So if we start
flushing then immediately go to the background, then we might execute
while backgrounded.
This commit is contained in:
Jonah Williams 2025-05-28 14:28:17 -07:00 committed by GitHub
parent 3e8b531b18
commit af3627ac2b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 107 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
engine/src/flutter/ci/licenses_golden/excluded_files
View File

@ -191,6 +191,7 @@
../../../flutter/impeller/renderer/backend/gles/test
../../../flutter/impeller/renderer/backend/gles/unique_handle_gles_unittests.cc
../../../flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/metal/context_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/metal/texture_mtl_unittests.mm
../../../flutter/impeller/renderer/backend/vulkan/allocator_vk_unittests.cc

5
engine/src/flutter/impeller/playground/backend/metal/playground_impl_mtl.h
View File

@ -38,7 +38,7 @@ class PlaygroundImplMTL final : public PlaygroundImpl {
std::shared_ptr<ContextMTL> context_;
std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop_;
std::shared_ptr<SwapchainTransientsMTL> swapchain_transients_;
std::shared_ptr<const fml::SyncSwitch> is_gpu_disabled_sync_switch_;
std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch_;
// |PlaygroundImpl|
std::shared_ptr<Context> GetContext() const override;
@ -50,6 +50,9 @@ class PlaygroundImplMTL final : public PlaygroundImpl {
std::unique_ptr<Surface> AcquireSurfaceFrame(
std::shared_ptr<Context> context) override;
// |PlaygroundImpl|
void SetGPUDisabled(bool disabled) const override;
PlaygroundImplMTL(const PlaygroundImplMTL&) = delete;
PlaygroundImplMTL& operator=(const PlaygroundImplMTL&) = delete;

4
engine/src/flutter/impeller/playground/backend/metal/playground_impl_mtl.mm
View File

@ -138,4 +138,8 @@ fml::Status PlaygroundImplMTL::SetCapabilities(
return fml::Status();
}
void PlaygroundImplMTL::SetGPUDisabled(bool disabled) const {
is_gpu_disabled_sync_switch_->SetSwitch(disabled);
}
} // namespace impeller

4
engine/src/flutter/impeller/playground/playground.cc
View File

@ -527,4 +527,8 @@ Playground::VKProcAddressResolver Playground::CreateVKProcAddressResolver()
return impl_->CreateVKProcAddressResolver();
}
void Playground::SetGPUDisabled(bool value) const {
impl_->SetGPUDisabled(value);
}
} // namespace impeller

5
engine/src/flutter/impeller/playground/playground.h
View File

@ -122,6 +122,11 @@ class Playground {
std::function<void*(void* instance, const char* proc_name)>;
VKProcAddressResolver CreateVKProcAddressResolver() const;
/// @brief Mark the GPU as unavilable.
///
/// Only supported on the Metal backend.
void SetGPUDisabled(bool disabled) const;
protected:
const PlaygroundSwitches switches_;

2
engine/src/flutter/impeller/playground/playground_impl.h
View File

@ -40,6 +40,8 @@ class PlaygroundImpl {
virtual Playground::VKProcAddressResolver CreateVKProcAddressResolver() const;
virtual void SetGPUDisabled(bool disabled) const {}
protected:
const PlaygroundSwitches switches_;

1
engine/src/flutter/impeller/renderer/backend/metal/BUILD.gn
View File

@ -70,6 +70,7 @@ impeller_component("metal_unittests") {
sources = [
"allocator_mtl_unittests.mm",
"context_mtl_unittests.mm",
"swapchain_transients_mtl_unittests.mm",
"texture_mtl_unittests.mm",
]

5
engine/src/flutter/impeller/renderer/backend/metal/context_mtl.h
View File

@ -146,6 +146,9 @@ class ContextMTL final : public Context,
void StoreTaskForGPU(const fml::closure& task,
const fml::closure& failure) override;
// visible for testing.
void FlushTasksAwaitingGPU();
private:
class SyncSwitchObserver : public fml::SyncSwitch::Observer {
public:
@ -191,8 +194,6 @@ class ContextMTL final : public Context,
std::shared_ptr<CommandBuffer> CreateCommandBufferInQueue(
id<MTLCommandQueue> queue) const;
void FlushTasksAwaitingGPU();
ContextMTL(const ContextMTL&) = delete;
ContextMTL& operator=(const ContextMTL&) = delete;

20
engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm
View File

@ -418,8 +418,26 @@ void ContextMTL::FlushTasksAwaitingGPU() {
Lock lock(tasks_awaiting_gpu_mutex_);
std::swap(tasks_awaiting_gpu, tasks_awaiting_gpu_);
}
std::vector<PendingTasks> tasks_to_queue;
for (const auto& task : tasks_awaiting_gpu) {
task.task();
is_gpu_disabled_sync_switch_->Execute(fml::SyncSwitch::Handlers()
.SetIfFalse([&] { task.task(); })
.SetIfTrue([&] {
// Lost access to the GPU
// immediately after it was
// activated. This may happen if
// the app was quickly
// foregrounded/backgrounded
// from a push notification.
// Store the tasks on the
// context again.
tasks_to_queue.push_back(task);
}));
}
if (!tasks_to_queue.empty()) {
Lock lock(tasks_awaiting_gpu_mutex_);
tasks_awaiting_gpu_.insert(tasks_awaiting_gpu_.end(),
tasks_to_queue.begin(), tasks_to_queue.end());
}
}

64
engine/src/flutter/impeller/renderer/backend/metal/context_mtl_unittests.mm Normal file
View File

@ -0,0 +1,64 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "flutter/testing/testing.h"
#include "impeller/core/device_buffer_descriptor.h"
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
#include "impeller/playground/playground_test.h"
#include "impeller/renderer/backend/metal/allocator_mtl.h"
#include "impeller/renderer/backend/metal/context_mtl.h"
#include "impeller/renderer/backend/metal/formats_mtl.h"
#include "impeller/renderer/backend/metal/texture_mtl.h"
#include "impeller/renderer/capabilities.h"
#include <QuartzCore/CAMetalLayer.h>
#include <memory>
#include <thread>
#include "gtest/gtest.h"
namespace impeller {
namespace testing {
using ContextMTLTest = PlaygroundTest;
INSTANTIATE_METAL_PLAYGROUND_SUITE(ContextMTLTest);
TEST_P(ContextMTLTest, FlushTask) {
auto& context_mtl = ContextMTL::Cast(*GetContext());
int executed = 0;
int failed = 0;
context_mtl.StoreTaskForGPU([&]() { executed++; }, [&]() { failed++; });
context_mtl.FlushTasksAwaitingGPU();
EXPECT_EQ(executed, 1);
EXPECT_EQ(failed, 0);
}
TEST_P(ContextMTLTest, FlushTaskWithGPULoss) {
auto& context_mtl = ContextMTL::Cast(*GetContext());
int executed = 0;
int failed = 0;
context_mtl.StoreTaskForGPU([&]() { executed++; }, [&]() { failed++; });
// If tasks are flushed while the GPU is disabled, then
// they should not be executed.
SetGPUDisabled(/*disabled=*/true);
context_mtl.FlushTasksAwaitingGPU();
EXPECT_EQ(executed, 0);
EXPECT_EQ(failed, 0);
// Toggling availibility should flush tasks.
SetGPUDisabled(/*disabled=*/false);
EXPECT_EQ(executed, 1);
EXPECT_EQ(failed, 0);
}
} // namespace testing
} // namespace impeller