diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index dc786d51f0486..45e6771274782 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -183,6 +183,7 @@ ../../../flutter/impeller/golden_tests/README.md ../../../flutter/impeller/playground ../../../flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc +../../../flutter/impeller/renderer/backend/gles/device_buffer_gles_unittests.cc ../../../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 diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn index 4c8598a449eb4..a28309f5fa828 100644 --- a/impeller/renderer/backend/gles/BUILD.gn +++ b/impeller/renderer/backend/gles/BUILD.gn @@ -15,6 +15,7 @@ impeller_component("gles_unittests") { testonly = true sources = [ "buffer_bindings_gles_unittests.cc", + "device_buffer_gles_unittests.cc", "test/capabilities_unittests.cc", "test/formats_gles_unittests.cc", "test/gpu_tracer_gles_unittests.cc", diff --git a/impeller/renderer/backend/gles/device_buffer_gles.cc b/impeller/renderer/backend/gles/device_buffer_gles.cc index b072a8ebc0a63..0a7478d8f7e95 100644 --- a/impeller/renderer/backend/gles/device_buffer_gles.cc +++ b/impeller/renderer/backend/gles/device_buffer_gles.cc @@ -17,14 +17,12 @@ DeviceBufferGLES::DeviceBufferGLES(DeviceBufferDescriptor desc, std::shared_ptr backing_store) : DeviceBuffer(desc), reactor_(std::move(reactor)), - handle_(reactor_ ? reactor_->CreateHandle(HandleType::kBuffer) - : HandleGLES::DeadHandle()), backing_store_(std::move(backing_store)) {} // |DeviceBuffer| DeviceBufferGLES::~DeviceBufferGLES() { - if (!handle_.IsDead()) { - reactor_->CollectHandle(handle_); + if (handle_.has_value() && !handle_->IsDead()) { + reactor_->CollectHandle(*handle_); } } @@ -57,7 +55,11 @@ bool DeviceBufferGLES::OnCopyHostBuffer(const uint8_t* source, } std::optional DeviceBufferGLES::GetHandle() const { - return reactor_->GetGLHandle(handle_); + if (handle_.has_value()) { + return reactor_->GetGLHandle(*handle_); + } else { + return std::nullopt; + } } void DeviceBufferGLES::Flush(std::optional range) const { @@ -90,7 +92,16 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const { return false; } - auto buffer = reactor_->GetGLHandle(handle_); + if (!handle_.has_value()) { + handle_ = reactor_->CreateUntrackedHandle(HandleType::kBuffer); +#ifdef IMPELLER_DEBUG + if (handle_.has_value() && label_.has_value()) { + reactor_->SetDebugLabel(*handle_, *label_); + } +#endif + } + + auto buffer = reactor_->GetGLHandle(*handle_); if (!buffer.has_value()) { return false; } @@ -118,7 +129,10 @@ bool DeviceBufferGLES::BindAndUploadDataIfNecessary(BindingType type) const { // |DeviceBuffer| bool DeviceBufferGLES::SetLabel(std::string_view label) { #ifdef IMPELLER_DEBUG - reactor_->SetDebugLabel(handle_, label); + label_ = label; + if (handle_.has_value()) { + reactor_->SetDebugLabel(*handle_, label); + } #endif // IMPELLER_DEBUG return true; } diff --git a/impeller/renderer/backend/gles/device_buffer_gles.h b/impeller/renderer/backend/gles/device_buffer_gles.h index 8255995f9d83a..f080d4e3e0ffe 100644 --- a/impeller/renderer/backend/gles/device_buffer_gles.h +++ b/impeller/renderer/backend/gles/device_buffer_gles.h @@ -45,7 +45,9 @@ class DeviceBufferGLES final private: std::shared_ptr reactor_; - HandleGLES handle_; + std::optional label_; + // Mutable for lazy evaluation. + mutable std::optional handle_; mutable std::shared_ptr backing_store_; mutable std::optional dirty_range_ = std::nullopt; mutable bool initialized_ = false; diff --git a/impeller/renderer/backend/gles/device_buffer_gles_unittests.cc b/impeller/renderer/backend/gles/device_buffer_gles_unittests.cc new file mode 100644 index 0000000000000..1fe910faab0d9 --- /dev/null +++ b/impeller/renderer/backend/gles/device_buffer_gles_unittests.cc @@ -0,0 +1,49 @@ +// 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" // IWYU pragma: keep +#include "gtest/gtest.h" +#include "impeller/renderer/backend/gles/device_buffer_gles.h" +#include "impeller/renderer/backend/gles/test/mock_gles.h" + +namespace impeller { +namespace testing { + +using ::testing::_; + +namespace { +class TestWorker : public ReactorGLES::Worker { + public: + bool CanReactorReactOnCurrentThreadNow( + const ReactorGLES& reactor) const override { + return true; + } +}; +} // namespace + +TEST(DeviceBufferGLESTest, BindUniformData) { + auto mock_gles_impl = std::make_unique(); + + EXPECT_CALL(*mock_gles_impl, GenBuffers(1, _)).Times(1); + + std::shared_ptr mock_gled = + MockGLES::Init(std::move(mock_gles_impl)); + ProcTableGLES::Resolver resolver = kMockResolverGLES; + auto proc_table = std::make_unique(resolver); + auto worker = std::make_shared(); + auto reactor = std::make_shared(std::move(proc_table)); + reactor->AddWorker(worker); + + std::shared_ptr backing_store = std::make_shared(); + ASSERT_TRUE(backing_store->Truncate(Bytes{sizeof(float)})); + DeviceBufferGLES device_buffer(DeviceBufferDescriptor{.size = sizeof(float)}, + reactor, backing_store); + EXPECT_FALSE(device_buffer.GetHandle().has_value()); + EXPECT_TRUE(device_buffer.BindAndUploadDataIfNecessary( + DeviceBufferGLES::BindingType::kUniformBuffer)); + EXPECT_TRUE(device_buffer.GetHandle().has_value()); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/renderer/backend/gles/test/mock_gles.cc b/impeller/renderer/backend/gles/test/mock_gles.cc index 5b890d989f37a..c62ff4fe71483 100644 --- a/impeller/renderer/backend/gles/test/mock_gles.cc +++ b/impeller/renderer/backend/gles/test/mock_gles.cc @@ -174,6 +174,13 @@ void mockGenTextures(GLsizei n, GLuint* textures) { CallMockMethod(&IMockGLESImpl::GenTextures, n, textures); } +static_assert(CheckSameSignature::value); + +void mockGenBuffers(GLsizei n, GLuint* buffers) { + CallMockMethod(&IMockGLESImpl::GenBuffers, n, buffers); +} + static_assert(CheckSameSignature::value); @@ -248,6 +255,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) { return reinterpret_cast(mockGenTextures); } else if (strcmp(name, "glObjectLabelKHR") == 0) { return reinterpret_cast(mockObjectLabelKHR); + } else if (strcmp(name, "glGenBuffers") == 0) { + return reinterpret_cast(mockGenBuffers); } else { return reinterpret_cast(&doNothing); } diff --git a/impeller/renderer/backend/gles/test/mock_gles.h b/impeller/renderer/backend/gles/test/mock_gles.h index 35b041b13d5e2..681b894ac410b 100644 --- a/impeller/renderer/backend/gles/test/mock_gles.h +++ b/impeller/renderer/backend/gles/test/mock_gles.h @@ -35,6 +35,7 @@ class IMockGLESImpl { GLenum target, GLuint64* result) {} virtual void DeleteQueriesEXT(GLsizei size, const GLuint* queries) {} + virtual void GenBuffers(GLsizei n, GLuint* buffers) {} }; class MockGLESImpl : public IMockGLESImpl { @@ -68,6 +69,7 @@ class MockGLESImpl : public IMockGLESImpl { DeleteQueriesEXT, (GLsizei size, const GLuint* queries), (override)); + MOCK_METHOD(void, GenBuffers, (GLsizei n, GLuint* buffers), (override)); }; /// @brief Provides a mocked version of the |ProcTableGLES| class.