From 817091641c2b4d06a6f640635a3bd81e92605718 Mon Sep 17 00:00:00 2001 From: BZLZHH Date: Wed, 5 Aug 2026 05:24:21 -0400 Subject: [PATCH] [Fix] (MG_Impl): give a cube map the storage and the layered attachment it asks for direct_state_access.framebuffers_texture_attachment threw on both backends, and three separate things were wrong on the way to a cube map framebuffer. glTexStorage1D/2D/3D validated their target by converting it to a single TextureUploadTarget. GL_TEXTURE_CUBE_MAP has no single upload target - it allocates all six faces - so the conversion produced Unknown and a legal glTexStorage2D(GL_TEXTURE_CUBE_MAP, ...) was rejected with INVALID_ENUM, which is where the case threw. The accepted set for these entry points is the dimension's storage targets, which IsTextureStorageTargetForDimension already spells out, so that is what they check now. TextureStorage2D then allocated only the primary upload target, leaving a cube map with one face out of six - cube-incomplete, so every framebuffer it was attached to answered GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT. It allocates every upload target the object has; for every other 2D target that is the same single target as before. ResolveRepresentableFramebufferTextureUploadTarget declined every layered target but 2D array, so glNamedFramebufferTexture on a cube map reported "not represented by the current framebuffer attachment model". Cube maps, cube map arrays, 1D arrays, 2D multisample arrays and 3D textures are all the same shape as the 2D array that already worked - glFramebufferTexture binds the whole texture and the attachment records a representative upload target - so they are all handled now. DirectGLES routes a layered attachment to glFramebufferTexture, which is exactly this. Takes framebuffers_texture_attachment from failing to passing on both backends. --- .../GLImpl/Framebuffer/GL_Framebuffer.cpp | 26 +++++++- .../MG_Impl/GLImpl/Texture/GL_Texture.cpp | 65 +++++++++++++++---- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp index 077e58c3..95d1e1bc 100644 --- a/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp +++ b/MobileGL/MG_Impl/GLImpl/Framebuffer/GL_Framebuffer.cpp @@ -435,12 +435,36 @@ namespace MobileGL::MG_Impl::GLImpl { case TextureTarget::Texture2DMultisample: outUploadTarget = TextureUploadTarget::Texture2DMultisample; return true; + // Everything below attaches as a LAYERED attachment (GL 4.6 core 9.2.8): glFramebufferTexture + // on one of these binds the whole texture, not one image, and the upload target named here is + // only the representative the attachment model records - the first face for a cube map, the + // whole array otherwise. DirectGLES routes a layered attachment to glFramebufferTexture, which + // is exactly this. case TextureTarget::Texture2DArray: outUploadTarget = TextureUploadTarget::Texture2DArray; outLayered = true; return true; + case TextureTarget::Texture1DArray: + outUploadTarget = TextureUploadTarget::Texture1DArray; + outLayered = true; + return true; + case TextureTarget::Texture2DMultisampleArray: + outUploadTarget = TextureUploadTarget::Texture2DMultisampleArray; + outLayered = true; + return true; + case TextureTarget::Texture3D: + outUploadTarget = TextureUploadTarget::Texture3D; + outLayered = true; + return true; + case TextureTarget::TextureCubeMap: + outUploadTarget = TextureUploadTarget::CubeMapPositiveX; + outLayered = true; + return true; + case TextureTarget::TextureCubeMapArray: + outUploadTarget = TextureUploadTarget::CubeMapArray; + outLayered = true; + return true; default: - // TODO: Extend layered framebuffer attachment support to 3D, cube, 1D array, and multisample array textures. outUploadTarget = TextureUploadTarget::Unknown; return false; } diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 3993d3b7..9f24e5a5 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -3736,15 +3736,22 @@ namespace MobileGL::MG_Impl::GLImpl { textureInternalFormat, MG_Util::ConvertGLEnumToTexturePixelDataType(realType)); textureObject->SetInternalFormat(textureInternalFormat); - for (GLsizei level = 0; level < levels; ++level) { - const GLsizei levelWidth = std::max(1, width >> level); - const GLsizei levelHeight = std::max(1, height >> level); - const SizeT byteSize = static_cast(levelWidth) * static_cast(levelHeight) * bytesPerPixel; - textureMipmapObject->AllocateStorage(textureUploadTarget, level, {{levelWidth, levelHeight, 1}, byteSize}); - textureMipmapObject->MarkStorageDirty(textureUploadTarget, level, false); + // A cube map has six upload targets and glTexStorage2D allocates all of them at once (GL 4.6 + // core 8.19). Allocating only the primary one left the object cube-incomplete, so every + // framebuffer it was attached to reported GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT. Every other + // 2D target has exactly one upload target, so this loop is a no-op change for them. + for (const auto uploadTarget : textureObject->GetUploadTargets()) { + for (GLsizei level = 0; level < levels; ++level) { + const GLsizei levelWidth = std::max(1, width >> level); + const GLsizei levelHeight = std::max(1, height >> level); + const SizeT byteSize = + static_cast(levelWidth) * static_cast(levelHeight) * bytesPerPixel; + textureMipmapObject->AllocateStorage(uploadTarget, level, {{levelWidth, levelHeight, 1}, byteSize}); + textureMipmapObject->MarkStorageDirty(uploadTarget, level, false); + } + // See TextureStorage1D. + textureMipmapObject->TruncateMipmapLevels(uploadTarget, static_cast(levels)); } - // See TextureStorage1D. - textureMipmapObject->TruncateMipmapLevels(textureUploadTarget, static_cast(levels)); textureObject->SetImmutableLevels(static_cast(levels)); } @@ -3875,9 +3882,19 @@ namespace MobileGL::MG_Impl::GLImpl { void TexStorage1D(GLenum target, GLsizei levels, GLenum internalformat, GLsizei width) { const auto textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); - const auto textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target); if (!TextureImpl::ValidateTextureTarget(textureTarget)) return; - if (!TextureImpl::ValidateTextureUploadTarget(textureUploadTarget)) return; + // Not ValidateTextureUploadTarget: GL_TEXTURE_CUBE_MAP is a legal glTexStorage2D target but + // has no single upload target - it allocates all six faces - so validating one would reject + // it. The accepted set for this entry point is the dimension's storage targets, and the + // by-name form below does the per-face work. + if (!IsTextureStorageTargetForDimension(textureTarget, 1)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + std::format("Target {} does not take 1D immutable storage.", + MG_Util::ConvertGLEnumToString(target)))); + return; + } auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); auto& bindingSlot = activeUnit.GetBindingSlot(textureTarget); @@ -3890,9 +3907,19 @@ namespace MobileGL::MG_Impl::GLImpl { void TexStorage2D(GLenum target, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height) { const auto textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); - const auto textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target); if (!TextureImpl::ValidateTextureTarget(textureTarget)) return; - if (!TextureImpl::ValidateTextureUploadTarget(textureUploadTarget)) return; + // Not ValidateTextureUploadTarget: GL_TEXTURE_CUBE_MAP is a legal glTexStorage2D target but + // has no single upload target - it allocates all six faces - so validating one would reject + // it. The accepted set for this entry point is the dimension's storage targets, and the + // by-name form below does the per-face work. + if (!IsTextureStorageTargetForDimension(textureTarget, 2)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + std::format("Target {} does not take 2D immutable storage.", + MG_Util::ConvertGLEnumToString(target)))); + return; + } auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); auto& bindingSlot = activeUnit.GetBindingSlot(textureTarget); @@ -3906,9 +3933,19 @@ namespace MobileGL::MG_Impl::GLImpl { void TexStorage3D(GLenum target, GLsizei levels, GLenum internalformat, GLsizei width, GLsizei height, GLsizei depth) { const auto textureTarget = MG_Util::ConvertGLEnumToTextureTarget(target); - const auto textureUploadTarget = MG_Util::ConvertGLEnumToTextureUploadTarget(target); if (!TextureImpl::ValidateTextureTarget(textureTarget)) return; - if (!TextureImpl::ValidateTextureUploadTarget(textureUploadTarget)) return; + // Not ValidateTextureUploadTarget: GL_TEXTURE_CUBE_MAP is a legal glTexStorage2D target but + // has no single upload target - it allocates all six faces - so validating one would reject + // it. The accepted set for this entry point is the dimension's storage targets, and the + // by-name form below does the per-face work. + if (!IsTextureStorageTargetForDimension(textureTarget, 3)) { + MG_State::pGLContext->RecordError( + ErrorCode::InvalidEnum, + MakeUnique("MG_Impl/GLImpl", __func__, + std::format("Target {} does not take 3D immutable storage.", + MG_Util::ConvertGLEnumToString(target)))); + return; + } auto& activeUnit = MG_State::pGLContext->GetTextureUnitObject(MG_State::pGLContext->GetActiveTextureUnit()); auto& bindingSlot = activeUnit.GetBindingSlot(textureTarget);