diff --git a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp index 020b186f..62e0e1a2 100644 --- a/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp +++ b/MobileGL/MG_Impl/GLImpl/Exporting/Definitions.cpp @@ -1003,9 +1003,9 @@ DECLARE_GL_FUNCTION_HEAD(void, ClearTexImage, GLuint texture, GLint level, GLenu DECLARE_GL_FUNCTION_HEAD(void, ClearTexSubImage, GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type, const void* data) DECLARE_GL_FUNCTION_END_NO_RETURN(void, ClearTexSubImage, texture, level, xoffset, yoffset, zoffset, width, height, depth, format, type, data) DECLARE_GL_FUNCTION_HEAD(void, BindBuffersBase, GLenum target, GLuint first, GLsizei count, const GLuint* buffers) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindBuffersBase, target, first, count, buffers) DECLARE_GL_FUNCTION_HEAD(void, BindBuffersRange, GLenum target, GLuint first, GLsizei count, const GLuint* buffers, const GLintptr* offsets, const GLsizeiptr* sizes) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindBuffersRange, target, first, count, buffers, offsets, sizes) -DECLARE_GL_FUNCTION_STUB_HEAD(void, BindTextures, GLuint first, GLsizei count, const GLuint* textures) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, BindTextures, first, count, textures) +DECLARE_GL_FUNCTION_HEAD(void, BindTextures, GLuint first, GLsizei count, const GLuint* textures) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindTextures, first, count, textures) DECLARE_GL_FUNCTION_HEAD(void, BindSamplers, GLuint first, GLsizei count, const GLuint* samplers) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindSamplers, first, count, samplers) -DECLARE_GL_FUNCTION_STUB_HEAD(void, BindImageTextures, GLuint first, GLsizei count, const GLuint* textures) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, BindImageTextures, first, count, textures) +DECLARE_GL_FUNCTION_HEAD(void, BindImageTextures, GLuint first, GLsizei count, const GLuint* textures) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindImageTextures, first, count, textures) DECLARE_GL_FUNCTION_HEAD(void, BindVertexBuffers, GLuint first, GLsizei count, const GLuint* buffers, const GLintptr* offsets, const GLsizei* strides) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindVertexBuffers, first, count, buffers, offsets, strides) DECLARE_GL_FUNCTION_STUB_HEAD(void, ClipControl, GLenum origin, GLenum depth) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, ClipControl, origin, depth) DECLARE_GL_FUNCTION_HEAD(void, CreateTransformFeedbacks, GLsizei n, GLuint* ids) DECLARE_GL_FUNCTION_END_NO_RETURN(void, CreateTransformFeedbacks, n, ids) diff --git a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp index 0939d680..912d6ef3 100644 --- a/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp +++ b/MobileGL/MG_Impl/GLImpl/Sampler/GL_Sampler.cpp @@ -9,6 +9,7 @@ #include "GL_Sampler.h" #include "Validators.h" #include "../Getter/GL_Getter.h" +#include "../Texture/GL_Texture.h" #include #include #include @@ -269,15 +270,13 @@ namespace MobileGL::MG_Impl::GLImpl { } } - // The number of texture units a sampler may be bound to. GL 3.3 core 3.8.2 names - // GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, which is what the backend advertises; the frontend's - // MAX_TEXTURE_IMAGE_UNITS is only the capacity of the unit array, so it is a clamp on the - // answer and never the answer itself - gating on it alone accepts every unit up to 192 no - // matter what the driver reports. + // The number of texture units a sampler may be bound to is the same count a TEXTURE may be + // bound to - GL 3.3 core 3.8.2 names GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS for both - so it is + // computed once, in GetCombinedTextureImageUnitCount, and named here for the sampler-side + // readers below. Two copies of that arithmetic is how glBindSamplers and glBindTextures would + // come to disagree about which units exist. static GLint GetSamplerBindableTextureUnitCount() { - GLint maxTextureUnits = 0; - GetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &maxTextureUnits); - return std::min(std::max(maxTextureUnits, 0), MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS); + return GetCombinedTextureImageUnitCount(); } void BindSampler_State(GLuint unit, GLuint sampler) { diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 39ebf6fd..b4237cb7 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -4549,6 +4549,132 @@ namespace MobileGL::MG_Impl::GLImpl { MG_State::pGLContext->NoteTextureUnitTouched(static_cast(unit), changed); } + GLint GetCombinedTextureImageUnitCount() { + GLint maxTextureUnits = 0; + GetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &maxTextureUnits); + return std::min(std::max(maxTextureUnits, 0), MG_State::GLState::TextureState::MAX_TEXTURE_IMAGE_UNITS); + } + + namespace { + // ARB_multi_bind checks the whole [first, first + count) range before binding anything and + // reports an overrun as INVALID_OPERATION - not the INVALID_VALUE the single-bind entry + // points report for an out-of-range unit, and not after binding the in-range prefix. + Bool ValidateMultiBindUnitRange(GLuint first, GLsizei count, GLint unitCount, const char* funcName) { + if (count < 0) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", funcName, "count must be non-negative.")); + return false; + } + if (static_cast(first) + static_cast(count) > static_cast(unitCount)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", funcName, + std::format("first + count ({} + {}) exceeds the {} available units.", + first, count, unitCount))); + return false; + } + return true; + } + + // ARB_multi_bind states the equivalence to a loop of single binds "except that + // will not be created if they do not exist": glBindTexture instantiates a name GenTextures + // merely reserved, the multi-bind entry points must refuse it. The error class is + // INVALID_OPERATION for both of them, where the scalar glBindImageTexture reports + // INVALID_VALUE - hence the check here rather than inside BindImageTexture. + // + // Deliberately PER ELEMENT: the extension defines these calls as a loop, so a bad entry + // costs its own unit and leaves the rest of the range bound. + SharedPtr ResolveMultiBindTexture(GLuint texture, GLsizei index, + const char* funcName) { + SharedPtr textureObject = + MG_State::pGLContext->GetTextureObject(texture); + if (!textureObject) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", funcName, + std::format("textures[{}] ({}) is not the name of an existing texture object.", index, + texture))); + } + return textureObject; + } + + // ARB_multi_bind: an element naming texture zero unbinds EVERY target of its unit, i.e. + // rebinds each target's default texture object - the unit's initial state. Same rule + // glBindTextureUnit(unit, 0) follows. + void UnbindAllTargetsOnUnit(Int unit) { + auto& textureUnit = MG_State::pGLContext->GetTextureUnitObject(unit); + Bool changed = false; + for (auto& slot : textureUnit.GetAllBindingSlots()) { + if (slot.Bind(MG_State::pGLContext->GetDefaultTextureObject(slot.GetTarget()))) changed = true; + } + MG_State::pGLContext->NoteTextureUnitTouched(unit, changed); + } + } // namespace + + // ARB_multi_bind: glBindTextures binds each texture to ITS OWN target on unit + i, so + // there is no target parameter and no way to express it through glBindTexture - the per-unit, + // by-object form glBindTextureUnit uses is the one that matches. A NULL unbinds the + // whole range. + void BindTextures(GLuint first, GLsizei count, const GLuint* textures) { + if (!ValidateMultiBindUnitRange(first, count, GetCombinedTextureImageUnitCount(), __func__)) return; + + for (GLsizei i = 0; i < count; ++i) { + const GLuint texture = textures ? textures[i] : 0; + const Int unit = static_cast(first) + i; + if (texture == 0) { + UnbindAllTargetsOnUnit(unit); + continue; + } + const SharedPtr textureObject = + ResolveMultiBindTexture(texture, i, __func__); + if (!textureObject) continue; + + auto& textureUnit = MG_State::pGLContext->GetTextureUnitObject(unit); + const Bool changed = textureUnit.GetBindingSlot(textureObject->GetTarget()).Bind(textureObject); + MG_State::pGLContext->NoteTextureUnitTouched(unit, changed); + } + } + + // ARB_multi_bind: glBindImageTextures is a loop of glBindImageTexture with every parameter but + // the unit and the texture fixed by the spec - level 0, layered, layer 0, READ_WRITE, and the + // texture's own internal format. An element that names texture zero resets the unit. + void BindImageTextures(GLuint first, GLsizei count, const GLuint* textures) { + if (!ValidateMultiBindUnitRange(first, count, static_cast(GetAdvertisedImageUnitCount()), __func__)) { + return; + } + + for (GLsizei i = 0; i < count; ++i) { + const GLuint texture = textures ? textures[i] : 0; + const GLuint unit = first + static_cast(i); + if (texture == 0) { + BindImageTexture(unit, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_R8); + continue; + } + const SharedPtr textureObject = + ResolveMultiBindTexture(texture, i, __func__); + if (!textureObject) continue; + + // "An INVALID_OPERATION error is generated if the internal format of any texture is not + // supported for image textures" - a texture that has never been given storage has no + // format at all and lands here too, rather than being reported as a bad enum by the + // scalar path. + const GLenum format = MG_Util::ConvertTextureInternalFormatToGLEnum(textureObject->GetFormat()); + if (!IsValidImageTextureFormat(format)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique( + "MG_Impl/GLImpl", __func__, + std::format("textures[{}] ({}) has an internal format that is not supported for image " + "textures.", + i, texture))); + continue; + } + BindImageTexture(unit, texture, 0, GL_TRUE, 0, GL_READ_WRITE, format); + } + } + void GetTextureImage(GLuint texture, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels) { auto textureObject = GetTextureObjectByName(texture, __func__); if (!textureObject) return; diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h index 49c9e217..7f800cf5 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.h @@ -132,5 +132,11 @@ namespace MobileGL::MG_Impl::GLImpl { void CompressedTexImage1D(GLenum target, GLint level, GLenum internalformat, GLsizei width, GLint border, GLsizei imageSize, const void* data); void BindTexture(GLenum target, GLuint texture); + void BindTextures(GLuint first, GLsizei count, const GLuint* textures); + void BindImageTextures(GLuint first, GLsizei count, const GLuint* textures); void ActiveTexture(GLenum texture); + // The number of texture image units a texture or a sampler may be bound to: what the backend + // advertises as GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, clamped by the frontend's fixed unit-array + // capacity. Shared so the texture and sampler multi-bind range checks cannot drift apart. + GLint GetCombinedTextureImageUnitCount(); } // namespace MobileGL::MG_Impl::GLImpl diff --git a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp index b3f55d6a..d1169f43 100644 --- a/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp +++ b/MobileGL/MG_Test/State/NegativeApiErrorsTest.cpp @@ -128,6 +128,134 @@ namespace { DrainErrors(); } + // KHR-GL44.multi_bind.errors_bind_textures / .errors_bind_image_textures / .errors_bind_samplers. + // Both entry points were silent no-op stubs, so every row here answered GL_NO_ERROR. + // errors_bind_samplers is in the list because that case checks the invalid-name rule by calling + // glBindTextures with a sampler-name array - a name from the wrong namespace is simply not an + // existing texture. + TEST_F(NegativeApiErrorsTest, MultiBindTexturesRejectsBadRangesAndNames) { + GLint maxUnits = 0; + GetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &maxUnits); + ASSERT_GT(maxUnits, 0); + GLint maxImageUnits = 0; + GetIntegerv(GL_MAX_IMAGE_UNITS, &maxImageUnits); + + GLuint texture = 0; + GenTextures(1, &texture); + BindTexture(GL_TEXTURE_2D, texture); + TexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, 4, 4); + + // Reserved by glGenTextures but never bound: not an object yet, so the multi-bind entry + // points must refuse it instead of creating it the way glBindTexture would. + GLuint reservedOnly = 0; + GenTextures(1, &reservedOnly); + ASSERT_NE(reservedOnly, 0u); + ASSERT_EQ(IsTexture(reservedOnly), GL_FALSE); + DrainErrors(); + + const GLuint good[1] = {texture}; + const GLuint mixed[2] = {texture, reservedOnly}; + + std::vector rows = { + {"glBindTextures with negative count", [&] { BindTextures(0, -1, good); }, GL_INVALID_VALUE}, + {"glBindTextures with first + count past the last unit", + [&] { BindTextures(static_cast(maxUnits), 1, good); }, GL_INVALID_OPERATION}, + {"glBindTextures with a reserved-but-uncreated name", [&] { BindTextures(0, 2, mixed); }, + GL_INVALID_OPERATION}, + {"glBindImageTextures with negative count", [&] { BindImageTextures(0, -1, good); }, GL_INVALID_VALUE}, + }; + if (maxImageUnits > 0) { + rows.push_back({"glBindImageTextures with first + count past the last image unit", + [&] { BindImageTextures(static_cast(maxImageUnits), 1, good); }, + GL_INVALID_OPERATION}); + rows.push_back({"glBindImageTextures with a reserved-but-uncreated name", + [&] { BindImageTextures(0, 2, mixed); }, GL_INVALID_OPERATION}); + } + RunRows(rows); + + // The loop semantics again: the good element at index 0 binds, the bad one does not. + GLint bound = -1; + GetIntegeri_v(GL_TEXTURE_BINDING_2D, 0, &bound); + EXPECT_EQ(static_cast(bound), texture) << "a rejected element must not take the valid ones with it"; + GetIntegeri_v(GL_TEXTURE_BINDING_2D, 1, &bound); + EXPECT_EQ(bound, 0) << "the rejected element must not have bound anything"; + DrainErrors(); + } + + // KHR-GL44.multi_bind.functional_bind_textures / .functional_bind_image_textures: the binding + // has to land on the texture's OWN target - glBindTextures takes no target parameter - and + // element zero has to unbind every target of its unit. + TEST_F(NegativeApiErrorsTest, MultiBindTexturesBindsToTheTexturesOwnTarget) { + GLuint textures[2] = {0, 0}; + GenTextures(2, textures); + BindTexture(GL_TEXTURE_1D, textures[0]); + TexStorage1D(GL_TEXTURE_1D, 1, GL_RGBA8, 4); + BindTexture(GL_TEXTURE_3D, textures[1]); + TexStorage3D(GL_TEXTURE_3D, 1, GL_RGBA8, 4, 4, 4); + // Leave the active unit's slots clean so only the multi-bind result is under test. + BindTexture(GL_TEXTURE_1D, 0); + BindTexture(GL_TEXTURE_3D, 0); + DrainErrors(); + + BindTextures(0, 2, textures); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLint bound = -1; + GetIntegeri_v(GL_TEXTURE_BINDING_1D, 0, &bound); + EXPECT_EQ(static_cast(bound), textures[0]) << "a 1D texture must land on the unit's 1D slot"; + GetIntegeri_v(GL_TEXTURE_BINDING_3D, 0, &bound); + EXPECT_EQ(bound, 0) << "no other target of the unit may be touched"; + GetIntegeri_v(GL_TEXTURE_BINDING_3D, 1, &bound); + EXPECT_EQ(static_cast(bound), textures[1]) << "a 3D texture must land on the unit's 3D slot"; + + // A zero element - and a NULL array - unbind EVERY target of the unit, not just one. + const GLuint zeros[1] = {0}; + BindTextures(0, 1, zeros); + GetIntegeri_v(GL_TEXTURE_BINDING_1D, 0, &bound); + EXPECT_EQ(bound, 0); + BindTextures(1, 1, nullptr); + GetIntegeri_v(GL_TEXTURE_BINDING_3D, 1, &bound); + EXPECT_EQ(bound, 0) << "a NULL unbinds the range"; + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLint maxImageUnits = 0; + GetIntegerv(GL_MAX_IMAGE_UNITS, &maxImageUnits); + if (maxImageUnits > 0) { + // ARB_multi_bind fixes every glBindImageTexture parameter but the unit and the name: + // level 0, layered, layer 0, READ_WRITE, and the texture's own internal format. + BindImageTextures(0, 1, &textures[1]); + EXPECT_EQ(GetError(), GL_NO_ERROR); + GetIntegeri_v(GL_IMAGE_BINDING_NAME, 0, &bound); + EXPECT_EQ(static_cast(bound), textures[1]); + GetIntegeri_v(GL_IMAGE_BINDING_LEVEL, 0, &bound); + EXPECT_EQ(bound, 0); + GetIntegeri_v(GL_IMAGE_BINDING_LAYERED, 0, &bound); + EXPECT_EQ(bound, GL_TRUE); + GetIntegeri_v(GL_IMAGE_BINDING_ACCESS, 0, &bound); + EXPECT_EQ(bound, GL_READ_WRITE); + GetIntegeri_v(GL_IMAGE_BINDING_FORMAT, 0, &bound); + EXPECT_EQ(bound, GL_RGBA8); + + BindImageTextures(0, 1, nullptr); + GetIntegeri_v(GL_IMAGE_BINDING_NAME, 0, &bound); + EXPECT_EQ(bound, 0) << "a NULL resets the image unit"; + } + + // Through the EXPORTED entry points, not just the GLImpl functions: both of these were + // declared with the stub macro, so a working implementation that is never wired into + // Definitions.cpp still answers GL_NO_ERROR and binds nothing. + ::glBindTextures(0, 1, &textures[0]); + GetIntegeri_v(GL_TEXTURE_BINDING_1D, 0, &bound); + EXPECT_EQ(static_cast(bound), textures[0]) << "glBindTextures is still exported as a no-op stub"; + if (maxImageUnits > 0) { + ::glBindImageTextures(0, 1, &textures[1]); + GetIntegeri_v(GL_IMAGE_BINDING_NAME, 0, &bound); + EXPECT_EQ(static_cast(bound), textures[1]) + << "glBindImageTextures is still exported as a no-op stub"; + } + DrainErrors(); + } + TEST_F(NegativeApiErrorsTest, BufferRangeOffsetAlignmentAppliesToTheBindingPoint) { GLint ssboAlignment = 0; GetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, &ssboAlignment);