[Feat] (MG_Impl): give the by-name texture image queries their error set

glGetTextureImage resolved a texture by name and went straight to the read, skipping every object-level rule glGetTexImage enforces through GetTexImage_State - and on DirectVulkan it skipped the level checks in CopyTextureImageToClientOrPBO_State as well, because that backend answers GetTextureImage itself. Fifteen of the sixteen conditions in direct_state_access.textures_image_query_errors went unreported. The object-level half of that error set now lives in ValidateTextureImageQuery and both entry points run it. Three rules are new rather than merely relocated: - Multisample and buffer textures are not in the accepted target list; neither has a single image to return. - The destination-size checks (bufSize, and the span written into a bound pixel pack buffer) move ahead of the read. They existed, but downstream of it, where any early bail-out - an unmapped level, a pack step that declines the format - swallowed them. Both measure the tightly packed span summed over the object's faces, which is the least a query can produce, so nothing that would have fit is rejected. - IsDepthLikeInternalFormat had no case for StencilIndex8, so a colour client format read back against a stencil-only texture looked like a matching pair. glGetCompressedTextureImage was a do-nothing stub. It validates the name and the level, then reports INVALID_OPERATION: no format MobileGL can hold is compressed, and answering GL_NO_ERROR without writing would hand the caller stale memory - the same reasoning GetCompressedTexImage_State already follows. Takes direct_state_access.textures_image_query_errors from failing to passing on both backends.
2026-09-08 04:08:32 +09:00 · 2026-08-05 01:46:10 -04:00
parent 7d6f6603c1
commit 31ea6aa5a3
4 changed files with 159 additions and 62 deletions
@@ -1074,7 +1074,7 @@ DECLARE_GL_FUNCTION_HEAD(void, TextureParameteriv, GLuint texture, GLenum pname,
 DECLARE_GL_FUNCTION_HEAD(void, GenerateTextureMipmap, GLuint texture) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GenerateTextureMipmap, texture)
 DECLARE_GL_FUNCTION_HEAD(void, BindTextureUnit, GLuint unit, GLuint texture) DECLARE_GL_FUNCTION_END_NO_RETURN(void, BindTextureUnit, unit, texture)
 DECLARE_GL_FUNCTION_HEAD(void, GetTextureImage, GLuint texture, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetTextureImage, texture, level, format, type, bufSize, pixels)
-DECLARE_GL_FUNCTION_STUB_HEAD(void, GetCompressedTextureImage, GLuint texture, GLint level, GLsizei bufSize, void* pixels) DECLARE_GL_FUNCTION_STUB_END_NO_RETURN(void, GetCompressedTextureImage, texture, level, bufSize, pixels)
+DECLARE_GL_FUNCTION_HEAD(void, GetCompressedTextureImage, GLuint texture, GLint level, GLsizei bufSize, void* pixels) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetCompressedTextureImage, texture, level, bufSize, pixels)
 DECLARE_GL_FUNCTION_HEAD(void, GetTextureLevelParameterfv, GLuint texture, GLint level, GLenum pname, GLfloat* params) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetTextureLevelParameterfv, texture, level, pname, params)
 DECLARE_GL_FUNCTION_HEAD(void, GetTextureLevelParameteriv, GLuint texture, GLint level, GLenum pname, GLint* params) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetTextureLevelParameteriv, texture, level, pname, params)
 DECLARE_GL_FUNCTION_HEAD(void, GetTextureParameterfv, GLuint texture, GLenum pname, GLfloat* params) DECLARE_GL_FUNCTION_END_NO_RETURN(void, GetTextureParameterfv, texture, pname, params)
@@ -3315,6 +3315,136 @@ namespace MobileGL::MG_Impl::GLImpl {
    }

    // Add to GL_Texture.cpp
