moved device buffer to an untracked handle (flutter#57021)

fixes: flutter/flutter#159745

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

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

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",

impeller/renderer/backend/gles/device_buffer_gles.cc

@@ -17,14 +17,12 @@ DeviceBufferGLES::DeviceBufferGLES(DeviceBufferDescriptor desc,
                                    std::shared_ptr<Allocation> 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<GLuint> 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> 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;
 }

impeller/renderer/backend/gles/device_buffer_gles.h

@@ -45,7 +45,9 @@ class DeviceBufferGLES final
 
  private:
   std::shared_ptr<ReactorGLES> reactor_;
-  HandleGLES handle_;
+  std::optional<std::string> label_;
+  // Mutable for lazy evaluation.
+  mutable std::optional<HandleGLES> handle_;
   mutable std::shared_ptr<Allocation> backing_store_;
   mutable std::optional<Range> dirty_range_ = std::nullopt;
   mutable bool initialized_ = false;

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<MockGLESImpl>();
+
+  EXPECT_CALL(*mock_gles_impl, GenBuffers(1, _)).Times(1);
+
+  std::shared_ptr<MockGLES> mock_gled =
+      MockGLES::Init(std::move(mock_gles_impl));
+  ProcTableGLES::Resolver resolver = kMockResolverGLES;
+  auto proc_table = std::make_unique<ProcTableGLES>(resolver);
+  auto worker = std::make_shared<TestWorker>();
+  auto reactor = std::make_shared<ReactorGLES>(std::move(proc_table));
+  reactor->AddWorker(worker);
+
+  std::shared_ptr<Allocation> backing_store = std::make_shared<Allocation>();
+  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

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<decltype(mockGenTextures),  //
+                                 decltype(glGenTextures)>::value);
+
+void mockGenBuffers(GLsizei n, GLuint* buffers) {
+  CallMockMethod(&IMockGLESImpl::GenBuffers, n, buffers);
+}
+
 static_assert(CheckSameSignature<decltype(mockGenTextures),  //
                                  decltype(glGenTextures)>::value);
 
@@ -248,6 +255,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
     return reinterpret_cast<void*>(mockGenTextures);
   } else if (strcmp(name, "glObjectLabelKHR") == 0) {
     return reinterpret_cast<void*>(mockObjectLabelKHR);
+  } else if (strcmp(name, "glGenBuffers") == 0) {
+    return reinterpret_cast<void*>(mockGenBuffers);
   } else {
     return reinterpret_cast<void*>(&doNothing);
   }

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.