From 7b593e39ef8dad01e5a1d3ca24e009c22516443f Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 11 Aug 2026 09:02:22 -0400 Subject: [PATCH] [Fix, Test] (MG_Impl, MG_State): negative-path GL errors for multi_bind, indirect_parameters, texture_storage, compute dispatch/link and buffer-range alignment; indexed getters answer the full pname table --- MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp | 46 ++- .../MG_Impl/GLImpl/Drawing/GL_Drawing.cpp | 79 +++++ MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp | 29 +- .../GLImpl/RenderState/GL_RenderState.cpp | 14 +- .../MG_Impl/GLImpl/Sampler/GL_Sampler.cpp | 19 ++ .../MG_Impl/GLImpl/Texture/GL_Texture.cpp | 36 +++ .../GLState/ProgramState/ProgramLinkTask.cpp | 17 + MobileGL/MG_Test/State/CMakeLists.txt | 25 ++ .../MG_Test/State/NegativeApiErrorsTest.cpp | 298 ++++++++++++++++++ 9 files changed, 547 insertions(+), 16 deletions(-) create mode 100644 MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp diff --git a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp index 50dc0f5d..850d304f 100644 --- a/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp @@ -1491,8 +1491,8 @@ namespace MobileGL::MG_Impl::GLImpl { // offset and size, which is also how glBindBuffersRange spells "reset this element" // (a NULL buffers array, or a zero entry inside one). static Bool ValidateBufferRangeOffsetAndSize(GLenum target, GLintptr offset, GLsizeiptr size, - const char* funcName) { - if (size <= 0) { + const char* funcName, Bool hasBuffer = true) { + if (hasBuffer && size <= 0) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique("MG_Impl/GLImpl", funcName, @@ -1528,15 +1528,18 @@ namespace MobileGL::MG_Impl::GLImpl { } } // A transform feedback capture binding is addressed in 32-bit components, so BOTH the - // offset and the size must be multiples of 4. - if (target == GL_TRANSFORM_FEEDBACK_BUFFER && ((offset % 4) != 0 || (size % 4) != 0)) { + // offset and the size must be multiples of 4. An atomic counter binding is addressed in + // 32-bit counters and constrains its offset the same way (GL 4.6 core 6.1.1) - that one + // has no queryable alignment pname, which is why it was missing here. + const Bool isFourByteAddressed = + target == GL_TRANSFORM_FEEDBACK_BUFFER || target == GL_ATOMIC_COUNTER_BUFFER; + if (isFourByteAddressed && ((offset % 4) != 0 || (hasBuffer && (size % 4) != 0))) { MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( "MG_Impl/GLImpl", funcName, - std::format("offset ({}) and size ({}) must both be multiples of 4 for " - "GL_TRANSFORM_FEEDBACK_BUFFER.", - offset, size))); + std::format("offset ({}) and size ({}) must both be multiples of 4 for {}.", offset, size, + MG_Util::ConvertGLEnumToString(target)))); return false; } return true; @@ -1548,7 +1551,12 @@ namespace MobileGL::MG_Impl::GLImpl { BufferTarget bufferTarget = MG_Util::ConvertGLEnumToBufferTarget(target); if (!BufferImpl::ValidateBufferBindingPointTarget(bufferTarget)) return; if (!BufferImpl::ValidateBufferBindingPointIndex(bufferTarget, index)) return; - if (buffer != 0 && !ValidateBufferRangeOffsetAndSize(target, offset, size, __func__)) return; + // The target's alignment rules are a property of the BINDING POINT, not of the buffer, + // so they apply even when buffer is zero - which is exactly how + // KHR-GL43.shader_storage_buffer_object.negative-api-bind probes the SSBO alignment + // (glBindBufferRange(SHADER_STORAGE_BUFFER, 0, 0, alignment - 1, 0)). Only the size + // rules need a buffer, since buffer 0 detaches the binding point and ignores size. + if (!ValidateBufferRangeOffsetAndSize(target, offset, size, __func__, /*hasBuffer: */ buffer != 0)) return; if (bufferTarget == BufferTarget::TransformFeedback && MG_State::pGLContext->IsTransformFeedbackActive()) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, @@ -1732,8 +1740,29 @@ namespace MobileGL::MG_Impl::GLImpl { return BufferImpl::ValidateBufferBindingPointRange(bufferTarget, first, count, funcName); } + // ARB_multi_bind states the equivalence to a loop of single binds "except that ... buffers + // will not be created if they do not exist": glBindBuffer instantiates a name glGenBuffers + // merely reserved, glBindBuffers* must refuse it instead. That is INVALID_OPERATION, and it + // is an all-or-nothing check - one bad name leaves every binding point in the range alone + // (KHR-GL44.multi_bind.errors_bind_buffers). + static Bool ValidateMultiBindBufferNames(const GLuint* buffers, GLsizei count, const char* funcName) { + if (buffers == nullptr) return true; + for (GLsizei i = 0; i < count; ++i) { + if (buffers[i] == 0) continue; + if (MG_State::pGLContext->ValidateBufferObject(buffers[i])) continue; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", funcName, + std::format("buffers[{}] ({}) is not the name of an existing buffer object.", i, buffers[i]))); + return false; + } + return true; + } + void BindBuffersBase(GLenum target, GLuint first, GLsizei count, const GLuint* buffers) { if (!ValidateMultiBindBufferRange(target, first, count, __func__)) return; + if (!ValidateMultiBindBufferNames(buffers, count, __func__)) return; for (GLsizei i = 0; i < count; ++i) { BindBufferBase_State(target, first + i, buffers ? buffers[i] : 0); } @@ -1748,6 +1777,7 @@ namespace MobileGL::MG_Impl::GLImpl { void BindBuffersRange(GLenum target, GLuint first, GLsizei count, const GLuint* buffers, const GLintptr* offsets, const GLsizeiptr* sizes) { if (!ValidateMultiBindBufferRange(target, first, count, __func__)) return; + if (!ValidateMultiBindBufferNames(buffers, count, __func__)) return; for (GLsizei i = 0; i < count; ++i) { if (!buffers || buffers[i] == 0) { BindBufferBase_State(target, first + i, 0); diff --git a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp index 51e0aa49..347fa9ac 100644 --- a/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp +++ b/MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp @@ -520,6 +520,20 @@ namespace MobileGL::MG_Impl::GLImpl { "No buffer is bound to GL_DISPATCH_INDIRECT_BUFFER.")); return; } + // ...and the same INVALID_OPERATION covers "the command would source data beyond the end + // of the bound buffer object" (GL 4.6 core 19): the dispatch reads three uints starting + // at `indirect`. + constexpr SizeT kDispatchIndirectCommandSize = 3 * sizeof(Uint32); + if (static_cast(indirect) + kDispatchIndirectCommandSize > indirectBuffer->GetSize()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", __func__, + std::format("indirect ({}) + 12 bytes runs past the end of the {}-byte buffer bound to " + "GL_DISPATCH_INDIRECT_BUFFER.", + indirect, indirectBuffer->GetSize()))); + return; + } dispatchComputeIndirect(indirect); } @@ -580,6 +594,61 @@ namespace MobileGL::MG_Impl::GLImpl { MultiDrawArraysIndirect_Backend(mode, indirect, drawcount, stride); } + // ARB_indirect_parameters / GL 4.6 core 10.4: `drawcount` is a byte offset into the buffer + // bound to PARAMETER_BUFFER and holds one uint draw count. Three errors have to be raised + // before the call reaches a backend, and none of them was + // (KHR-GL46.indirect_parameters_tests.MultiDraw{Arrays,Elements}IndirectCount): + // * drawcount not a multiple of four INVALID_VALUE + // * nothing bound to PARAMETER_BUFFER, or the uint at `drawcount` + // lies past its end INVALID_OPERATION + // * maxdrawcount commands from `indirect` run past the end of the + // buffer bound to DRAW_INDIRECT_BUFFER INVALID_OPERATION + static Bool ValidateIndirectCountDraw(GLintptr indirect, GLintptr drawcount, GLsizei maxdrawcount, + GLsizei stride, SizeT commandSize, const char* funcName) { + if (drawcount < 0 || (drawcount % 4) != 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", funcName, + "drawcount must be non-negative and a multiple of four.")); + return false; + } + const auto& parameterBuffer = + MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::Parameter).GetBoundObject(); + if (!parameterBuffer || + static_cast(drawcount) + sizeof(Uint32) > parameterBuffer->GetSize()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", funcName, + "No buffer is bound to GL_PARAMETER_BUFFER, or drawcount runs past " + "the end of the one that is.")); + return false; + } + if (maxdrawcount < 0 || stride < 0 || indirect < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", funcName, + "indirect, maxdrawcount and stride must all be non-negative.")); + return false; + } + const SizeT effectiveStride = stride != 0 ? static_cast(stride) : commandSize; + const auto& indirectBuffer = + MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject(); + // A zero maxdrawcount sources nothing, so it cannot run past anything. + const SizeT requiredBytes = + maxdrawcount == 0 ? 0 + : static_cast(indirect) + + static_cast(maxdrawcount - 1) * effectiveStride + commandSize; + if (!indirectBuffer || requiredBytes > indirectBuffer->GetSize()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", funcName, + "maxdrawcount commands would be sourced from beyond the end of the " + "buffer bound to GL_DRAW_INDIRECT_BUFFER.")); + return false; + } + return true; + } + void MultiDrawElementsIndirectCount(GLenum mode, GLenum type, const void* indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride) { auto multiDrawElementsIndirectCount = MG_Backend::gBackendFunctionsTable.GL.MultiDrawElementsIndirectCount; @@ -590,6 +659,11 @@ namespace MobileGL::MG_Impl::GLImpl { "Backend does not support indirect-parameter indexed draws.")); return; } + // DrawElementsIndirectCommand: count, instanceCount, firstIndex, baseVertex, baseInstance. + if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, + 5 * sizeof(Uint32), __func__)) { + return; + } MultiDrawElementsIndirectCount_Backend(mode, type, indirect, drawcount, maxdrawcount, stride); } @@ -603,6 +677,11 @@ namespace MobileGL::MG_Impl::GLImpl { "Backend does not support indirect-parameter array draws.")); return; } + // DrawArraysIndirectCommand: count, instanceCount, first, baseInstance. + if (!ValidateIndirectCountDraw(reinterpret_cast(indirect), drawcount, maxdrawcount, stride, + 4 * sizeof(Uint32), __func__)) { + return; + } MultiDrawArraysIndirectCount_Backend(mode, indirect, drawcount, maxdrawcount, stride); } diff --git a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp index a5694457..f723c3e4 100644 --- a/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp +++ b/MobileGL/MG_Impl/GLImpl/Getter/GL_Getter.cpp @@ -961,15 +961,30 @@ namespace MobileGL::MG_Impl::GLImpl { } } - auto getInteger64i = MG_Backend::gBackendFunctionsTable.GL.GetInteger64i_v; - if (!getInteger64i) { - *data = 0; - MG_State::pGLContext->RecordError( - ErrorCode::InvalidOperation, - MakeUnique("MG_Impl/GLImpl", __func__, "Backend does not support indexed integer queries.")); + // The one indexed pname whose value genuinely needs 64 bits: a vertex buffer binding + // offset is an intptr, so taking the 32-bit route below would truncate it. + if (target == GL_VERTEX_BINDING_OFFSET) { + if (index >= VertexArrayImpl::GetMaxVertexAttribBindings()) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", __func__, + "Vertex buffer binding index is out of range.")); + return; + } + const auto& vao = MG_State::pGLContext->GetBoundVertexArray(); + *data = vao ? static_cast(vao->GetBindingPoint(index).Offset) : 0; return; } - getInteger64i(target, index, data); + + // Everything else is 32-bit indexed state that the glGetIntegeri_v pname table already + // owns, and GL 4.6 core 22.1 says every indexed query answers every indexed pname. + // Handing the leftovers straight to the backend instead made glGetInteger64i_v disagree + // with glGetIntegeri_v on the very same pname - GL_MAX_COMPUTE_WORK_GROUP_COUNT read + // back 0 while the 32-bit view said 65535 (KHR-GL43.compute_shader.max), because a + // frontend-only value simply is not in the driver's table. + GLint values[4] = {}; + GetIntegeri_v(target, index, values); + *data = static_cast(values[0]); } void GetInteger64v(GLenum pname, GLint64* params) { diff --git a/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp b/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp index aa1fe5a1..14697130 100644 --- a/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp +++ b/MobileGL/MG_Impl/GLImpl/RenderState/GL_RenderState.cpp @@ -8,6 +8,7 @@ #include "GL_RenderState.h" #include +#include #include #include #include @@ -380,7 +381,18 @@ namespace MobileGL::MG_Impl::GLImpl { return; } - *data = IsEnabledi_State(target, index); + // GL 4.6 core 22.1: glGetBooleani_v answers EVERY indexed state, not just the indexed + // capabilities - a non-boolean value simply reads back as "is it non-zero". Routing the + // non-capability enums to the pname table glGetIntegeri_v already owns is what makes + // that true; without it a query like glGetBooleani_v(GL_MAX_COMPUTE_WORK_GROUP_COUNT, 0) + // came back GL_INVALID_ENUM (KHR-GL43.compute_shader.max). + if (MG_Util::ConvertGLEnumToCapabilityInput(target) != CapabilityInput::Unknown) { + *data = IsEnabledi_State(target, index); + return; + } + GLint values[4] = {}; + GetIntegeri_v(target, index, values); + *data = values[0] != 0 ? GL_TRUE : GL_FALSE; } GLboolean IsEnabled_State(GLenum cap) { diff --git a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp index f5ad6ad3..7f9c2ae8 100644 --- a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp +++ b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp @@ -336,6 +336,25 @@ namespace MobileGL::MG_Impl::GLImpl { return; } + // ...and the names are checked up front for the same reason, with the extra rule that + // ARB_multi_bind spells out separately: "samplers will not be created if they do not + // exist". The single-bind path instantiates a name glGenSamplers merely reserved; here + // a name that is not an existing sampler OBJECT is INVALID_OPERATION and nothing binds + // (KHR-GL44.multi_bind.errors_bind_samplers). + if (samplers != nullptr) { + for (GLsizei i = 0; i < count; ++i) { + if (samplers[i] == 0) continue; + if (MG_State::pGLContext->ValidateSamplerObject(samplers[i])) continue; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", "BindSamplers", + std::format("samplers[{}] ({}) is not the name of an existing sampler object.", i, + samplers[i]))); + return; + } + } + for (GLsizei i = 0; i < count; ++i) { BindSampler_State(first + i, samplers ? samplers[i] : 0); } diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index e365b558..39ebf6fd 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -4110,10 +4110,46 @@ namespace MobileGL::MG_Impl::GLImpl { textureObject->SetImmutableLevels(static_cast(levels)); } + // No block-compressed format is defined for a three-dimensional image, so glTexStorage3D on + // TEXTURE_3D must reject one - and with INVALID_OPERATION, not the INVALID_ENUM an unknown + // sized format gets (GL 4.6 core 8.19 / Khronos bug 11239, KHR-GLxx.texture_storage + // .compressed_data). Written against the enum ranges rather than a name list because the + // families are contiguous and MobileGL's own internal-format enum drops the ones it cannot + // carry, which would make this check silently narrower than the API surface. + static Bool IsCompressedGLInternalFormat(GLenum internalformat) { + switch (internalformat) { + case 0x8225: // GL_COMPRESSED_RED + case 0x8226: // GL_COMPRESSED_RG + case 0x84ED: // GL_COMPRESSED_RGB + case 0x84EE: // GL_COMPRESSED_RGBA + case 0x8C48: // GL_COMPRESSED_SRGB + case 0x8C49: // GL_COMPRESSED_SRGB_ALPHA + return true; + default: + break; + } + return (internalformat >= 0x83F0 && internalformat <= 0x83F3) || // S3TC / DXT + (internalformat >= 0x8DBB && internalformat <= 0x8DBE) || // RGTC + (internalformat >= 0x8E8C && internalformat <= 0x8E8F) || // BPTC + (internalformat >= 0x9270 && internalformat <= 0x9279) || // ETC2 / EAC + (internalformat >= 0x93B0 && internalformat <= 0x93BD) || // ASTC LDR + (internalformat >= 0x93D0 && internalformat <= 0x93DD); // ASTC sRGB + } + void TextureStorage3D(GLuint texture, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth) { auto textureObject = GetTextureObjectByName(texture, __func__); if (!textureObject) return; + if (textureObject->GetTarget() == TextureTarget::Texture3D && + IsCompressedGLInternalFormat(internalformat)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", __func__, + std::format("{} is a compressed internal format and cannot back GL_TEXTURE_3D storage.", + MG_Util::ConvertGLEnumToString(internalformat)))); + return; + } TextureInternalFormat textureInternalFormat = MG_Util::ConvertGLEnumToTextureInternalFormat(internalformat); if (!ValidateTextureStorageInternalFormat(textureInternalFormat, __func__)) return; if (!ValidateTextureStorageShape(textureObject, 3, levels, width, height, depth, __func__)) return; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp index b19f0c3b..ebb0be57 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramLinkTask.cpp @@ -446,6 +446,23 @@ namespace MobileGL::MG_State::GLState { Bool ProgramLinkTask::ConsumeShaders(Vector>& outShaders) { outShaders.assign(in.shaders.size(), nullptr); + // GL 4.6 core 7.3: a compute shader may only be linked with other compute shaders - + // the compute pipeline has no other stages to link against, so a program that mixes + // them must fail to link (KHR-GL43.compute_shader.api-program). + { + Bool hasCompute = false; + Bool hasNonCompute = false; + for (const LinkShaderInput& input : in.shaders) { + (input.stage == ShaderStage::Compute ? hasCompute : hasNonCompute) = true; + } + if (hasCompute && hasNonCompute) { + artifacts.infoLog = + "A compute shader cannot be linked with shaders of any other stage."; + DeferLog(std::format("ProgramObject {}: Link failed - {}", in.externalIndex, artifacts.infoLog)); + return false; + } + } + for (SizeT i = 0; i < in.shaders.size(); i++) { const LinkShaderInput& input = in.shaders[i]; const GLenum shaderType = MG_Util::ConvertShaderStageToGLEnum(input.stage); diff --git a/MobileGL/MG_Test/State/CMakeLists.txt b/MobileGL/MG_Test/State/CMakeLists.txt index 63e3f664..a3227690 100644 --- a/MobileGL/MG_Test/State/CMakeLists.txt +++ b/MobileGL/MG_Test/State/CMakeLists.txt @@ -50,3 +50,28 @@ if (MSVC) endif() gtest_discover_tests(RenderStateTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) + +add_executable( + NegativeApiErrorsTest + NegativeApiErrorsTest.cpp +) + +target_include_directories(NegativeApiErrorsTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL + ${MGL_ROOT}/3rdparty/xxHash + ${MGL_ROOT}/3rdparty/Vulkan-Headers/include + ${MGL_ROOT}/3rdparty/SPIRV-Reflect +) + +target_link_libraries( + NegativeApiErrorsTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + +if (MSVC) + target_compile_options(NegativeApiErrorsTest PRIVATE /Zc:preprocessor) +endif() + +gtest_discover_tests(NegativeApiErrorsTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) diff --git a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp new file mode 100644 index 00000000..151e61d9 --- /dev/null +++ b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp @@ -0,0 +1,298 @@ +// MobileGL - MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +// The negative-path GL errors the conformance suite checks and MobileGL used to answer +// GL_NO_ERROR to. Every row here is a call the spec requires to fail, lifted from the CTS case +// that found it: +// * KHR-GL44.multi_bind.errors_bind_buffers / .errors_bind_samplers - ARB_multi_bind's +// "buffers/samplers will not be created if they do not exist" rule, plus the atomic-counter +// offset alignment the single-bind path never had. +// * KHR-GL43.shader_storage_buffer_object.negative-api-bind - the SSBO offset alignment is a +// property of the binding point and applies with buffer 0 too. +// * KHR-GL46.indirect_parameters_tests.MultiDraw{Arrays,Elements}IndirectCount - the three +// errors that guard a parameter-buffer draw. +// * KHR-GL43.compute_shader.api-indirect / .api-program. +// * KHR-GLxx.texture_storage.compressed_data - compressed formats on TEXTURE_3D. +// Plus the indexed-getter parity RC-7b is about: glGetBooleani_v / glGetInteger64i_v / +// glGetFloati_v / glGetDoublei_v must answer every pname glGetIntegeri_v answers. +// +// GPU-free: all of it is frontend validation. + +#include + +#include +#include +#include + +#include "Includes.h" +#include "Init.h" +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace MobileGL; +using namespace MobileGL::MG_Impl::GLImpl; + +namespace { + class NegativeApiErrorsTest : public ::testing::Test { + protected: + void SetUp() override { + MobileGL::Initialize(); + MG_State::pGLContext = MakeUnique(); + } + + void TearDown() override { + EXPECT_EQ(GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind"; + } + + static void DrainErrors() { + for (int i = 0; i < 16 && GetError() != GL_NO_ERROR; ++i) { + } + } + + static GLuint MakeBuffer(GLenum target, GLsizeiptr size) { + GLuint buffer = 0; + GenBuffers(1, &buffer); + BindBuffer(target, buffer); + BufferData(target, size, nullptr, GL_STATIC_DRAW); + return buffer; + } + + // One table row: run the call, assert exactly the expected error, leave nothing pending. + struct Row { + const char* what; + std::function call; + GLenum expected; + }; + + static void RunRows(const std::vector& rows) { + for (const Row& row : rows) { + DrainErrors(); + row.call(); + EXPECT_EQ(GetError(), row.expected) << row.what; + DrainErrors(); + } + } + }; + + TEST_F(NegativeApiErrorsTest, MultiBindRejectsNamesThatAreNotObjectsYet) { + const GLuint buffer = MakeBuffer(GL_UNIFORM_BUFFER, 1024); + // Reserved by glGenBuffers but never turned into an object: legal for glBindBuffer, + // which creates it, and illegal for glBindBuffersBase, which must not. + GLuint reservedOnly = 0; + GenBuffers(1, &reservedOnly); + ASSERT_NE(reservedOnly, 0u); + ASSERT_EQ(IsBuffer(reservedOnly), GL_FALSE); + + GLuint samplerReservedOnly = 0; + GenSamplers(1, &samplerReservedOnly); + DrainErrors(); + + const GLuint mixedBuffers[2] = {buffer, reservedOnly}; + const GLuint samplers[1] = {samplerReservedOnly}; + const GLintptr offsets[2] = {0, 0}; + const GLsizeiptr sizes[2] = {256, 256}; + + RunRows({ + {"glBindBuffersBase with a reserved-but-uncreated name", + [&] { BindBuffersBase(GL_UNIFORM_BUFFER, 0, 2, mixedBuffers); }, GL_INVALID_OPERATION}, + {"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); }, + GL_INVALID_OPERATION}, + }); + + // The rejected call must have bound nothing at all. + GLint bound = -1; + GetIntegeri_v(GL_UNIFORM_BUFFER_BINDING, 0, &bound); + EXPECT_EQ(bound, 0); + DrainErrors(); + } + + TEST_F(NegativeApiErrorsTest, BufferRangeOffsetAlignmentAppliesToTheBindingPoint) { + GLint ssboAlignment = 0; + GetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, &ssboAlignment); + ASSERT_GT(ssboAlignment, 1) << "the alignment rule is untestable at alignment 1"; + const GLuint atomicBuffer = MakeBuffer(GL_ATOMIC_COUNTER_BUFFER, 1024); + DrainErrors(); + + RunRows({ + // buffer 0 detaches the binding point, but the target's alignment rule still holds. + {"glBindBufferRange(SHADER_STORAGE_BUFFER, buffer 0, misaligned offset)", + [&] { BindBufferRange(GL_SHADER_STORAGE_BUFFER, 0, 0, ssboAlignment - 1, 0); }, GL_INVALID_VALUE}, + // An atomic counter binding is addressed in 32-bit counters; it has no queryable + // alignment pname, which is how its rule went missing. + {"glBindBufferRange(ATOMIC_COUNTER_BUFFER, offset 3)", + [&] { BindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, atomicBuffer, 3, 16); }, GL_INVALID_VALUE}, + {"glBindBufferRange(ATOMIC_COUNTER_BUFFER, size 15)", + [&] { BindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, atomicBuffer, 4, 15); }, GL_INVALID_VALUE}, + }); + + // ...and the aligned form still works. + DrainErrors(); + BindBufferRange(GL_ATOMIC_COUNTER_BUFFER, 0, atomicBuffer, 4, 16); + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + + TEST_F(NegativeApiErrorsTest, DispatchComputeIndirectChecksTheBoundBufferExtent) { + // Six uints: an indirect dispatch reads three, so offset 16 runs off the end. + const GLuint dispatchBuffer = MakeBuffer(GL_DISPATCH_INDIRECT_BUFFER, 6 * sizeof(GLuint)); + DrainErrors(); + + RunRows({ + {"glDispatchComputeIndirect(-2)", [] { DispatchComputeIndirect(-2); }, GL_INVALID_VALUE}, + {"glDispatchComputeIndirect(3)", [] { DispatchComputeIndirect(3); }, GL_INVALID_VALUE}, + {"glDispatchComputeIndirect(16) past the end of a 24-byte buffer", + [] { DispatchComputeIndirect(16); }, GL_INVALID_OPERATION}, + {"glDispatchComputeIndirect(0) with nothing bound", + [&] { + BindBuffer(GL_DISPATCH_INDIRECT_BUFFER, 0); + DispatchComputeIndirect(0); + }, + GL_INVALID_OPERATION}, + }); + static_cast(dispatchBuffer); + } + + TEST_F(NegativeApiErrorsTest, IndirectParameterDrawsCheckBothBuffers) { + // Two DrawArraysIndirectCommands (16 bytes each) and a roomy parameter buffer. + MakeBuffer(GL_DRAW_INDIRECT_BUFFER, 2 * 4 * sizeof(GLuint)); + const GLuint parameterBuffer = MakeBuffer(GL_PARAMETER_BUFFER, 200); + DrainErrors(); + + RunRows({ + {"glMultiDrawArraysIndirectCount with drawcount 2 (not a multiple of four)", + [] { MultiDrawArraysIndirectCount(GL_TRIANGLE_STRIP, nullptr, 2, 1, 0); }, GL_INVALID_VALUE}, + {"glMultiDrawArraysIndirectCount with maxdrawcount past the indirect buffer", + [] { MultiDrawArraysIndirectCount(GL_TRIANGLE_STRIP, nullptr, 0, 4, 0); }, GL_INVALID_OPERATION}, + {"glMultiDrawElementsIndirectCount with drawcount 2", + [] { MultiDrawElementsIndirectCount(GL_TRIANGLE_STRIP, GL_UNSIGNED_BYTE, nullptr, 2, 1, 0); }, + GL_INVALID_VALUE}, + {"glMultiDrawArraysIndirectCount with no parameter buffer bound", + [&] { + BindBuffer(GL_PARAMETER_BUFFER, 0); + MultiDrawArraysIndirectCount(GL_TRIANGLE_STRIP, nullptr, 0, 2, 0); + }, + GL_INVALID_OPERATION}, + }); + static_cast(parameterBuffer); + } + + TEST_F(NegativeApiErrorsTest, TexStorage3DRejectsCompressedFormatsOnTexture3D) { + GLuint texture = 0; + GenTextures(1, &texture); + BindTexture(GL_TEXTURE_3D, texture); + DrainErrors(); + + RunRows({ + {"glTexStorage3D(TEXTURE_3D, GL_COMPRESSED_RED_RGTC1)", + [] { TexStorage3D(GL_TEXTURE_3D, 1, 0x8DBB /* GL_COMPRESSED_RED_RGTC1 */, 8, 8, 8); }, + GL_INVALID_OPERATION}, + {"glTexStorage3D(TEXTURE_3D, GL_COMPRESSED_RG_RGTC2)", + [] { TexStorage3D(GL_TEXTURE_3D, 1, 0x8DBD /* GL_COMPRESSED_RG_RGTC2 */, 8, 8, 8); }, + GL_INVALID_OPERATION}, + }); + + // An uncompressed sized format on the same target still allocates. + DrainErrors(); + TexStorage3D(GL_TEXTURE_3D, 1, GL_RGBA8, 8, 8, 8); + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + + TEST_F(NegativeApiErrorsTest, LinkRejectsAComputeAndNonComputeMix) { + const auto attach = [](GLuint program, GLenum stage, const char* source) { + const GLuint shader = CreateShader(stage); + ShaderSource(shader, 1, &source, nullptr); + CompileShader(shader); + AttachShader(program, shader); + }; + const GLuint program = CreateProgram(); + attach(program, GL_COMPUTE_SHADER, R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430) buffer Output { uint g_output[]; }; +void main() { g_output[gl_GlobalInvocationID.x] = 0; } +)"); + attach(program, GL_VERTEX_SHADER, R"(#version 430 core +layout(location = 0) in vec4 g_position; +void main() { gl_Position = g_position; } +)"); + attach(program, GL_FRAGMENT_SHADER, R"(#version 430 core +layout(location = 0) out vec4 g_color; +void main() { g_color = vec4(1); } +)"); + LinkProgram(program); + + GLint status = GL_TRUE; + GetProgramiv(program, GL_LINK_STATUS, &status); + EXPECT_EQ(status, GL_FALSE) << "a compute shader must not link with any other stage"; + DrainErrors(); + } + + // RC-7b: the four non-int indexed getters have to answer the same pname table glGetIntegeri_v + // does. glGetBooleani_v used to route everything through the indexed-capability path + // (GL_INVALID_ENUM for anything else) and glGetInteger64i_v straight to the driver, which + // does not have MobileGL's frontend-only values at all. + TEST_F(NegativeApiErrorsTest, IndexedGettersAgreeWithGetIntegeriv) { + DrainErrors(); + const GLenum pnames[] = {GL_MAX_COMPUTE_WORK_GROUP_COUNT, GL_MAX_COMPUTE_WORK_GROUP_SIZE}; + for (GLenum pname : pnames) { + for (GLuint index = 0; index < 3; ++index) { + GLint reference = -1; + GetIntegeri_v(pname, index, &reference); + ASSERT_EQ(GetError(), GL_NO_ERROR) << "glGetIntegeri_v(" << pname << ", " << index << ")"; + ASSERT_GT(reference, 0) << "the reference value has to be non-trivial to compare against"; + + GLint64 as64 = -1; + GetInteger64i_v(pname, index, &as64); + EXPECT_EQ(as64, static_cast(reference)) << "glGetInteger64i_v(" << pname << ")"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLfloat asFloat = -1.0f; + GetFloati_v(pname, index, &asFloat); + EXPECT_FLOAT_EQ(asFloat, static_cast(reference)) << "glGetFloati_v(" << pname << ")"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLdouble asDouble = -1.0; + GetDoublei_v(pname, index, &asDouble); + EXPECT_DOUBLE_EQ(asDouble, static_cast(reference)) << "glGetDoublei_v(" << pname << ")"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLboolean asBool = GL_FALSE; + GetBooleani_v(pname, index, &asBool); + EXPECT_EQ(asBool, GL_TRUE) << "glGetBooleani_v(" << pname << ")"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + } + } + } + + // ...and the vertex-binding offset keeps its 64-bit width through glGetInteger64i_v, which is + // how KHR-GL4x.vertex_attrib_binding reads it. + TEST_F(NegativeApiErrorsTest, VertexBindingOffsetIsReadableThroughTheSixtyFourBitGetter) { + GLuint vao = 0; + GenVertexArrays(1, &vao); + BindVertexArray(vao); + const GLuint vbo = MakeBuffer(GL_ARRAY_BUFFER, 4096); + DrainErrors(); + + GLint64 offset = -1; + GetInteger64i_v(GL_VERTEX_BINDING_OFFSET, 0, &offset); + EXPECT_EQ(offset, 0); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + BindVertexBuffer(0, vbo, 2048, 128); + GetInteger64i_v(GL_VERTEX_BINDING_OFFSET, 0, &offset); + EXPECT_EQ(offset, 2048); + EXPECT_EQ(GetError(), GL_NO_ERROR); + } +} // namespace