From 293f64b3c259399bfb69ae29635e61469449751e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 20 Jul 2026 02:02:35 -0400 Subject: [PATCH] [Fix] (MG_Impl, DirectGLES): correct ARB_clear_texture error codes, reject cube maps in CopyTextureSubImage2D, advertise the extension on Espryt, and pin the error contracts with tests --- .../DirectGLES/BackendObject_DirectGLES.cpp | 2 +- .../MG_Impl/GLImpl/Texture/GL_Texture.cpp | 32 ++++++++++++-- MobileGL/MG_Test/Texture/TextureTest.cpp | 43 +++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp index d44b177d..5d1ea97e 100644 --- a/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/BackendObject_DirectGLES.cpp @@ -826,7 +826,7 @@ namespace MobileGL::MG_Backend::DirectGLES { E_GL_ARB_program_interface_query, E_GL_ARB_framebuffer_object, E_GL_EXT_framebuffer_object, E_GL_ARB_depth_texture, E_GL_ARB_buffer_storage, E_GL_ARB_texture_storage, E_GL_ARB_texture_storage_multisample, - E_GL_ARB_direct_state_access, + E_GL_ARB_clear_texture, E_GL_ARB_direct_state_access, E_GL_ARB_multi_draw_indirect, E_GL_ARB_indirect_parameters, E_GL_ARB_shader_draw_parameters, E_GL_ARB_gpu_shader5, E_GL_ARB_multi_bind, E_GL_ARB_shading_language_420pack, E_GL_ARB_vertex_attrib_binding, diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 7094ffa0..c14d4ce8 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -493,8 +493,15 @@ namespace MobileGL::MG_Impl::GLImpl { } auto mipmapTexture = std::static_pointer_cast(textureObject); - if (level < 0 || static_cast(level) >= mipmapTexture->GetMipmapLevelCount()) { + if (level < 0) { RecordClearTextureError(caller, ErrorCode::InvalidValue, + std::format("Texture level {} is negative.", level)); + return nullptr; + } + // ARB_clear_texture: clearing an image that was never defined by TexImage*/ + // TexStorage* is INVALID_OPERATION, not INVALID_VALUE. + if (static_cast(level) >= mipmapTexture->GetMipmapLevelCount()) { + RecordClearTextureError(caller, ErrorCode::InvalidOperation, std::format("Texture level {} is not defined.", level)); return nullptr; } @@ -536,6 +543,12 @@ namespace MobileGL::MG_Impl::GLImpl { return true; } + // Writes the clear into the CPU shadow and marks the whole level dirty, exactly like + // TexSubImage*_State does. Shared limitation of the level-granular shadow sync: the + // shadow does not reflect GPU-side writes (FBO rendering, imageStore), so a PARTIAL + // clear of a GPU-written level re-uploads stale shadow bytes outside the region on + // the next sync. Full-level clears (glClearTexImage, or a sub-clear covering the + // level) rewrite the entire shadow and are always correct. Bool ClearMipmapRegion(const SharedPtr& textureObject, TextureUploadTarget uploadTarget, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, @@ -4025,8 +4038,21 @@ namespace MobileGL::MG_Impl::GLImpl { void CopyTextureSubImage2D(GLuint texture, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height) { auto textureObject = GetTextureObjectByName(texture, __func__); - WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum target) { - CopyTexSubImage2D_Backend(target, level, xoffset, yoffset, x, y, width, height); + if (!textureObject) return; + // GL 4.6 sec. 8.8: the 2D form only accepts these effective targets; cube maps must + // go through CopyTextureSubImage3D with the face as a layer. + const auto target = textureObject->GetTarget(); + if (target != TextureTarget::Texture2D && target != TextureTarget::Texture1DArray && + target != TextureTarget::TextureRectangle) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidOperation, + MakeUnique("MG_Impl/GLImpl", __func__, + "CopyTextureSubImage2D requires a 2D, 1D-array, or " + "rectangle texture.")); + return; + } + WithTemporarilyBoundNamedTexture(textureObject, [&](GLenum glTarget) { + CopyTexSubImage2D_Backend(glTarget, level, xoffset, yoffset, x, y, width, height); }); } diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index e2f32287..b8154c1a 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -318,6 +318,49 @@ TEST_F(TextureTest, CopyTextureSubImage2DUsesNamedObjectAndRestoresBinding) { EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } +TEST_F(TextureTest, CopyTextureSubImage2DRejectsCubeMapTargets) { + const ScopedTextureBackendFunctionsOverride backendGuard; + MG_Backend::gBackendFunctionsTable.GL.CopyTexSubImage2D = RecordCopyTexSubImage2D; + g_copyTexSubImage2DCall = {}; + + GLuint cubeTexture = 0; + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_CUBE_MAP, 1, &cubeTexture); + MG_Impl::GLImpl::CopyTextureSubImage2D(cubeTexture, 0, 0, 0, 0, 0, 1, 1); + + // GL 4.6 sec. 8.8: the 2D form only accepts 2D/1D-array/rectangle effective targets. + EXPECT_FALSE(g_copyTexSubImage2DCall.Called); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_OPERATION)); +} + +TEST_F(TextureTest, ClearTexImageErrorContracts) { + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, 2, 2, 0, + GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + + // Zero texture name is INVALID_OPERATION (ARB_clear_texture). + MG_Impl::GLImpl::ClearTexImage(0, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_OPERATION)); + + // A negative level is INVALID_VALUE... + MG_Impl::GLImpl::ClearTexImage(texture, -1, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_VALUE)); + + // ...but clearing a level that was never defined is INVALID_OPERATION. + MG_Impl::GLImpl::ClearTexImage(texture, 5, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_OPERATION)); + + // A clear region outside the level is INVALID_VALUE. + MG_Impl::GLImpl::ClearTexSubImage(texture, 0, 1, 1, 0, 4, 4, 1, + GL_RGBA, GL_UNSIGNED_BYTE, nullptr); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_VALUE)); + + // An invalid pixel-transfer format is INVALID_ENUM from the shared validators. + MG_Impl::GLImpl::ClearTexImage(texture, 0, GL_NONE, GL_UNSIGNED_BYTE, nullptr); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), static_cast(GL_INVALID_ENUM)); +} + // GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT is float state that must answer every numeric query: GetFloatv // is authoritative and GetIntegerv would otherwise fall through to its INVALID_ENUM default. TEST_F(TextureTest, MaxTextureMaxAnisotropyIsAnsweredFromTheBackendLimit) {