From 1cd6cb0cdc88b7fddd9e7bd637fa028474d04eba Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 5 Mar 2025 12:40:20 -0800 Subject: [PATCH] [Impeller] fix macOS managed memory. (#164635) Still crashes with non UMA GPU. Reason is that the if check is using `MTLResourceStorageModeManaged` when it should be using `MTLStorageModeManaged` Fixes https://github.com/flutter/flutter/issues/163218 --- .../renderer/backend/metal/allocator_mtl.h | 7 +++++-- .../renderer/backend/metal/allocator_mtl.mm | 4 ++++ .../backend/metal/allocator_mtl_unittests.mm | 17 +++++++++++++++++ .../renderer/backend/metal/device_buffer_mtl.mm | 2 +- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h index c07a6269fb2..4764c00b419 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h @@ -44,6 +44,11 @@ class AllocatorMTL final : public Allocator { // |Allocator| Bytes DebugGetHeapUsage() const override; + // visible for testing. + void DebugSetSupportsUMA(bool value); + + AllocatorMTL(id device, std::string label); + private: friend class ContextMTL; @@ -60,8 +65,6 @@ class AllocatorMTL final : public Allocator { ISize max_texture_supported_; - AllocatorMTL(id device, std::string label); - // |Allocator| bool IsValid() const; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm index 56cd8279125..6233f249ec3 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm @@ -255,6 +255,10 @@ ISize AllocatorMTL::GetMaxTextureSizeSupported() const { return max_texture_supported_; } +void AllocatorMTL::DebugSetSupportsUMA(bool value) { + supports_uma_ = value; +} + Bytes AllocatorMTL::DebugGetHeapUsage() const { #ifdef IMPELLER_DEBUG return debug_allocater_->GetAllocationSize(); diff --git a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm index fe351f5f693..b490b3af13c 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl_unittests.mm @@ -3,6 +3,7 @@ // 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" @@ -70,5 +71,21 @@ TEST_P(AllocatorMTLTest, DebugTraceMemoryStatistics) { 0u); } +TEST_P(AllocatorMTLTest, ManagedMemory) { + auto& context_mtl = ContextMTL::Cast(*GetContext()); + auto allocator = std::make_unique(context_mtl.GetMTLDevice(), + "test-allocator"); + allocator->DebugSetSupportsUMA(false); + + DeviceBufferDescriptor desc; + desc.size = 100; + desc.storage_mode = StorageMode::kHostVisible; + + auto buffer = allocator->CreateBuffer(desc); + ASSERT_TRUE(buffer); + + EXPECT_NE(buffer->OnGetContents(), nullptr); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm index 6d5d846acea..f01ccce5bc3 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/device_buffer_mtl.mm @@ -25,7 +25,7 @@ id DeviceBufferMTL::GetMTLBuffer() const { uint8_t* DeviceBufferMTL::OnGetContents() const { #if !FML_OS_IOS if (storage_mode_ != MTLStorageModeShared && - storage_mode_ != MTLResourceStorageModeManaged) { + storage_mode_ != MTLStorageModeManaged) { return nullptr; } #else