[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

This commit is contained in:
2026-08-11 09:02:22 -04:00
parent d83b4dbbb5
commit 7b593e39ef
9 changed files with 547 additions and 16 deletions
+38 -8
View File
@@ -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<GenericErrorInfo>("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<GenericErrorInfo>(
"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<GenericErrorInfo>(
"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);
@@ -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<SizeT>(indirect) + kDispatchIndirectCommandSize > indirectBuffer->GetSize()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>(
"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<GenericErrorInfo>("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<SizeT>(drawcount) + sizeof(Uint32) > parameterBuffer->GetSize()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("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<GenericErrorInfo>("MG_Impl/GLImpl", funcName,
"indirect, maxdrawcount and stride must all be non-negative."));
return false;
}
const SizeT effectiveStride = stride != 0 ? static_cast<SizeT>(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<SizeT>(indirect) +
static_cast<SizeT>(maxdrawcount - 1) * effectiveStride + commandSize;
if (!indirectBuffer || requiredBytes > indirectBuffer->GetSize()) {
MG_State::pGLContext->RecordError(
ErrorCode::InvalidOperation,
MakeUnique<GenericErrorInfo>("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<GLintptr>(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<GLintptr>(indirect), drawcount, maxdrawcount, stride,
4 * sizeof(Uint32), __func__)) {
return;
}
MultiDrawArraysIndirectCount_Backend(mode, indirect, drawcount, maxdrawcount, stride);
}
+22 -7
View File
@@ -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<GenericErrorInfo>("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<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
"Vertex buffer binding index is out of range."));
return;
}
const auto& vao = MG_State::pGLContext->GetBoundVertexArray();
*data = vao ? static_cast<GLint64>(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<GLint64>(values[0]);
}
void GetInteger64v(GLenum pname, GLint64* params) {
@@ -8,6 +8,7 @@
#include "GL_RenderState.h"
#include <cmath>
#include <MG_Impl/GLImpl/Getter/GL_Getter.h>
#include <MG_State/GLState/Core.h>
#include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
#include <MG_Util/Converters/GLToMG/RenderStateEnumConverter.h>
@@ -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) {
@@ -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<GenericErrorInfo>(
"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);
}
@@ -4110,10 +4110,46 @@ namespace MobileGL::MG_Impl::GLImpl {
textureObject->SetImmutableLevels(static_cast<Uint>(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<GenericErrorInfo>(
"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;