[Fix, Test] (MG_Impl): validate indirect-dispatch and indirect-count arguments before the backend-availability check

MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp

@@ -493,15 +493,12 @@ namespace MobileGL::MG_Impl::GLImpl {
     }
 
     void DispatchComputeIndirect(GLintptr indirect) {
-        auto dispatchComputeIndirect = MG_Backend::gBackendFunctionsTable.GL.DispatchComputeIndirect;
+        // Argument and binding validation runs FIRST. Both are properties of the call and of GL
-        if (!dispatchComputeIndirect) {
+        // state, so a context whose backend cannot dispatch at all must still report the
-            MG_State::pGLContext->RecordError(
+        // argument error the spec names rather than masking every one of them with
-                ErrorCode::InvalidOperation,
+        // "unsupported" - which is what put GL_INVALID_OPERATION where
-                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
+        // KHR-GL43.compute_shader.api-indirect expects GL_INVALID_VALUE.
-                                             "Backend does not support indirect compute dispatch."));
+        //
-            return;
-        }
-        if (!ValidateCurrentProgramForCompute(__func__)) return;
         // GL 4.6 core 19: `indirect` is a byte offset into GL_DISPATCH_INDIRECT_BUFFER -
         // negative or misaligned is INVALID_VALUE, nothing bound is INVALID_OPERATION.
         if (indirect < 0 || (indirect % 4) != 0) {
@@ -534,6 +531,15 @@ namespace MobileGL::MG_Impl::GLImpl {
                                 indirect, indirectBuffer->GetSize())));
             return;
         }
+        auto dispatchComputeIndirect = MG_Backend::gBackendFunctionsTable.GL.DispatchComputeIndirect;
+        if (!dispatchComputeIndirect) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidOperation,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
+                                             "Backend does not support indirect compute dispatch."));
+            return;
+        }
+        if (!ValidateCurrentProgramForCompute(__func__)) return;
         dispatchComputeIndirect(indirect);
     }
 
@@ -651,6 +657,12 @@ namespace MobileGL::MG_Impl::GLImpl {
 
     void MultiDrawElementsIndirectCount(GLenum mode, GLenum type, const void* indirect, GLintptr drawcount,
                                         GLsizei maxdrawcount, GLsizei stride) {
+        // Argument validation before the backend-availability check: see DispatchComputeIndirect.
+        // DrawElementsIndirectCommand: count, instanceCount, firstIndex, baseVertex, baseInstance.
+        if (!ValidateIndirectCountDraw(reinterpret_cast<GLintptr>(indirect), drawcount, maxdrawcount, stride,
+                                       5 * sizeof(Uint32), __func__)) {
+            return;
+        }
         auto multiDrawElementsIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawElementsIndirectCount;
         if (!multiDrawElementsIndirectCount) {
             MG_State::pGLContext->RecordError(
@@ -659,16 +671,17 @@ namespace MobileGL::MG_Impl::GLImpl {
                                              "Backend does not support indirect-parameter indexed draws."));
             return;
         }
-        // DrawElementsIndirectCommand: count, instanceCount, firstIndex, baseVertex, baseInstance.
-        if (!ValidateIndirectCountDraw(reinterpret_cast<GLintptr>(indirect), drawcount, maxdrawcount, stride,
-                                       5 * sizeof(Uint32), __func__)) {
-            return;
-        }
         MultiDrawElementsIndirectCount_Backend(mode, type, indirect, drawcount, maxdrawcount, stride);
     }
 
     void MultiDrawArraysIndirectCount(GLenum mode, const void* indirect, GLintptr drawcount,
                                       GLsizei maxdrawcount, GLsizei stride) {
+        // Argument validation before the backend-availability check: see DispatchComputeIndirect.
+        // DrawArraysIndirectCommand: count, instanceCount, first, baseInstance.
+        if (!ValidateIndirectCountDraw(reinterpret_cast<GLintptr>(indirect), drawcount, maxdrawcount, stride,
+                                       4 * sizeof(Uint32), __func__)) {
+            return;
+        }
         auto multiDrawArraysIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawArraysIndirectCount;
         if (!multiDrawArraysIndirectCount) {
             MG_State::pGLContext->RecordError(
@@ -677,11 +690,6 @@ namespace MobileGL::MG_Impl::GLImpl {
                                              "Backend does not support indirect-parameter array draws."));
             return;
         }
-        // DrawArraysIndirectCommand: count, instanceCount, first, baseInstance.
-        if (!ValidateIndirectCountDraw(reinterpret_cast<GLintptr>(indirect), drawcount, maxdrawcount, stride,
-                                       4 * sizeof(Uint32), __func__)) {
-            return;
-        }
         MultiDrawArraysIndirectCount_Backend(mode, indirect, drawcount, maxdrawcount, stride);
     }
 

MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp

@@ -95,12 +95,16 @@ namespace {
         ASSERT_NE(reservedOnly, 0u);
         ASSERT_EQ(IsBuffer(reservedOnly), GL_FALSE);
 
-        GLuint samplerReservedOnly = 0;
+        // glGenSamplers, unlike glGenBuffers, creates the objects outright, so a sampler name is
-        GenSamplers(1, &samplerReservedOnly);
+        // only "not an existing object" once it has been deleted.
+        GLuint deadSampler = 0;
+        GenSamplers(1, &deadSampler);
+        ASSERT_NE(deadSampler, 0u);
+        DeleteSamplers(1, &deadSampler);
         DrainErrors();
 
         const GLuint mixedBuffers[2] = {buffer, reservedOnly};
-        const GLuint samplers[1] = {samplerReservedOnly};
+        const GLuint samplers[1] = {deadSampler};
         const GLintptr offsets[2] = {0, 0};
         const GLsizeiptr sizes[2] = {256, 256};
 
@@ -110,7 +114,7 @@ namespace {
             {"glBindBuffersRange with a reserved-but-uncreated name",
              [&] { BindBuffersRange(GL_UNIFORM_BUFFER, 0, 2, mixedBuffers, offsets, sizes); },
              GL_INVALID_OPERATION},
-            {"glBindSamplers with a reserved-but-uncreated name", [&] { BindSamplers(0, 1, samplers); },
+            {"glBindSamplers with a deleted sampler name", [&] { BindSamplers(0, 1, samplers); },
              GL_INVALID_OPERATION},
         });