+    // The half of the GetTexImage/GetTextureImage error set (GL 4.6 core 8.11) that depends on the
+    // resolved texture object rather than on how it was named. Shared because the by-name entry
+    // point does not route through GetTexImage_State and so used to enforce none of it.
+    Bool ValidateTextureImageQuery(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject, GLint level,
+                                   TextureInputFormat textureInputFormat, TexturePixelDataType texturePixelDataType,
+                                   GLsizei bufSize, const void* pixels, const char* caller) {
+        if (!TextureImpl::ValidateTextureObject(textureObject)) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidOperation,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "No valid texture bound to target"));
+            return false;
+        }
+
+        // A multisample texture has per-sample data with no single image to return, and a buffer
+        // texture's data lives in the buffer object - neither target is in the accepted list.
+        const auto target = textureObject->GetTarget();
+        if (target == TextureTarget::Texture2DMultisample || target == TextureTarget::Texture2DMultisampleArray ||
+            target == TextureTarget::TextureBuffer) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidOperation,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                             "Texture target has no image to read back."));
+            return false;
+        }
+
+        // Level range. The by-target path would reach these again inside
+        // CopyTextureImageToClientOrPBO_State, but the by-name path on a backend that answers
+        // GetTextureImage itself never gets there.
+        if (!TextureImpl::ValidateTextureLevelNumber(level)) return false;
+        if (target == TextureTarget::TextureRectangle && level != 0) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidValue,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                             "Level must be zero for rectangle textures"));
+            return false;
+        }
+
+        // For a cube map this is exactly cube completeness: IsComplete() wants all six faces.
+        if (!textureObject->IsComplete()) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidOperation,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "Texture is incomplete"));
+            return false;
+        }
+
+        // Check PBO state
+        const auto& pixelPackBufferObject =
+            MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject();
+
+        if (pixelPackBufferObject) {
+            // Check if PBO is mapped
+            if (pixelPackBufferObject->IsMapped()) {
+                MG_State::pGLContext->RecordError(
+                    ErrorCode::InvalidOperation,
+                    MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "Pixel pack buffer is currently mapped"));
+                return false;
+            }
+
+            // Check alignment
+            const SizeT typeSize = MG_Util::GetTexturePixelDataTypeSize(texturePixelDataType);
+            if (typeSize != 0 && reinterpret_cast<uintptr_t>(pixels) % typeSize != 0) {
+                MG_State::pGLContext->RecordError(
+                    ErrorCode::InvalidOperation,
+                    MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                                 "Pixel data not aligned for pixel pack buffer"));
+                return false;
+            }
+        }
+
+        // Shared format/type/internal-format matrix (packed-type pairing, depth-vs-color mismatch,
+        // integer-ness). Also rejects STENCIL_INDEX readback, which needs GL_ARB_texture_stencil8
+        // (not advertised by MobileGL).
+        if (!TextureImpl::ValidateTextureInternalFormatCompatibleWithInput(
+                textureInputFormat, textureObject->GetFormat(), texturePixelDataType)) {
+            return false;
+        }
+
+        // GetTexImage-specific: DEPTH_STENCIL readback needs a depth-stencil texture (a depth-only
+        // texture has no stencil data to return).
+        if (textureInputFormat == TextureInputFormat::DepthStencil &&
+            textureObject->GetFormat() != TextureInternalFormat::DepthStencil &&
+            textureObject->GetFormat() != TextureInternalFormat::Depth24Stencil8 &&
+            textureObject->GetFormat() != TextureInternalFormat::Depth32FStencil8) {
+            MG_State::pGLContext->RecordError(
+                ErrorCode::InvalidOperation,
+                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                             "DEPTH_STENCIL readback requires a depth-stencil texture"));
+            return false;
+        }
+
+        // The destination has to be big enough. This has to happen here rather than after the read
+        // has been packed: any of the reasons the read can bail out early - an unmapped level, a
+        // pack step that declines the format - would otherwise swallow the error entirely.
+        if (textureObject->GetStorageType() == TextureStorageType::Mipmap) {
+            const auto* textureMipmapObject =
+                static_cast<const MG_State::GLState::TextureObjectMipmap*>(textureObject.get());
+            const auto& uploadTargets = textureObject->GetUploadTargets();
+            if (!uploadTargets.empty() && static_cast<Uint>(level) < textureMipmapObject->GetMipmapLevelCount()) {
+                // Tightly packed, and summed over every face because a cube map query returns all
+                // six. Pack pixel-store state only ever grows this, so a request rejected here
+                // could not have fit under any packing.
+                const auto texelSize = textureMipmapObject->GetMipmapTexelSize(uploadTargets[0], level);
+                const SizeT required = MG_Util::CalculateInputTextureImageSize(textureInputFormat,
+                                                                               texturePixelDataType, texelSize) *
+                                       uploadTargets.size();
+
+                if (bufSize >= 0 && static_cast<SizeT>(bufSize) < required) {
+                    MG_State::pGLContext->RecordError(
+                        ErrorCode::InvalidOperation,
+                        MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller, "Destination buffer is too small."));
+                    return false;
+                }
+
+                if (pixelPackBufferObject) {
+                    const SizeT bufferSize = pixelPackBufferObject->GetSize();
+                    const SizeT offset = reinterpret_cast<SizeT>(pixels);
+                    if (offset > bufferSize || required > bufferSize - offset) {
+                        MG_State::pGLContext->RecordError(
+                            ErrorCode::InvalidOperation,
+                            MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", caller,
+                                                         "Packing would write past the end of the pixel pack buffer."));
+                        return false;
+                    }
+                }
+            }
+        }
+
+        return true;
+    }
+
    Bool GetTexImage_State(GLenum target, GLint level, GLenum format, GLenum type, GLvoid* pixels) {
        // ======================= Converting ================================
        TextureUploadTarget textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target);
