Implements the naming of untracked gles handles

Author: gaaclarke

impeller/renderer/backend/gles/reactor_gles.cc

@@ -301,6 +301,7 @@ bool ReactorGLES::ConsolidateHandles() {
     WriterLock handles_lock(handles_mutex_);
     handles_to_delete.reserve(handles_to_collect_count_);
     handles_to_collect_count_ = 0;
+    std::swap(handles_to_name, handles_to_name_);
     for (auto& handle : handles_) {
       // Collect dead handles.
       if (handle.second.pending_collection) {
@@ -317,15 +318,6 @@ bool ReactorGLES::ConsolidateHandles() {
         }
         handle.second.name = gl_handle;
       }
-      // Set pending debug labels.
-      if (handle.second.pending_debug_label.has_value() &&
-          handle.first.GetType() != HandleType::kFence) {
-        handles_to_name.emplace_back(std::make_tuple(
-            ToDebugResourceType(handle.first.GetType()),
-            handle.second.name.value().handle,
-            std::move(handle.second.pending_debug_label.value())));
-        handle.second.pending_debug_label = std::nullopt;
-      }
     }
     for (const auto& handle_to_delete : handles_to_delete) {
       handles_.erase(std::get<0>(handle_to_delete));
@@ -386,15 +378,24 @@ void ReactorGLES::SetupDebugGroups() {
 
 void ReactorGLES::SetDebugLabel(const HandleGLES& handle,
                                 std::string_view label) {
+  FML_DCHECK(handle.GetType() != HandleType::kFence);
   if (!can_set_debug_labels_) {
     return;
   }
   if (handle.IsDead()) {
     return;
   }
   WriterLock handles_lock(handles_mutex_);
-  if (auto found = handles_.find(handle); found != handles_.end()) {
-    found->second.pending_debug_label = label;
+  if (handle.untracked_id_.has_value()) {
+    handles_to_name_.emplace_back(
+        std::make_tuple(ToDebugResourceType(handle.GetType()),
+                        handle.untracked_id_.value(), std::string(label)));
+  } else {
+    if (auto found = handles_.find(handle); found != handles_.end()) {
+      handles_to_name_.emplace_back(std::make_tuple(
+          ToDebugResourceType(handle.GetType()),
+          found->second.name.value().handle, std::string(label)));
+    }
   }
 }
 

impeller/renderer/backend/gles/reactor_gles.h

@@ -272,7 +272,6 @@ class ReactorGLES {
 
   struct LiveHandle {
     std::optional<GLStorage> name;
-    std::optional<std::string> pending_debug_label;
     bool pending_collection = false;
     fml::ScopedCleanupClosure callback = {};
 
@@ -296,6 +295,9 @@ class ReactorGLES {
   mutable RWMutex handles_mutex_;
   LiveHandles handles_ IPLR_GUARDED_BY(handles_mutex_);
   int32_t handles_to_collect_count_ IPLR_GUARDED_BY(handles_mutex_) = 0;
+  std::vector<
+      std::tuple<DebugResourceType, GLint, std::string>> handles_to_name_
+      IPLR_GUARDED_BY(handles_mutex_);
 
   mutable Mutex workers_mutex_;
   mutable std::map<WorkerID, std::weak_ptr<Worker>> workers_ IPLR_GUARDED_BY(

impeller/renderer/backend/gles/test/mock_gles.cc

@@ -83,6 +83,9 @@ void mockGetIntegerv(GLenum name, int* value) {
     case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
       *value = 8;
       break;
+    case GL_MAX_LABEL_LENGTH_KHR:
+      *value = 64;
+      break;
     default:
       *value = 0;
       break;
@@ -170,6 +173,8 @@ static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT),  //
 void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) {
   RecordGLCall("glUniform1fv");
 }
+static_assert(CheckSameSignature<decltype(mockUniform1fv),  //
+                                 decltype(glUniform1fv)>::value);
 
 void mockGenTextures(GLsizei n, GLuint* textures) {
   RecordGLCall("glGenTextures");
@@ -182,8 +187,17 @@ void mockGenTextures(GLsizei n, GLuint* textures) {
   }
 }
 
-static_assert(CheckSameSignature<decltype(mockUniform1fv),  //
-                                 decltype(glUniform1fv)>::value);
+static_assert(CheckSameSignature<decltype(mockGenTextures),  //
+                                 decltype(glGenTextures)>::value);
+
+void mockObjectLabelKHR(GLenum identifier,
+                        GLuint name,
+                        GLsizei length,
+                        const GLchar* label) {
+  RecordGLCall("glObjectLabelKHR");
+}
+static_assert(CheckSameSignature<decltype(mockObjectLabelKHR),  //
+                                 decltype(glObjectLabelKHR)>::value);
 
 std::shared_ptr<MockGLES> MockGLES::Init(
     const std::optional<std::vector<const unsigned char*>>& extensions,
@@ -230,6 +244,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
     return reinterpret_cast<void*>(mockUniform1fv);
   } else if (strcmp(name, "glGenTextures") == 0) {
     return reinterpret_cast<void*>(mockGenTextures);
+  } else if (strcmp(name, "glObjectLabelKHR") == 0) {
+    return reinterpret_cast<void*>(mockObjectLabelKHR);
   } else {
     return reinterpret_cast<void*>(&doNothing);
   }

impeller/renderer/backend/gles/test/reactor_unittests.cc

@@ -91,6 +91,26 @@ TEST(ReactorGLES, UntrackedHandle) {
                calls.end());
 }
 
+TEST(ReactorGLES, NameUntrackedHandle) {
+  std::shared_ptr<MockGLES> mock_gles = MockGLES::Init();
+  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);
+
+  mock_gles->SetNextTexture(1234u);
+  HandleGLES handle = reactor->CreateUntrackedHandle(HandleType::kTexture);
+  mock_gles->GetCapturedCalls();
+  reactor->SetDebugLabel(handle, "hello, joe!");
+  // Without an operation nothing is cleaned up.
+  EXPECT_TRUE(reactor->AddOperation([&](const ReactorGLES&) {}));
+  EXPECT_TRUE(reactor->React());
+  std::vector<std::string> calls = mock_gles->GetCapturedCalls();
+  EXPECT_TRUE(std::find(calls.begin(), calls.end(), "glObjectLabelKHR") !=
+              calls.end());
+}
+
 TEST(ReactorGLES, PerThreadOperationQueues) {
   auto mock_gles = MockGLES::Init();
   ProcTableGLES::Resolver resolver = kMockResolverGLES;