diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 16626a9f..11eea70e 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -382,6 +382,31 @@ namespace MobileGL::MG_Impl::GLImpl { } } + // GL 4.6 core 10.3.9: every DrawElements-family count is a sizei and "if count is negative, an + // INVALID_VALUE error is generated". The same sentence covers instancecount and the + // MultiDraw* drawcount, so one helper serves all of them; the parameter is named for the + // caller so the message says which argument the application actually got wrong. + static Bool ValidateNonNegativeDrawArgument(const char* functionName, const char* argumentName, GLsizei value) { + if (value >= 0) return true; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", functionName, + String(argumentName) + " must be non-negative.")); + return false; + } + + // GL 4.6 core 10.3.9 for DrawRangeElements*: "if end < start, an INVALID_VALUE error is + // generated". Both are uints, so a caller that passes -1 for start arrives here as + // 0xFFFFFFFF and is caught by the same comparison - which is exactly what + // KHR-GL4x.draw_elements_base_vertex_tests.invalid_count_argument checks. + static Bool ValidateDrawElementsRange(const char* functionName, GLuint start, GLuint end) { + if (end >= start) return true; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", functionName, "end must not be less than start.")); + return false; + } + // GL 4.6 core 10.9: inside a conditional block whose predicate did not pass, the drawing // commands, Clear, ClearBuffer* and the compute dispatches are DISCARDED. The gate sits on the // wrappers that ISSUE the backend call rather than at the top of each entry point, so that @@ -836,6 +861,9 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; + if (!ValidateDrawElementsIndexType(__func__, type)) return; + if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; + if (!ValidateDrawElementsRange(__func__, start, end)) return; DrawRangeElementsBaseVertex_Backend(mode, start, end, count, type, indices, basevertex); } @@ -860,6 +888,9 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; + if (!ValidateDrawElementsIndexType(__func__, type)) return; + if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; + if (!ValidateNonNegativeDrawArgument(__func__, "instancecount", instancecount)) return; DrawElementsInstancedBaseVertex_Backend(mode, count, type, indices, instancecount, basevertex); } @@ -914,6 +945,8 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; + if (!ValidateDrawElementsIndexType(__func__, type)) return; + if (!ValidateNonNegativeDrawArgument(__func__, "count", count)) return; AccountTransformFeedbackPrimitives(mode, count); DrawElementsBaseVertex_Backend(mode, count, type, indices, basevertex); } @@ -952,6 +985,19 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidatePrimitiveModeEnum(__func__, mode)) return; if (!ValidateCurrentProgramForExecution(__func__)) return; if (!ValidatePrimitiveModeForBackend(__func__, mode)) return; + if (!ValidateDrawElementsIndexType(__func__, type)) return; + if (!ValidateNonNegativeDrawArgument(__func__, "drawcount", drawcount)) return; + // GL 4.6 core 10.5 defines MultiDrawElementsBaseVertex as drawcount separate + // DrawElementsBaseVertex calls, so each element of the count array carries the same + // non-negative requirement the single-draw entry point applies to its own count. The + // whole call is rejected before any sub-draw is issued, which is what makes the error + // observable at all - a driver that drew the valid prefix first would leave the + // framebuffer half-written. + if (count != nullptr) { + for (GLsizei draw = 0; draw < drawcount; ++draw) { + if (!ValidateNonNegativeDrawArgument(__func__, "every element of count", count[draw])) return; + } + } MultiDrawElementsBaseVertex_Backend(mode, count, type, indices, drawcount, basevertex); } diff --git a/MobileGL/MG_IntegrationTest/Scenarios/MultiDrawScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/MultiDrawScenario.cpp index 14bb5265..4c221aec 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/MultiDrawScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/MultiDrawScenario.cpp @@ -518,5 +518,62 @@ void main() { ExpectSameImage(batched, unrolled, "a batch with zero-count sub-draws"); } + // The base-vertex family's argument checks (GL 4.6 core 10.3.9). These are what + // KHR-GL4x.draw_elements_base_vertex_tests.invalid_* assert, and the reason the group sat + // NotSupported for so long hid the fact that the entry points forwarded any argument + // straight to the backend: a negative count reached the emulation as a huge unsigned + // size. Each case drains the error queue first so the assertion names the call it made. + TEST_F(MultiDrawScenario, BaseVertexDrawsRejectMalformedArguments) { + if (!Ready()) return; + constexpr int kPad = 0; + BuildScene(kPad, kQuadIndices, sizeof(kQuadIndices)); + // A bound program and VAO are prerequisites, not decoration: the entry points check + // "is there something to execute" (GL_INVALID_OPERATION) before they look at any + // argument, so without these every case below would pass for the wrong reason. + glUseProgram(m_program); + glBindVertexArray(m_vao); + ASSERT_EQ(FirstGLError(), GLenum(GL_NO_ERROR)) << "scene setup left a GL error behind"; + + const auto expectError = [&](const char* what, GLenum expected) { + EXPECT_EQ(FirstGLError(), expected) << what; + // FirstGLError stops at the first one; make sure nothing else is queued so the + // next case starts clean. + while (glGetError() != GL_NO_ERROR) { + } + }; + + glDrawElementsBaseVertex(GL_TRIANGLES, -1, GL_UNSIGNED_INT, nullptr, 0); + expectError("glDrawElementsBaseVertex with a negative count", GL_INVALID_VALUE); + + glDrawElementsBaseVertex(GL_TRIANGLES, 3, GL_NONE, nullptr, 0); + expectError("glDrawElementsBaseVertex with a non-index type", GL_INVALID_ENUM); + + glDrawRangeElementsBaseVertex(GL_TRIANGLES, 3, 0, 3, GL_UNSIGNED_INT, nullptr, 0); + expectError("glDrawRangeElementsBaseVertex with end < start", GL_INVALID_VALUE); + + // start = -1 arrives as 0xFFFFFFFF, so this is the same end < start rule seen from + // the other side - and it is the shape the CTS's invalid_count case actually uses. + glDrawRangeElementsBaseVertex(GL_TRIANGLES, static_cast(-1), 2, 1, GL_UNSIGNED_INT, nullptr, 0); + expectError("glDrawRangeElementsBaseVertex with a wrapped start", GL_INVALID_VALUE); + + glDrawElementsInstancedBaseVertex(GL_TRIANGLES, 3, GL_UNSIGNED_INT, nullptr, -1, 0); + expectError("glDrawElementsInstancedBaseVertex with a negative instancecount", GL_INVALID_VALUE); + + const GLsizei negativeCount = -1; + const void* offsets[1] = {reinterpret_cast(0)}; + const GLint baseVertices[1] = {0}; + glMultiDrawElementsBaseVertex(GL_TRIANGLES, &negativeCount, GL_UNSIGNED_INT, offsets, 1, baseVertices); + expectError("glMultiDrawElementsBaseVertex with a negative element of count", GL_INVALID_VALUE); + + const GLsizei validCount = 6; + glMultiDrawElementsBaseVertex(GL_TRIANGLES, &validCount, GL_UNSIGNED_INT, offsets, -1, baseVertices); + expectError("glMultiDrawElementsBaseVertex with a negative drawcount", GL_INVALID_VALUE); + + // The well-formed call still has to go through, or the checks above would be + // indistinguishable from a blanket rejection. + glMultiDrawElementsBaseVertex(GL_TRIANGLES, &validCount, GL_UNSIGNED_INT, offsets, 1, baseVertices); + expectError("a well-formed glMultiDrawElementsBaseVertex", GL_NO_ERROR); + } + } // namespace } // namespace MGITest