From bebe534bad1f2e061c1b9065163b8d2018785ca2 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 11 Jul 2026 00:19:33 -0400 Subject: [PATCH] [Fix] (MG_Impl/GLImpl): stop double-recording GL errors for a bad program handle glBindFragDataLocation, glGetFragDataLocation and glGetFragDataIndex each recorded a redundant GL_INVALID_OPERATION on top of the error that TryToGetProgramObject already recorded (GL_INVALID_VALUE for an unknown name, GL_INVALID_OPERATION for a non-program object). One bad call thus queued two errors, so an app calling glGetError twice saw a spurious second error, and any following code that expects a clean error queue (e.g. a later test) picked up the stale one. Drop the second RecordError from all three call sites and rely on the single error TryToGetProgramObject already reports -- matching the clean `if (!programObject) return;` pattern the rest of GL_Program.cpp uses. The first, app-visible error is unchanged; only the redundant second is gone. ProgramTest's invalid-handle case now asserts exactly one error (mutation- verified: reintroducing the second record fails it) and keeps a defensive error-queue drain. ProgramTest 24/24. --- .../MG_Impl/GLImpl/Program/GL_Program.cpp | 26 ++++--------------- MobileGL/MG_Test/Program/ProgramTest.cpp | 6 +++-- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index 9fb0fcf5..c4b40dde 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -1485,13 +1485,9 @@ namespace MobileGL::MG_Impl::GLImpl { void BindFragDataLocation_State(GLuint program, GLuint colorNumber, const char* name) { auto& programObject = TryToGetProgramObject(program); - if (programObject == nullptr) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, - std::to_string(program) + " is not the name of a program object.")); - return; - } + // TryToGetProgramObject already recorded the error for a bad handle (GL_INVALID_VALUE for an + // unknown name, GL_INVALID_OPERATION for a non-program object); do not record a second one. + if (!programObject) return; if (name == nullptr) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, @@ -1520,13 +1516,7 @@ namespace MobileGL::MG_Impl::GLImpl { GLint GetFragDataLocation_State(GLuint program, const char* name) { auto& programObject = TryToGetProgramObject(program); - if (programObject == nullptr) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, - std::to_string(program) + " is not the name of a program object.")); - return -1; - } + if (!programObject) return -1; // TryToGetProgramObject already recorded the error. if (name == nullptr) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, @@ -1545,13 +1535,7 @@ namespace MobileGL::MG_Impl::GLImpl { GLint GetFragDataIndex_State(GLuint program, const char* name) { auto& programObject = TryToGetProgramObject(program); - if (programObject == nullptr) { - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, - std::to_string(program) + " is not the name of a program object.")); - return -1; - } + if (!programObject) return -1; // TryToGetProgramObject already recorded the error. if (name == nullptr) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index 8efc4950..15200bac 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -1564,8 +1564,10 @@ TEST_F(ProgramTest, GetFragDataIndexRejectsInvalidProgram) { // routes through the shared program-name check, which records GL_INVALID_VALUE for an unknown name. EXPECT_EQ(GetFragDataIndex(999999u, "fragColor"), -1); EXPECT_EQ(GetError(), GL_INVALID_VALUE); - // The name check and the entry point each queue an error for an unknown handle; drain the rest so - // no stale error leaks into a later test (the fixture does not reset the error queue). + // Exactly ONE error is recorded per bad call: the redundant second GL_INVALID_OPERATION that the + // FragData entry points used to queue on top of the name check has been removed. + EXPECT_EQ(GetError(), GL_NO_ERROR); + // Defensive drain: keep the shared error queue clean regardless (the fixture never resets it). while (GetError() != GL_NO_ERROR) {} }