diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index eb0d9d7d..2dbc8753 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -5906,31 +5906,19 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!out.texture) return false; const TextureTarget stateTarget = MG_Util::ConvertGLEnumToTextureTarget(appTarget); out.target = TextureImpl::ConvertTextureTargetToBackendGLEnum(stateTarget); - if (stateTarget == TextureTarget::Texture1DArray) { - out.x = x; - out.y = 0; - out.z = y; - return true; - } + // No axis remap for GL_TEXTURE_1D_ARRAY. The frontend STORES a 1D array with its layers + // on y (GetBackendUploadSize moves them across to the ES 2D array's z), but this entry + // point does not ADDRESS it that way: GL 4.6 core 18.3.2 treats every array texture as a + // stack of slices on z and gives a 1D array a height of 1 - exactly the shape the ES 2D + // array has - so GL's (x, 0, layer) and the ES image's (x, 0, layer) already agree. + // Remapping y into z here fetched the wrong slice for every call that spelled the layer + // the way GL defines it. out.x = x; out.y = y; out.z = z; return true; } - // The region extent swaps the same two axes for a 1D array, and does so for whichever side - // of the copy is one - GL forbids a copy whose two endpoints disagree about how many layers - // move, so at most one of the two can be a 1D array only in the degenerate single-layer - // case, where the swap is the identity anyway. - static void ApplyGLESCopyImageExtent(GLenum appSrcTarget, GLenum appDstTarget, GLsizei& height, GLsizei& depth) { - const TextureTarget srcStateTarget = MG_Util::ConvertGLEnumToTextureTarget(appSrcTarget); - const TextureTarget dstStateTarget = MG_Util::ConvertGLEnumToTextureTarget(appDstTarget); - if (srcStateTarget != TextureTarget::Texture1DArray && dstStateTarget != TextureTarget::Texture1DArray) { - return; - } - std::swap(height, depth); - } - static TextureInternalFormat GetCopyImageEndpointFormat(const CopyImageEndpoint& endpoint) { if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->GetInternalFormat(); return endpoint.Texture ? endpoint.Texture->GetFormat() : TextureInternalFormat::Unknown; @@ -6041,9 +6029,10 @@ namespace MobileGL::MG_Backend::DirectGLES { return; } - GLsizei copyHeight = srcHeight; - GLsizei copyDepth = srcDepth; - ApplyGLESCopyImageExtent(srcTarget, dstTarget, copyHeight, copyDepth); + // Verbatim: GL already spells a 1D array's extent the way the ES 2D array it maps onto + // wants it (height 1, layers on depth) - see MakeGLESCopyImageEndpoint. + const GLsizei copyHeight = srcHeight; + const GLsizei copyDepth = srcDepth; const TextureInternalFormat srcFormat = GetCopyImageEndpointFormat(srcEndpoint); const TextureInternalFormat dstFormat = GetCopyImageEndpointFormat(dstEndpoint); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index fd6862be..92faee42 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -8964,6 +8964,7 @@ void main() { outMapping.baseSlice = baseSlice; outMapping.availableSlices = std::max(1u, image.depth >> mipLevel); return true; + case TextureTarget::Texture1DArray: case TextureTarget::Texture2DArray: case TextureTarget::Texture2DMultisampleArray: case TextureTarget::TextureCubeMap: @@ -8971,15 +8972,18 @@ void main() { // A cube map is an array of six faces here (see TryResolveTextureShapeInfo), and GL // numbers its faces on the same z axis an array texture numbers its layers, so both // arrive as a plain layer range. + // + // GL_TEXTURE_1D_ARRAY belongs here too, and needs no remap: this backend STORES it + // as a VK_IMAGE_TYPE_1D image whose layers live in arrayLayers (ToVulkanLevelExtent + // moves the count across), and GL 4.6 core 18.3.2 ADDRESSES it as a stack of slices + // on z with an image height of 1 - so the frontend's y/height are already the 0/1 + // Vulkan requires and the layer lands in baseArrayLayer either way. outMapping.slicesAreDepth = false; outMapping.baseSlice = baseSlice; outMapping.availableSlices = image.arrayLayers; return true; default: - // GL_TEXTURE_1D_ARRAY carries its layers on the Y axis (srcY/srcHeight), which - // would have to be remapped against a Vulkan extent that also has to stay height 1 - // for a VK_IMAGE_TYPE_1D image; GL_TEXTURE_BUFFER has no image at all. Declined - // rather than mis-addressed. + // GL_TEXTURE_BUFFER has no image at all. Declined rather than mis-addressed. return false; } } diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 4ad78d9b..9ee3320d 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -3750,19 +3750,34 @@ namespace MobileGL::MG_Impl::GLImpl { return GetCopyImageLevelSize(endpoint.Texture, uploadTarget, level); } - // How far the region's z axis may reach. It does not mean the same thing on every target - // GL 4.6 core 18.3.2 accepts: on a CUBE MAP it selects among the six faces, which this - // frontend keeps as six separate one-slice upload targets - so the level's own extent - // says 1 and the real bound is 6. A cube-map ARRAY is one upload target whose depth - // already counts layer-faces, and a 1D array carries its layers on y (which is where GL - // puts them for this entry point too), so both are answered by the level extent. - Int GetCopyImageEndpointLayerCount(const MG_Backend::CopyImageEndpoint& endpoint, - const IntVec3& levelSize) { - if (!endpoint.IsRenderbuffer() && endpoint.Texture && - endpoint.Texture->GetTarget() == TextureTarget::TextureCubeMap) { - return 6; + // The per-axis extent of one endpoint's image AS THIS ENTRY POINT ADDRESSES IT, which is + // not always the level extent this frontend stores. + // + // GL 4.6 core 18.3.2 treats EVERY array texture as a stack of slices addressed by z, and + // gives a 1D array an image height of 1. This frontend stores a 1D array the way + // glTexImage2D(GL_TEXTURE_1D_ARRAY, w, layers) writes it instead - layers on y - so the + // two views have to be told apart here. Measuring y against the LAYER count is what let + // srcY = 14 on a 16-wide, 16-layer 1D array come back GL_NO_ERROR + // (KHR-GL43.copy_image.non_existent_mipmap / the src_test_case y variants); the CTS is + // unambiguous about the convention, forcing height = 1 for 1D and 1D_ARRAY and listing + // 1D_ARRAY as multilayer. + // + // A CUBE MAP is the other target whose z bound is not the level extent: this frontend + // keeps its six faces as six separate one-slice upload targets, so the level says 1 and + // the real bound is 6. A cube-map ARRAY is one upload target whose depth already counts + // layer-faces, and every remaining target is answered by the level extent verbatim. + IntVec3 GetCopyImageEndpointRegionBounds(const MG_Backend::CopyImageEndpoint& endpoint, + const IntVec3& levelSize) { + const TextureTarget target = (!endpoint.IsRenderbuffer() && endpoint.Texture) + ? endpoint.Texture->GetTarget() + : TextureTarget::Unknown; + if (target == TextureTarget::TextureCubeMap) { + return {levelSize.x(), levelSize.y(), 6}; } - return std::max(levelSize.z(), 1); + if (target == TextureTarget::Texture1DArray) { + return {levelSize.x(), 1, std::max(levelSize.y(), 1)}; + } + return {levelSize.x(), levelSize.y(), std::max(levelSize.z(), 1)}; } // GL 4.6 core 18.3.2 requires INVALID_VALUE when the region exceeds either image's @@ -3780,9 +3795,9 @@ namespace MobileGL::MG_Impl::GLImpl { // reject a copy GL allows. Every caller has already established that the level // exists and that the image is complete, so this is a belt-and-braces guard. if (levelSize.x() <= 0 || levelSize.y() <= 0) return true; - const Int layers = GetCopyImageEndpointLayerCount(endpoint, levelSize); - if (x >= 0 && y >= 0 && z >= 0 && static_cast(x) + width <= levelSize.x() && - static_cast(y) + height <= levelSize.y() && static_cast(z) + depth <= layers) { + const IntVec3 bounds = GetCopyImageEndpointRegionBounds(endpoint, levelSize); + if (x >= 0 && y >= 0 && z >= 0 && static_cast(x) + width <= bounds.x() && + static_cast(y) + height <= bounds.y() && static_cast(z) + depth <= bounds.z()) { return true; } MG_State::pGLContext->RecordError( @@ -3791,7 +3806,7 @@ namespace MobileGL::MG_Impl::GLImpl { "MG_Impl/GLImpl", "ValidateCopyImageSubData_State", std::format("The {} region [{}, {}, {}] + [{} x {} x {}] does not fit inside the {} x {} x {} " "image.", - endpointName, x, y, z, width, height, depth, levelSize.x(), levelSize.y(), layers))); + endpointName, x, y, z, width, height, depth, bounds.x(), bounds.y(), bounds.z()))); return false; } } // namespace diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 0c17236d..8bdfc511 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -4982,10 +4982,14 @@ TEST_F(TextureTest, CopyImageSubDataCountsCubeMapFacesOnTheZAxis) { ExpectSingleGlError(GL_INVALID_VALUE); } -// The other axis convention: GL puts a 1D ARRAY's layers on y for this entry point (srcY is the -// first layer, srcHeight the layer count), which is also where this frontend keeps them - so the -// level extent answers directly and z stays a single slice. -TEST_F(TextureTest, CopyImageSubDataBoundsA1DArraysLayersOnTheYAxis) { +// The other axis convention, and it is NOT the one this frontend stores. GL 4.6 core 18.3.2 +// treats every array texture as a stack of slices on Z and gives a 1D array an image HEIGHT OF +// ONE (which is exactly what the CTS asserts: it forces height = 1 for GL_TEXTURE_1D_ARRAY and +// lists the target as multilayer). MobileGL keeps a 1D array's layers on y internally - that is +// what glTexImage2D(GL_TEXTURE_1D_ARRAY, w, layers) writes - so this entry point has to convert, +// and measuring srcY against the LAYER count is what let an out-of-range srcY come back +// GL_NO_ERROR (KHR-GL43.copy_image.exceeding_boundaries, the src_test_case y variants). +TEST_F(TextureTest, CopyImageSubDataBoundsA1DArraysLayersOnTheZAxis) { const ScopedTextureBackendFunctionsOverride backendGuard; MG_Backend::gBackendFunctionsTable.GL.CopyImageSubData = RecordCopyImageSubData; g_copyImageSubDataCall = {}; @@ -5006,14 +5010,33 @@ TEST_F(TextureTest, CopyImageSubDataBoundsA1DArraysLayersOnTheYAxis) { GTEST_SKIP() << "this context could not give the 1D arrays storage"; } - MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 3, 0, dstTexture, GL_TEXTURE_1D_ARRAY, - 0, 0, 3, 0, 4, 5, 1); + // 16 wide, 8 layers. Five layers from layer 3 is legal, and it is spelled on z with a + // height of 1 - the layer count rides on srcDepth. + MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 0, 3, dstTexture, GL_TEXTURE_1D_ARRAY, + 0, 0, 0, 3, 4, 1, 5); EXPECT_TRUE(g_copyImageSubDataCall.Called); EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + // One layer past the last one is out of bounds on z. g_copyImageSubDataCall = {}; - MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 4, 0, dstTexture, GL_TEXTURE_1D_ARRAY, - 0, 0, 0, 0, 4, 5, 1); + MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 0, 4, dstTexture, GL_TEXTURE_1D_ARRAY, + 0, 0, 0, 0, 4, 1, 5); + EXPECT_FALSE(g_copyImageSubDataCall.Called); + ExpectSingleGlError(GL_INVALID_VALUE); + + // The image is one texel HIGH whatever its layer count is, so any srcY past 0 is out of + // bounds - this is the KHR-GL43.copy_image case that used to be measured against the 8 + // layers and pass. + g_copyImageSubDataCall = {}; + MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 6, 0, dstTexture, GL_TEXTURE_1D_ARRAY, + 0, 0, 6, 0, 4, 1, 1); + EXPECT_FALSE(g_copyImageSubDataCall.Called); + ExpectSingleGlError(GL_INVALID_VALUE); + + // ... and a height of 1 at y = 0 is the only legal y extent. + g_copyImageSubDataCall = {}; + MG_Impl::GLImpl::CopyImageSubData(srcTexture, GL_TEXTURE_1D_ARRAY, 0, 0, 0, 0, dstTexture, GL_TEXTURE_1D_ARRAY, + 0, 0, 0, 0, 4, 2, 1); EXPECT_FALSE(g_copyImageSubDataCall.Called); ExpectSingleGlError(GL_INVALID_VALUE); }