Skip to content

Commit 8c736f4

Browse files
authored
Implements the naming of untracked gles handles (flutter/engine#56948)
issue: flutter#159745 flutter/engine#56927 introduced untracked handles, but naming them didn't work. This adds a test to make sure they work. I kept naming thread-safe since it isn't happening often anyways. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 9e25a25 commit 8c736f4

File tree

5 files changed

+48
-7
lines changed

5 files changed

+48
-7
lines changed

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ static bool ResourceIsLive(const ProcTableGLES& gl,
360360

361361
bool ProcTableGLES::SetDebugLabel(DebugResourceType type,
362362
GLint name,
363-
const std::string& label) const {
363+
std::string_view label) const {
364364
if (debug_label_max_length_ <= 0) {
365365
return true;
366366
}

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class ProcTableGLES {
311311

312312
bool SetDebugLabel(DebugResourceType type,
313313
GLint name,
314-
const std::string& label) const;
314+
std::string_view label) const;
315315

316316
void PushDebugGroup(const std::string& string) const;
317317

engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,15 +386,23 @@ void ReactorGLES::SetupDebugGroups() {
386386

387387
void ReactorGLES::SetDebugLabel(const HandleGLES& handle,
388388
std::string_view label) {
389+
FML_DCHECK(handle.GetType() != HandleType::kFence);
389390
if (!can_set_debug_labels_) {
390391
return;
391392
}
392393
if (handle.IsDead()) {
393394
return;
394395
}
395-
WriterLock handles_lock(handles_mutex_);
396-
if (auto found = handles_.find(handle); found != handles_.end()) {
397-
found->second.pending_debug_label = label;
396+
if (handle.untracked_id_.has_value()) {
397+
FML_DCHECK(CanReactOnCurrentThread());
398+
const auto& gl = GetProcTable();
399+
gl.SetDebugLabel(ToDebugResourceType(handle.GetType()),
400+
handle.untracked_id_.value(), label);
401+
} else {
402+
WriterLock handles_lock(handles_mutex_);
403+
if (auto found = handles_.find(handle); found != handles_.end()) {
404+
found->second.pending_debug_label = label;
405+
}
398406
}
399407
}
400408

engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ void mockGetIntegerv(GLenum name, int* value) {
8383
case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
8484
*value = 8;
8585
break;
86+
case GL_MAX_LABEL_LENGTH_KHR:
87+
*value = 64;
88+
break;
8689
default:
8790
*value = 0;
8891
break;
@@ -170,6 +173,8 @@ static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT), //
170173
void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) {
171174
RecordGLCall("glUniform1fv");
172175
}
176+
static_assert(CheckSameSignature<decltype(mockUniform1fv), //
177+
decltype(glUniform1fv)>::value);
173178

174179
void mockGenTextures(GLsizei n, GLuint* textures) {
175180
RecordGLCall("glGenTextures");
@@ -182,8 +187,17 @@ void mockGenTextures(GLsizei n, GLuint* textures) {
182187
}
183188
}
184189

185-
static_assert(CheckSameSignature<decltype(mockUniform1fv), //
186-
decltype(glUniform1fv)>::value);
190+
static_assert(CheckSameSignature<decltype(mockGenTextures), //
191+
decltype(glGenTextures)>::value);
192+
193+
void mockObjectLabelKHR(GLenum identifier,
194+
GLuint name,
195+
GLsizei length,
196+
const GLchar* label) {
197+
RecordGLCall("glObjectLabelKHR");
198+
}
199+
static_assert(CheckSameSignature<decltype(mockObjectLabelKHR), //
200+
decltype(glObjectLabelKHR)>::value);
187201

188202
std::shared_ptr<MockGLES> MockGLES::Init(
189203
const std::optional<std::vector<const unsigned char*>>& extensions,
@@ -230,6 +244,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) {
230244
return reinterpret_cast<void*>(mockUniform1fv);
231245
} else if (strcmp(name, "glGenTextures") == 0) {
232246
return reinterpret_cast<void*>(mockGenTextures);
247+
} else if (strcmp(name, "glObjectLabelKHR") == 0) {
248+
return reinterpret_cast<void*>(mockObjectLabelKHR);
233249
} else {
234250
return reinterpret_cast<void*>(&doNothing);
235251
}

engine/src/flutter/impeller/renderer/backend/gles/test/reactor_unittests.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,23 @@ TEST(ReactorGLES, UntrackedHandle) {
9191
calls.end());
9292
}
9393

94+
TEST(ReactorGLES, NameUntrackedHandle) {
95+
std::shared_ptr<MockGLES> mock_gles = MockGLES::Init();
96+
ProcTableGLES::Resolver resolver = kMockResolverGLES;
97+
auto proc_table = std::make_unique<ProcTableGLES>(resolver);
98+
auto worker = std::make_shared<TestWorker>();
99+
auto reactor = std::make_shared<ReactorGLES>(std::move(proc_table));
100+
reactor->AddWorker(worker);
101+
102+
mock_gles->SetNextTexture(1234u);
103+
HandleGLES handle = reactor->CreateUntrackedHandle(HandleType::kTexture);
104+
mock_gles->GetCapturedCalls();
105+
reactor->SetDebugLabel(handle, "hello, joe!");
106+
std::vector<std::string> calls = mock_gles->GetCapturedCalls();
107+
EXPECT_TRUE(std::find(calls.begin(), calls.end(), "glObjectLabelKHR") !=
108+
calls.end());
109+
}
110+
94111
TEST(ReactorGLES, PerThreadOperationQueues) {
95112
auto mock_gles = MockGLES::Init();
96113
ProcTableGLES::Resolver resolver = kMockResolverGLES;

0 commit comments

Comments
 (0)