From 831a7a179dba2da1766880a24b075d107104f78f Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 28 Jul 2025 14:16:15 -0700 Subject: [PATCH] [Flutter GPU] Fix Android/GLES crashers. (#172588) Continuation of https://github.com/flutter/flutter/pull/165630. Resolves https://github.com/flutter/flutter/issues/164278. Resolves https://github.com/bdero/flutter_scene/issues/35. Partly fixes: https://github.com/flutter/flutter/issues/160948. * Fixes OpenGLES crashers. * Threadsafe GLES pipeline creation. * Threadsafe GLES texture creation. * Fix GLES internal float texture formats. * Encode GLES passes on the raster thread. * Overwrite GLES textures on the raster thread. * Fixes vulkan memory leak (https://github.com/flutter/flutter/issues/164278). --- engine/src/flutter/impeller/core/allocator.cc | 8 ++-- engine/src/flutter/impeller/core/allocator.h | 21 ++++++++++- .../entity/contents/host_buffer_unittests.cc | 3 +- .../entity/render_target_cache_unittests.cc | 3 +- .../renderer/backend/gles/allocator_gles.cc | 5 ++- .../renderer/backend/gles/allocator_gles.h | 4 +- .../backend/gles/pipeline_library_gles.cc | 37 ++++++++++++------- .../backend/gles/pipeline_library_gles.h | 6 ++- .../renderer/backend/gles/texture_gles.cc | 19 ++++++---- .../renderer/backend/gles/texture_gles.h | 5 ++- .../renderer/backend/metal/allocator_mtl.h | 4 +- .../renderer/backend/metal/allocator_mtl.mm | 3 +- .../backend/metal/pipeline_library_mtl.h | 3 +- .../backend/metal/pipeline_library_mtl.mm | 3 +- .../renderer/backend/vulkan/allocator_vk.cc | 3 +- .../renderer/backend/vulkan/allocator_vk.h | 4 +- .../backend/vulkan/pipeline_library_vk.cc | 3 +- .../backend/vulkan/pipeline_library_vk.h | 6 ++- .../impeller/renderer/pipeline_library.h | 22 ++++++++++- .../flutter/impeller/renderer/testing/mocks.h | 2 +- engine/src/flutter/lib/gpu/command_buffer.cc | 23 +++++++----- engine/src/flutter/lib/gpu/context.cc | 18 +++++---- engine/src/flutter/lib/gpu/context.h | 4 +- engine/src/flutter/lib/gpu/device_buffer.cc | 4 +- .../src/flutter/lib/gpu/lib/src/texture.dart | 11 ++++-- engine/src/flutter/lib/gpu/render_pass.cc | 7 ++-- engine/src/flutter/lib/gpu/shader.cc | 4 +- engine/src/flutter/lib/gpu/texture.cc | 28 ++++++++++++-- engine/src/flutter/lib/gpu/texture.h | 8 ++-- .../painting/image_decoder_no_gl_unittests.h | 4 +- 30 files changed, 190 insertions(+), 85 deletions(-) diff --git a/engine/src/flutter/impeller/core/allocator.cc b/engine/src/flutter/impeller/core/allocator.cc index 19314c5bfcd..7e0f472d15e 100644 --- a/engine/src/flutter/impeller/core/allocator.cc +++ b/engine/src/flutter/impeller/core/allocator.cc @@ -46,8 +46,8 @@ std::shared_ptr Allocator::CreateBuffer( return OnCreateBuffer(desc); } -std::shared_ptr Allocator::CreateTexture( - const TextureDescriptor& desc) { +std::shared_ptr Allocator::CreateTexture(const TextureDescriptor& desc, + bool threadsafe) { const auto max_size = GetMaxTextureSizeSupported(); if (desc.size.width > max_size.width || desc.size.height > max_size.height) { VALIDATION_LOG << "Requested texture size " << desc.size @@ -60,10 +60,10 @@ std::shared_ptr Allocator::CreateTexture( << " exceeds maximum supported for size " << desc.size; TextureDescriptor corrected_desc = desc; corrected_desc.mip_count = desc.size.MipCount(); - return OnCreateTexture(corrected_desc); + return OnCreateTexture(corrected_desc, threadsafe); } - return OnCreateTexture(desc); + return OnCreateTexture(desc, threadsafe); } uint16_t Allocator::MinimumBytesPerRow(PixelFormat format) const { diff --git a/engine/src/flutter/impeller/core/allocator.h b/engine/src/flutter/impeller/core/allocator.h index 50da86fe10b..9a3ce694a3a 100644 --- a/engine/src/flutter/impeller/core/allocator.h +++ b/engine/src/flutter/impeller/core/allocator.h @@ -30,7 +30,23 @@ class Allocator { std::shared_ptr CreateBuffer( const DeviceBufferDescriptor& desc); - std::shared_ptr CreateTexture(const TextureDescriptor& desc); + //------------------------------------------------------------------------------ + /// @brief Creates a new texture. + /// + /// @param[in] desc The descriptor of the texture to create. + /// @param[in] threadsafe Whether mutations to this texture should be + /// protected with a threadsafe barrier. + /// + /// This parameter only affects the OpenGLES rendering + /// backend. + /// + /// If any interaction with this texture (including + /// creation) will be done on a thread other than + /// where the OpenGLES context resides, then + /// `threadsafe`, must be set to `true`. + /// + std::shared_ptr CreateTexture(const TextureDescriptor& desc, + bool threadsafe = false); //------------------------------------------------------------------------------ /// @brief Minimum value for `row_bytes` on a Texture. The row @@ -62,7 +78,8 @@ class Allocator { const DeviceBufferDescriptor& desc) = 0; virtual std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) = 0; + const TextureDescriptor& desc, + bool threadsafe = false) = 0; private: Allocator(const Allocator&) = delete; diff --git a/engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc b/engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc index 7ba47b780f8..573417276d8 100644 --- a/engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc @@ -206,7 +206,8 @@ class FailingAllocator : public Allocator { return delegate_->CreateBuffer(desc); } - std::shared_ptr OnCreateTexture(const TextureDescriptor& desc) { + std::shared_ptr OnCreateTexture(const TextureDescriptor& desc, + bool threadsafe) { return delegate_->CreateTexture(desc); } diff --git a/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc b/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc index 4fe3ffb0c9c..498c11e5884 100644 --- a/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc +++ b/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc @@ -39,7 +39,8 @@ class TestAllocator : public Allocator { }; virtual std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override { + const TextureDescriptor& desc, + bool threadsafe) override { if (should_fail) { return nullptr; } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.cc index 820ec22d691..16e7ccd91d3 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.cc @@ -39,8 +39,9 @@ std::shared_ptr AllocatorGLES::OnCreateBuffer( // |Allocator| std::shared_ptr AllocatorGLES::OnCreateTexture( - const TextureDescriptor& desc) { - return std::make_shared(reactor_, desc); + const TextureDescriptor& desc, + bool threadsafe) { + return std::make_shared(reactor_, desc, threadsafe); } // |Allocator| diff --git a/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.h index 14728414f46..89bc377bd9d 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/allocator_gles.h @@ -31,8 +31,8 @@ class AllocatorGLES final : public Allocator { const DeviceBufferDescriptor& desc) override; // |Allocator| - std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override; + std::shared_ptr OnCreateTexture(const TextureDescriptor& desc, + bool threadsafe) override; // |Allocator| ISize GetMaxTextureSizeSupported() const override; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.cc index 215d5b6d719..765345cac38 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.cc @@ -185,7 +185,8 @@ std::shared_ptr PipelineLibraryGLES::CreatePipeline( const std::weak_ptr& weak_library, const PipelineDescriptor& desc, const std::shared_ptr& vert_function, - const std::shared_ptr& frag_function) { + const std::shared_ptr& frag_function, + bool threadsafe) { auto strong_library = weak_library.lock(); if (!strong_library) { @@ -209,14 +210,22 @@ std::shared_ptr PipelineLibraryGLES::CreatePipeline( const auto has_cached_program = !!cached_program; - auto pipeline = std::shared_ptr(new PipelineGLES( - reactor, // - weak_library, // - desc, // - has_cached_program - ? std::move(cached_program) - : std::make_shared(UniqueHandleGLES::MakeUntracked( - reactor, HandleType::kProgram)))); + std::shared_ptr program_handle = nullptr; + if (has_cached_program) { + program_handle = std::move(cached_program); + } else { + program_handle = threadsafe ? std::make_shared( + reactor, HandleType::kProgram) + : std::make_shared( + UniqueHandleGLES::MakeUntracked( + reactor, HandleType::kProgram)); + } + + auto pipeline = std::shared_ptr( + new PipelineGLES(reactor, // + weak_library, // + desc, // + std::move(program_handle))); auto program = reactor->GetGLHandle(pipeline->GetProgramHandle()); @@ -258,7 +267,8 @@ std::shared_ptr PipelineLibraryGLES::CreatePipeline( // |PipelineLibrary| PipelineFuture PipelineLibraryGLES::GetPipeline( PipelineDescriptor descriptor, - bool async) { + bool async, + bool threadsafe) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { return found->second; } @@ -290,10 +300,11 @@ PipelineFuture PipelineLibraryGLES::GetPipeline( weak_this = weak_from_this(), // descriptor, // vert_function, // - frag_function // + frag_function, // + threadsafe // ](const ReactorGLES& reactor) { - promise->set_value( - CreatePipeline(weak_this, descriptor, vert_function, frag_function)); + promise->set_value(CreatePipeline(weak_this, descriptor, vert_function, + frag_function, threadsafe)); }); FML_CHECK(result); diff --git a/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.h index 3b9f29f7d22..25c51c8113d 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/pipeline_library_gles.h @@ -99,7 +99,8 @@ class PipelineLibraryGLES final // |PipelineLibrary| PipelineFuture GetPipeline(PipelineDescriptor descriptor, - bool async) override; + bool async, + bool threadsafe) override; // |PipelineLibrary| PipelineFuture GetPipeline( @@ -119,7 +120,8 @@ class PipelineLibraryGLES final const std::weak_ptr& weak_library, const PipelineDescriptor& desc, const std::shared_ptr& vert_shader, - const std::shared_ptr& frag_shader); + const std::shared_ptr& frag_shader, + bool threadsafe); std::shared_ptr GetProgramForKey(const ProgramKey& key); diff --git a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc index 402fb220daa..2f4008bfbdf 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc @@ -88,12 +88,12 @@ struct TexImage2DData { type = GL_UNSIGNED_BYTE; break; case PixelFormat::kR32G32B32A32Float: - internal_format = GL_RGBA; + internal_format = GL_RGBA32F; external_format = GL_RGBA; type = GL_FLOAT; break; case PixelFormat::kR16G16B16A16Float: - internal_format = GL_RGBA; + internal_format = GL_RGBA16F; external_format = GL_RGBA; type = GL_HALF_FLOAT; break; @@ -149,7 +149,7 @@ std::shared_ptr TextureGLES::WrapFBO( TextureDescriptor desc, GLuint fbo) { auto texture = std::shared_ptr( - new TextureGLES(std::move(reactor), desc, fbo, std::nullopt)); + new TextureGLES(std::move(reactor), desc, false, fbo, std::nullopt)); if (!texture->IsValid()) { return nullptr; } @@ -168,8 +168,8 @@ std::shared_ptr TextureGLES::WrapTexture( VALIDATION_LOG << "Cannot wrap a non-texture handle."; return nullptr; } - auto texture = std::shared_ptr( - new TextureGLES(std::move(reactor), desc, std::nullopt, external_handle)); + auto texture = std::shared_ptr(new TextureGLES( + std::move(reactor), desc, false, std::nullopt, external_handle)); if (!texture->IsValid()) { return nullptr; } @@ -183,15 +183,18 @@ std::shared_ptr TextureGLES::CreatePlaceholder( } TextureGLES::TextureGLES(std::shared_ptr reactor, - TextureDescriptor desc) + TextureDescriptor desc, + bool threadsafe) : TextureGLES(std::move(reactor), // desc, // + threadsafe, // std::nullopt, // std::nullopt // ) {} TextureGLES::TextureGLES(std::shared_ptr reactor, TextureDescriptor desc, + bool threadsafe, std::optional fbo, std::optional external_handle) : Texture(desc), @@ -201,7 +204,9 @@ TextureGLES::TextureGLES(std::shared_ptr reactor, reactor_->GetProcTable().GetCapabilities())), handle_(external_handle.has_value() ? external_handle.value() - : reactor_->CreateUntrackedHandle(ToHandleType(type_))), + : (threadsafe ? reactor_->CreateHandle(ToHandleType(type_)) + : reactor_->CreateUntrackedHandle( + ToHandleType(type_)))), is_wrapped_(fbo.has_value() || external_handle.has_value()), wrapped_fbo_(fbo) { // Ensure the texture descriptor itself is valid. diff --git a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h index 84deb194403..95893e8428a 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h @@ -75,7 +75,9 @@ class TextureGLES final : public Texture, std::shared_ptr reactor, TextureDescriptor desc); - TextureGLES(std::shared_ptr reactor, TextureDescriptor desc); + TextureGLES(std::shared_ptr reactor, + TextureDescriptor desc, + bool threadsafe = false); // |Texture| ~TextureGLES() override; @@ -164,6 +166,7 @@ class TextureGLES final : public Texture, TextureGLES(std::shared_ptr reactor, TextureDescriptor desc, + bool threadsafe, std::optional fbo, std::optional external_handle); 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 4764c00b419..91a47484a95 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.h @@ -73,8 +73,8 @@ class AllocatorMTL final : public Allocator { const DeviceBufferDescriptor& desc) override; // |Allocator| - std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override; + std::shared_ptr OnCreateTexture(const TextureDescriptor& desc, + bool threadsafe) override; // |Allocator| uint16_t MinimumBytesPerRow(PixelFormat format) const override; 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 42de0b17ccc..0b4f846da20 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/allocator_mtl.mm @@ -159,7 +159,8 @@ std::shared_ptr AllocatorMTL::OnCreateBuffer( } std::shared_ptr AllocatorMTL::OnCreateTexture( - const TextureDescriptor& desc) { + const TextureDescriptor& desc, + bool threadsafe) { if (!IsValid()) { return nullptr; } diff --git a/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h index 8d6a6bdcddc..197cf8d43c8 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h @@ -34,7 +34,8 @@ class PipelineLibraryMTL final : public PipelineLibrary { // |PipelineLibrary| PipelineFuture GetPipeline(PipelineDescriptor descriptor, - bool async) override; + bool async, + bool threadsafe) override; // |PipelineLibrary| PipelineFuture GetPipeline( diff --git a/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm index 1e69aeb0a23..270fbe5653d 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm @@ -113,7 +113,8 @@ bool PipelineLibraryMTL::IsValid() const { // |PipelineLibrary| PipelineFuture PipelineLibraryMTL::GetPipeline( PipelineDescriptor descriptor, - bool async) { + bool async, + bool threadsafe) { if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { return found->second; } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc index 9bd546340a8..52e7c495051 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -468,7 +468,8 @@ class AllocatedTextureSourceVK final : public TextureSourceVK { // |Allocator| std::shared_ptr AllocatorVK::OnCreateTexture( - const TextureDescriptor& desc) { + const TextureDescriptor& desc, + bool threadsafe) { if (!IsValid()) { return nullptr; } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h index 53e2772e740..6a2a35cbb7a 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/allocator_vk.h @@ -68,8 +68,8 @@ class AllocatorVK final : public Allocator { const DeviceBufferDescriptor& desc) override; // |Allocator| - std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override; + std::shared_ptr OnCreateTexture(const TextureDescriptor& desc, + bool threadsafe) override; // |Allocator| ISize GetMaxTextureSizeSupported() const override; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc index 109b4064675..02c4e6087a4 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.cc @@ -152,7 +152,8 @@ std::unique_ptr PipelineLibraryVK::CreateComputePipeline( // |PipelineLibrary| PipelineFuture PipelineLibraryVK::GetPipeline( PipelineDescriptor descriptor, - bool async) { + bool async, + bool threadsafe) { Lock lock(pipelines_mutex_); if (auto found = pipelines_.find(descriptor); found != pipelines_.end()) { return found->second; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.h index 2c2eac667b9..407b871dd08 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_library_vk.h @@ -59,8 +59,10 @@ class PipelineLibraryVK final bool IsValid() const override; // |PipelineLibrary| - PipelineFuture GetPipeline(PipelineDescriptor descriptor, - bool async) override; + PipelineFuture GetPipeline( + PipelineDescriptor descriptor, + bool async, + bool threadsafe = false) override; // |PipelineLibrary| PipelineFuture GetPipeline( diff --git a/engine/src/flutter/impeller/renderer/pipeline_library.h b/engine/src/flutter/impeller/renderer/pipeline_library.h index 4c5a8617d0b..472f226ba18 100644 --- a/engine/src/flutter/impeller/renderer/pipeline_library.h +++ b/engine/src/flutter/impeller/renderer/pipeline_library.h @@ -41,9 +41,29 @@ class PipelineLibrary : public std::enable_shared_from_this { virtual bool IsValid() const = 0; + //------------------------------------------------------------------------------ + /// @brief Creates a pipeline. + /// + /// @param[in] descriptor The descriptor of the texture to create. + /// @param[in] async Whether to allow pipeline creation to be deferred. + /// If `false`, pipeline creation will block on the + /// current thread. + /// + /// @param[in] threadsafe Whether mutations to this texture should be + /// protected with a threadsafe barrier. + /// + /// This parameter only affects the OpenGLES rendering + /// backend. + /// + /// If any interaction with this texture (including + /// creation) will be done on a thread other than + /// where the OpenGLES context resides, then + /// `threadsafe`, must be set to `true`. + /// virtual PipelineFuture GetPipeline( PipelineDescriptor descriptor, - bool async = true) = 0; + bool async = true, + bool threadsafe = false) = 0; virtual PipelineFuture GetPipeline( ComputePipelineDescriptor descriptor, diff --git a/engine/src/flutter/impeller/renderer/testing/mocks.h b/engine/src/flutter/impeller/renderer/testing/mocks.h index 0ce1717b9a5..82d9f8a8ad1 100644 --- a/engine/src/flutter/impeller/renderer/testing/mocks.h +++ b/engine/src/flutter/impeller/renderer/testing/mocks.h @@ -53,7 +53,7 @@ class MockAllocator : public Allocator { (override)); MOCK_METHOD(std::shared_ptr, OnCreateTexture, - (const TextureDescriptor& desc), + (const TextureDescriptor& desc, bool threadsafe), (override)); }; diff --git a/engine/src/flutter/lib/gpu/command_buffer.cc b/engine/src/flutter/lib/gpu/command_buffer.cc index 76ca1ed4878..2fc212f33ba 100644 --- a/engine/src/flutter/lib/gpu/command_buffer.cc +++ b/engine/src/flutter/lib/gpu/command_buffer.cc @@ -39,19 +39,20 @@ bool CommandBuffer::Submit() { bool CommandBuffer::Submit( const impeller::CommandBuffer::CompletionCallback& completion_callback) { - for (auto& encodable : encodables_) { - encodable->EncodeCommands(); - } - // For the GLES backend, command queue submission just flushes the reactor, // which needs to happen on the raster thread. if (context_->GetBackendType() == impeller::Context::BackendType::kOpenGLES) { auto dart_state = flutter::UIDartState::Current(); auto& task_runners = dart_state->GetTaskRunners(); - task_runners.GetRasterTaskRunner()->PostTask(fml::MakeCopyable( - [context = context_, command_buffer = command_buffer_, - completion_callback = completion_callback]() mutable { + task_runners.GetRasterTaskRunner()->PostTask( + fml::MakeCopyable([context = context_, command_buffer = command_buffer_, + completion_callback = completion_callback, + encodables = encodables_]() mutable { + for (auto& encodable : encodables) { + encodable->EncodeCommands(); + } + context->GetCommandQueue() ->Submit({command_buffer}, completion_callback) .ok(); @@ -59,6 +60,10 @@ bool CommandBuffer::Submit( return true; } + for (auto& encodable : encodables_) { + encodable->EncodeCommands(); + } + return context_->GetCommandQueue() ->Submit({command_buffer_}, completion_callback) .ok(); @@ -75,8 +80,8 @@ bool InternalFlutterGpu_CommandBuffer_Initialize( Dart_Handle wrapper, flutter::gpu::Context* contextWrapper) { auto res = fml::MakeRefCounted( - contextWrapper->GetContext(), - contextWrapper->GetContext()->CreateCommandBuffer()); + contextWrapper->GetContextShared(), + contextWrapper->GetContext().CreateCommandBuffer()); res->AssociateWithDartWrapper(wrapper); return true; diff --git a/engine/src/flutter/lib/gpu/context.cc b/engine/src/flutter/lib/gpu/context.cc index 1b8606dfa2d..36a715608a6 100644 --- a/engine/src/flutter/lib/gpu/context.cc +++ b/engine/src/flutter/lib/gpu/context.cc @@ -74,7 +74,11 @@ Context::Context(std::shared_ptr context) Context::~Context() = default; -std::shared_ptr Context::GetContext() { +impeller::Context& Context::GetContext() { + return *context_; +} + +std::shared_ptr& Context::GetContextShared() { return context_; } @@ -101,29 +105,27 @@ Dart_Handle InternalFlutterGpu_Context_InitializeDefault(Dart_Handle wrapper) { extern int InternalFlutterGpu_Context_GetDefaultColorFormat( flutter::gpu::Context* wrapper) { return static_cast(flutter::gpu::FromImpellerPixelFormat( - wrapper->GetContext()->GetCapabilities()->GetDefaultColorFormat())); + wrapper->GetContext().GetCapabilities()->GetDefaultColorFormat())); } extern int InternalFlutterGpu_Context_GetDefaultStencilFormat( flutter::gpu::Context* wrapper) { return static_cast(flutter::gpu::FromImpellerPixelFormat( - wrapper->GetContext()->GetCapabilities()->GetDefaultStencilFormat())); + wrapper->GetContext().GetCapabilities()->GetDefaultStencilFormat())); } extern int InternalFlutterGpu_Context_GetDefaultDepthStencilFormat( flutter::gpu::Context* wrapper) { return static_cast(flutter::gpu::FromImpellerPixelFormat( - wrapper->GetContext() - ->GetCapabilities() - ->GetDefaultDepthStencilFormat())); + wrapper->GetContext().GetCapabilities()->GetDefaultDepthStencilFormat())); } extern int InternalFlutterGpu_Context_GetMinimumUniformByteAlignment( flutter::gpu::Context* wrapper) { - return wrapper->GetContext()->GetCapabilities()->GetMinimumUniformAlignment(); + return wrapper->GetContext().GetCapabilities()->GetMinimumUniformAlignment(); } extern bool InternalFlutterGpu_Context_GetSupportsOffscreenMSAA( flutter::gpu::Context* wrapper) { - return flutter::gpu::SupportsNormalOffscreenMSAA(*wrapper->GetContext()); + return flutter::gpu::SupportsNormalOffscreenMSAA(wrapper->GetContext()); } diff --git a/engine/src/flutter/lib/gpu/context.h b/engine/src/flutter/lib/gpu/context.h index 4dbabcb5e43..3c915827cc9 100644 --- a/engine/src/flutter/lib/gpu/context.h +++ b/engine/src/flutter/lib/gpu/context.h @@ -30,7 +30,9 @@ class Context : public RefCountedDartWrappable { explicit Context(std::shared_ptr context); ~Context() override; - std::shared_ptr GetContext(); + impeller::Context& GetContext(); + + std::shared_ptr& GetContextShared(); private: /// An Impeller context that takes precedent over the IO state context when diff --git a/engine/src/flutter/lib/gpu/device_buffer.cc b/engine/src/flutter/lib/gpu/device_buffer.cc index b5232c28566..1304d9b2692 100644 --- a/engine/src/flutter/lib/gpu/device_buffer.cc +++ b/engine/src/flutter/lib/gpu/device_buffer.cc @@ -56,7 +56,7 @@ bool InternalFlutterGpu_DeviceBuffer_Initialize( desc.storage_mode = flutter::gpu::ToImpellerStorageMode(storage_mode); desc.size = size_in_bytes; auto device_buffer = - gpu_context->GetContext()->GetResourceAllocator()->CreateBuffer(desc); + gpu_context->GetContext().GetResourceAllocator()->CreateBuffer(desc); if (!device_buffer) { FML_LOG(ERROR) << "Failed to create device buffer."; return false; @@ -82,7 +82,7 @@ bool InternalFlutterGpu_DeviceBuffer_InitializeWithHostData( auto mapping = fml::NonOwnedMapping(reinterpret_cast(data.data()), data.length_in_bytes()); device_buffer = - gpu_context->GetContext()->GetResourceAllocator()->CreateBufferWithCopy( + gpu_context->GetContext().GetResourceAllocator()->CreateBufferWithCopy( mapping); } if (!device_buffer) { diff --git a/engine/src/flutter/lib/gpu/lib/src/texture.dart b/engine/src/flutter/lib/gpu/lib/src/texture.dart index 4c266e37c88..bc88c92d09c 100644 --- a/engine/src/flutter/lib/gpu/lib/src/texture.dart +++ b/engine/src/flutter/lib/gpu/lib/src/texture.dart @@ -24,7 +24,8 @@ base class Texture extends NativeFieldWrapperClass1 { this.enableRenderTargetUsage, this.enableShaderReadUsage, this.enableShaderWriteUsage, - ) : _coordinateSystem = coordinateSystem { + ) : _gpuContext = gpuContext, + _coordinateSystem = coordinateSystem { if (sampleCount != 1 && sampleCount != 4) { throw Exception("Only a sample count of 1 or 4 is currently supported"); } @@ -42,6 +43,8 @@ base class Texture extends NativeFieldWrapperClass1 { ); } + GpuContext _gpuContext; + final StorageMode storageMode; final PixelFormat format; final int width; @@ -99,7 +102,7 @@ base class Texture extends NativeFieldWrapperClass1 { 'The length of sourceBytes (bytes: ${sourceBytes.lengthInBytes}) must exactly match the size of the base mip level (bytes: ${baseMipSize})', ); } - bool success = _overwrite(sourceBytes); + bool success = _overwrite(_gpuContext, sourceBytes); if (!success) { throw Exception("Texture overwrite failed"); } @@ -153,10 +156,10 @@ base class Texture extends NativeFieldWrapperClass1 { ) external int _bytesPerTexel(); - @Native, Handle)>( + @Native, Pointer, Handle)>( symbol: 'InternalFlutterGpu_Texture_Overwrite', ) - external bool _overwrite(ByteData bytes); + external bool _overwrite(GpuContext gpuContext, ByteData bytes); @Native)>( symbol: 'InternalFlutterGpu_Texture_AsImage', diff --git a/engine/src/flutter/lib/gpu/render_pass.cc b/engine/src/flutter/lib/gpu/render_pass.cc index 71b8659bd25..03112143531 100644 --- a/engine/src/flutter/lib/gpu/render_pass.cc +++ b/engine/src/flutter/lib/gpu/render_pass.cc @@ -166,8 +166,9 @@ RenderPass::GetOrCreatePipeline() { dart_state->GetTaskRunners().GetRasterTaskRunner(), fml::MakeCopyable([promise = std::move(pipeline_promise), context = GetContext(), pipeline_desc]() mutable { - promise.set_value( - context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get()); + promise.set_value(context->GetPipelineLibrary() + ->GetPipeline(pipeline_desc, true, true) + .Get()); })); pipeline = pipeline_future.get(); } else { @@ -269,7 +270,7 @@ Dart_Handle InternalFlutterGpu_RenderPass_SetColorAttachment( // If the backend doesn't support normal MSAA, gracefully fallback to // rendering without MSAA. - if (!flutter::gpu::SupportsNormalOffscreenMSAA(*context->GetContext())) { + if (!flutter::gpu::SupportsNormalOffscreenMSAA(context->GetContext())) { desc.texture = desc.resolve_texture; desc.resolve_texture = nullptr; desc.store_action = impeller::StoreAction::kStore; diff --git a/engine/src/flutter/lib/gpu/shader.cc b/engine/src/flutter/lib/gpu/shader.cc index 26378f5e95c..d0b45da23bf 100644 --- a/engine/src/flutter/lib/gpu/shader.cc +++ b/engine/src/flutter/lib/gpu/shader.cc @@ -62,7 +62,7 @@ std::shared_ptr Shader::GetFunctionFromLibrary( } bool Shader::IsRegistered(Context& context) { - auto& lib = *context.GetContext()->GetShaderLibrary(); + auto& lib = *context.GetContext().GetShaderLibrary(); return GetFunctionFromLibrary(lib) != nullptr; } @@ -71,7 +71,7 @@ bool Shader::RegisterSync(Context& context) { return true; // Already registered. } - auto& lib = *context.GetContext()->GetShaderLibrary(); + auto& lib = *context.GetContext().GetShaderLibrary(); std::promise promise; auto future = promise.get_future(); diff --git a/engine/src/flutter/lib/gpu/texture.cc b/engine/src/flutter/lib/gpu/texture.cc index 740e3dc51ae..421aa55f205 100644 --- a/engine/src/flutter/lib/gpu/texture.cc +++ b/engine/src/flutter/lib/gpu/texture.cc @@ -6,6 +6,7 @@ #include "flutter/lib/gpu/formats.h" #include "flutter/lib/ui/painting/image.h" +#include "fml/make_copyable.h" #include "fml/mapping.h" #include "impeller/core/allocator.h" #include "impeller/core/formats.h" @@ -32,15 +33,33 @@ void Texture::SetCoordinateSystem( texture_->SetCoordinateSystem(coordinate_system); } -bool Texture::Overwrite(const tonic::DartByteData& source_bytes) { +bool Texture::Overwrite(Context& gpu_context, + const tonic::DartByteData& source_bytes) { const uint8_t* data = static_cast(source_bytes.data()); auto copy = std::vector(data, data + source_bytes.length_in_bytes()); // Texture::SetContents is a bit funky right now. It takes a shared_ptr of a // mapping and we're forced to copy here. auto mapping = std::make_shared(copy); + + // For the GLES backend, command queue submission just flushes the reactor, + // which needs to happen on the raster thread. + if (gpu_context.GetContext().GetBackendType() == + impeller::Context::BackendType::kOpenGLES) { + auto dart_state = flutter::UIDartState::Current(); + auto& task_runners = dart_state->GetTaskRunners(); + + task_runners.GetRasterTaskRunner()->PostTask( + fml::MakeCopyable([texture = texture_, mapping = mapping]() mutable { + if (!texture->SetContents(mapping)) { + FML_LOG(ERROR) << "Failed to set texture contents."; + } + })); + } + if (!texture_->SetContents(mapping)) { return false; } + gpu_context.GetContext().DisposeThreadLocalCachedResources(); return true; } @@ -109,7 +128,8 @@ bool InternalFlutterGpu_Texture_Initialize(Dart_Handle wrapper, return false; } auto texture = - gpu_context->GetContext()->GetResourceAllocator()->CreateTexture(desc); + gpu_context->GetContext().GetResourceAllocator()->CreateTexture(desc, + true); if (!texture) { FML_LOG(ERROR) << "Failed to create texture."; return false; @@ -132,8 +152,10 @@ void InternalFlutterGpu_Texture_SetCoordinateSystem( } bool InternalFlutterGpu_Texture_Overwrite(flutter::gpu::Texture* texture, + flutter::gpu::Context* gpu_context, Dart_Handle source_byte_data) { - return texture->Overwrite(tonic::DartByteData(source_byte_data)); + return texture->Overwrite(*gpu_context, + tonic::DartByteData(source_byte_data)); } extern int InternalFlutterGpu_Texture_BytesPerTexel( diff --git a/engine/src/flutter/lib/gpu/texture.h b/engine/src/flutter/lib/gpu/texture.h index a08619b0b68..04ddb575c30 100644 --- a/engine/src/flutter/lib/gpu/texture.h +++ b/engine/src/flutter/lib/gpu/texture.h @@ -27,7 +27,7 @@ class Texture : public RefCountedDartWrappable { void SetCoordinateSystem(impeller::TextureCoordinateSystem coordinate_system); - bool Overwrite(const tonic::DartByteData& source_bytes); + bool Overwrite(Context& gpu_context, const tonic::DartByteData& source_bytes); size_t GetBytesPerTexel(); @@ -68,8 +68,10 @@ extern void InternalFlutterGpu_Texture_SetCoordinateSystem( int coordinate_system); FLUTTER_GPU_EXPORT -extern bool InternalFlutterGpu_Texture_Overwrite(flutter::gpu::Texture* wrapper, - Dart_Handle source_byte_data); +extern bool InternalFlutterGpu_Texture_Overwrite( + flutter::gpu::Texture* wrapper, + flutter::gpu::Context* gpu_context, + Dart_Handle source_byte_data); FLUTTER_GPU_EXPORT extern int InternalFlutterGpu_Texture_BytesPerTexel( diff --git a/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.h b/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.h index 63388732607..4290b9808ed 100644 --- a/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.h +++ b/engine/src/flutter/lib/ui/painting/image_decoder_no_gl_unittests.h @@ -81,8 +81,8 @@ class TestImpellerAllocator : public impeller::Allocator { return std::make_shared(desc); } - std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override { + std::shared_ptr OnCreateTexture(const TextureDescriptor& desc, + bool threadsafe) override { return std::make_shared(desc); } };