@@ -3363,67 +3493,9 @@ namespace MobileGL::MG_Impl::GLImpl {
            isProxy ? TextureImpl::pProxyTextureManager->GetProxyTextureObject(textureUploadTarget)
                    : bindingSlot.GetBoundObject();

-        if (!TextureImpl::ValidateTextureObject(textureObject)) {
-            MG_State::pGLContext->RecordError(ErrorCode::InvalidOperation,
-                                              MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "GetTexImage_State",
-                                                                           "No valid texture bound to target"));
-            return false;
-        }
-
-        // Check texture completeness
-        if (!textureObject->IsComplete()) {
-            MG_State::pGLContext->RecordError(
-                ErrorCode::InvalidOperation,
-                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "GetTexImage_State", "Texture is incomplete"));
-            return false;
-        }
-
-        // Check PBO state
-        const auto& pixelPackBufferObject =
-            MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject();
-
-        if (pixelPackBufferObject) {
-            // Check if PBO is mapped
-            if (pixelPackBufferObject->IsMapped()) {
-                MG_State::pGLContext->RecordError(
-                    ErrorCode::InvalidOperation, MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "GetTexImage_State",
-                                                                              "Pixel pack buffer is currently mapped"));
-                return false;
-            }
-
-            // Check alignment
-            const SizeT typeSize = MG_Util::GetTexturePixelDataTypeSize(texturePixelDataType);
-            if (reinterpret_cast<uintptr_t>(pixels) % typeSize != 0) {
-                MG_State::pGLContext->RecordError(
-                    ErrorCode::InvalidOperation,
-                    MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "GetTexImage_State",
-                                                 "Pixel data not aligned for pixel pack buffer"));
-                return false;
-            }
-        }
-
-        // Shared format/type/internal-format matrix (packed-type pairing, depth-vs-color mismatch,
-        // integer-ness). Also rejects STENCIL_INDEX readback, which needs GL_ARB_texture_stencil8
-        // (not advertised by MobileGL).
-        if (!TextureImpl::ValidateTextureInternalFormatCompatibleWithInput(
-                textureInputFormat, textureObject->GetFormat(), texturePixelDataType)) {
-            return false;
-        }
-
-        // GetTexImage-specific: DEPTH_STENCIL readback needs a depth-stencil texture (a depth-only
-        // texture has no stencil data to return).
-        if (textureInputFormat == TextureInputFormat::DepthStencil &&
-            textureObject->GetFormat() != TextureInternalFormat::DepthStencil &&
-            textureObject->GetFormat() != TextureInternalFormat::Depth24Stencil8 &&
-            textureObject->GetFormat() != TextureInternalFormat::Depth32FStencil8) {
-            MG_State::pGLContext->RecordError(
-                ErrorCode::InvalidOperation,
-                MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "GetTexImage_State",
-                                             "DEPTH_STENCIL readback requires a depth-stencil texture"));
-            return false;
-        }
-
-        return true;
+        // glGetTexImage has no bufSize argument: -1 stands for "no client-side limit".
+        return ValidateTextureImageQuery(textureObject, level, textureInputFormat, texturePixelDataType, -1, pixels,
+                                         "GetTexImage_State");
    }

    void CopyTextureImageToClientOrPBO_State(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject,
@@ -4029,6 +4101,11 @@ namespace MobileGL::MG_Impl::GLImpl {
    void GetTextureImage(GLuint texture, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels) {
        auto textureObject = GetTextureObjectByName(texture, __func__);
        if (!textureObject) return;
+        if (!ValidateTextureImageQuery(textureObject, level, MG_Util::ConvertGLEnumToTextureInputFormat(format),
+                                       MG_Util::ConvertGLEnumToTexturePixelDataType(type), bufSize, pixels,
+                                       __func__)) {
+            return;
+        }
        const auto uploadTarget = GetPrimaryUploadTarget(textureObject);
        if (MG_Backend::pActiveBackendObject != nullptr &&
            MG_Backend::pActiveBackendObject->GetBackendType() == BackendType::DirectVulkan &&
@@ -4041,6 +4118,22 @@ namespace MobileGL::MG_Impl::GLImpl {
                                            __func__);
    }

+    void GetCompressedTextureImage(GLuint texture, GLint level, GLsizei bufSize, void* pixels) {
+        auto textureObject = GetTextureObjectByName(texture, __func__);
+        if (!textureObject) return;
+        // Level first: GL 4.6 core 8.11 wants INVALID_VALUE for an out-of-range level even when the
+        // texture would also fail the compressed check below.
+        if (!TextureImpl::ValidateTextureLevelNumber(level)) return;
+
+        // No texture MobileGL holds is compressed (see IsCompressedTextureFormat), so this is the
+        // only outcome today. Reporting success while writing nothing would hand the caller stale
+        // memory with GL_NO_ERROR - the same reasoning as GetCompressedTexImage_State.
+        MG_State::pGLContext->RecordError(
+            ErrorCode::InvalidOperation,
+            MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", __func__,
+                                         "Texture level is not stored in a compressed format."));
+    }
+
    void GetTextureSubImage(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLsizei width,
                            GLsizei height, GLsizei depth, GLenum format, GLenum type, GLsizei bufSize, void* pixels) {
        auto textureObject = GetTextureObjectByName(texture, __func__);
@@ -46,6 +46,7 @@ namespace MobileGL::MG_Impl::GLImpl {
    void GenerateTextureMipmap(GLuint texture);
    void BindTextureUnit(GLuint unit, GLuint texture);
    void GetTextureImage(GLuint texture, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels);
+    void GetCompressedTextureImage(GLuint texture, GLint level, GLsizei bufSize, void* pixels);
    void TexBufferRange(GLenum target, GLenum internalformat, GLuint buffer, GLintptr offset, GLsizeiptr size);
    void TextureBuffer(GLuint texture, GLenum internalformat, GLuint buffer);
    void TextureBufferRange(GLuint texture, GLenum internalformat, GLuint buffer, GLintptr offset, GLsizeiptr size);
@@ -226,6 +226,9 @@ namespace MobileGL::MG_Impl::GLImpl::TextureImpl {
            case TextureInternalFormat::Depth24Stencil8:
            case TextureInternalFormat::Depth32FStencil8:
            case TextureInternalFormat::DepthStencil:
+            // Stencil-only is not a colour format either: a colour client format read against a
+            // STENCIL_INDEX8 texture has to be the same INVALID_OPERATION as against a depth one.
+            case TextureInternalFormat::StencilIndex8:
                return true;
            default:
                return false;