[Fix] (Readback): read the cube face a readback names, and let glGetTextureSubImage name one

MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp

@@ -10703,19 +10703,30 @@ void main() {
                         "GetTexImage: failed to materialize pending clear for textureId=%d",
                         textureObject->GetExternalIndex());
 
+        // WHICH FACE the caller asked for. glGetTexImage names one face of a cube map through the
+        // TARGET token (GL_TEXTURE_CUBE_MAP_NEGATIVE_X and friends, GL 4.6 core 8.11), and a cube
+        // map's six faces are its VkImage's six ARRAY LAYERS - so unless the token is turned into a
+        // baseArrayLayer, every face token reads layer 0 and the whole cube answers as +X. The
+        // image's own target cannot supply this: a plain GL_TEXTURE_CUBE_MAP is not an array target,
+        // so the layer arithmetic below leaves it at one layer starting at zero, which is precisely
+        // the layer this face index has to displace. Same conversion, same reason, as
+        // VkClearManager's / VkRenderPassManager's ResolveAttachmentBaseArrayLayer, which resolve an
+        // ATTACHMENT's face; this is the readback's copy of it. Zero for every other target,
+        // including a cube map ARRAY - that one arrives as TextureUploadTarget::CubeMapArray with
+        // its layer-faces already counted in the level's z, not as a face token.
+        const Bool isCubeFaceTarget = textureUploadTarget >= TextureUploadTarget::CubeMapPositiveX &&
+            textureUploadTarget <= TextureUploadTarget::CubeMapNegativeZ;
+        const Int glCubeFaceLayer = isCubeFaceTarget
+            ? static_cast<Int>(textureUploadTarget) - static_cast<Int>(TextureUploadTarget::CubeMapPositiveX)
+            : 0;
+
         if ((resource->aspect & VK_IMAGE_ASPECT_COLOR_BIT) == 0) {
             if (format == GL_DEPTH_COMPONENT || format == GL_DEPTH_STENCIL || format == GL_STENCIL_INDEX) {
                 const auto levelSize =
                     textureMipmapObject->GetMipmapTexelSize(textureUploadTarget, static_cast<Uint>(level));
-                const Bool isCubeFace = textureUploadTarget >= TextureUploadTarget::CubeMapPositiveX &&
-                    textureUploadTarget <= TextureUploadTarget::CubeMapNegativeZ;
                 // Storage space: `resource` is the storage texture's, so a view's level and
                 // layer have to be shifted into its numbering (see ToStorageMipLevel).
-                const Int glArrayLayer = isCubeFace
+                const Uint32 arrayLayer = ToStorageArrayLayer(textureObject.get(), glCubeFaceLayer);
-                    ? static_cast<Int>(textureUploadTarget) -
-                        static_cast<Int>(TextureUploadTarget::CubeMapPositiveX)
-                    : 0;
-                const Uint32 arrayLayer = ToStorageArrayLayer(textureObject.get(), glArrayLayer);
                 const Uint32 storageLevel = ToStorageMipLevel(textureObject.get(), level);
                 // A 1D array's levelSize.y() is its LAYER count, and those layers are the rows
                 // GL wants back - but in Vulkan they are array layers of a one-row image, not
@@ -10810,7 +10821,10 @@ void main() {
         // Storage space, as above: a texture view reads its own level 0 out of whichever level
         // and layer of the parent it opened onto.
         copyRegion.imageSubresource.mipLevel = ToStorageMipLevel(textureObject.get(), level);
-        copyRegion.imageSubresource.baseArrayLayer = ToStorageArrayLayer(textureObject.get(), 0);
+        // glCubeFaceLayer, not 0: the cube face the target token named (see above). Non-zero for
+        // exactly one shape - a plain cube map read one face at a time - and layerCount is 1 there,
+        // so the copy stays inside the six layers the image has.
+        copyRegion.imageSubresource.baseArrayLayer = ToStorageArrayLayer(textureObject.get(), glCubeFaceLayer);
         copyRegion.imageSubresource.layerCount = static_cast<Uint32>(arrayLayers);
         copyRegion.imageExtent = {static_cast<Uint32>(width),
                                   is1dArrayImage ? 1u : static_cast<Uint32>(height),

MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp

@@ -5078,9 +5078,15 @@ namespace MobileGL::MG_Impl::GLImpl {
     // 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.
+    // `imagesQueried` is how many of the texture's upload-target images the query hands back, and
+    // exists for the destination-size check at the bottom. Zero means "all of them", which is what
+    // the whole-level forms return - every face of a cube map. glGetTextureSubImage naming ONE cube
+    // face passes 1: sizing that request against six faces' worth would reject the only buffer a
+    // single-face read has any reason to pass.
     Bool ValidateTextureImageQuery(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject, GLint level,
                                    TextureInputFormat textureInputFormat, TexturePixelDataType texturePixelDataType,
-                                   GLsizei bufSize, const void* pixels, const char* caller) {
+                                   GLsizei bufSize, const void* pixels, const char* caller,
+                                   SizeT imagesQueried = 0) {
         if (!TextureImpl::ValidateTextureObject(textureObject)) {
             MG_State::pGLContext->RecordError(
                 ErrorCode::InvalidOperation,
@@ -5196,12 +5202,14 @@ namespace MobileGL::MG_Impl::GLImpl {
                 return false;
             }
 
-            // Tightly packed, and summed over every face because a cube map query returns all
+            // Tightly packed, and summed over every face because a whole-level cube map query
-            // six. Pack pixel-store state only ever grows this, so a request rejected here
+            // returns all six - unless the caller named a single face, which is what a non-zero
-            // could not have fit under any packing.
+            // imagesQueried says. Pack pixel-store state only ever grows this, so a request
+            // rejected here could not have fit under any packing.
+            const SizeT imageCount = imagesQueried != 0 ? imagesQueried : uploadTargets.size();
             const SizeT required = MG_Util::CalculateInputTextureImageSize(textureInputFormat,
                                                                            texturePixelDataType, texelSize) *
-                                   uploadTargets.size();
+                                   imageCount;
 
             if (bufSize >= 0 && static_cast<SizeT>(bufSize) < required) {
                 MG_State::pGLContext->RecordError(
@@ -6423,6 +6431,23 @@ namespace MobileGL::MG_Impl::GLImpl {
         }
     }
 
+    // The half glGetTextureImage and glGetTextureSubImage share: which of the two readbacks answers,
+    // for ONE named upload target. Factored out so the sub-image form can name a cube FACE - the
+    // by-name spelling of the face token glGetTexImage takes - instead of re-deriving the target and
+    // silently landing on the +X face the way the delegation it replaces did.
+    static void GetTextureImageForUploadTarget(const SharedPtr<MG_State::GLState::ITextureObject>& textureObject,
+                                               TextureUploadTarget uploadTarget, GLint level, GLenum format,
+                                               GLenum type, GLsizei bufSize, void* pixels, const char* caller) {
+        if (MG_Backend::pActiveBackendObject != nullptr &&
+            MG_Backend::pActiveBackendObject->GetBackendType() == BackendType::DirectVulkan &&
+            MG_Backend::gBackendFunctionsTable.GL.GetTextureImage != nullptr) {
+            MG_Backend::gBackendFunctionsTable.GL.GetTextureImage(textureObject, uploadTarget, level, format, type,
+                                                                  bufSize, pixels);
+            return;
+        }
+        CopyTextureImageToClientOrPBO_State(textureObject, uploadTarget, level, format, type, bufSize, pixels, caller);
+    }
+
     void GetTextureImage(GLuint texture, GLint level, GLenum format, GLenum type, GLsizei bufSize, void* pixels) {
         auto textureObject = GetTextureObjectByName(texture, __func__);
         if (!textureObject) return;
@@ -6431,16 +6456,8 @@ namespace MobileGL::MG_Impl::GLImpl {
                                        __func__)) {
             return;
         }
-        const auto uploadTarget = GetPrimaryUploadTarget(textureObject);
+        GetTextureImageForUploadTarget(textureObject, GetPrimaryUploadTarget(textureObject), level, format, type,
-        if (MG_Backend::pActiveBackendObject != nullptr &&
+                                       bufSize, pixels, __func__);
-            MG_Backend::pActiveBackendObject->GetBackendType() == BackendType::DirectVulkan &&
-            MG_Backend::gBackendFunctionsTable.GL.GetTextureImage != nullptr) {
-            MG_Backend::gBackendFunctionsTable.GL.GetTextureImage(textureObject, uploadTarget, level, format, type,
-                                                                  bufSize, pixels);
-            return;
-        }
-        CopyTextureImageToClientOrPBO_State(textureObject, uploadTarget, level, format, type, bufSize, pixels,
-                                            __func__);
     }
 
     void GetCompressedTextureImage(GLuint texture, GLint level, GLsizei bufSize, void* pixels) {
@@ -6484,9 +6501,19 @@ namespace MobileGL::MG_Impl::GLImpl {
         }
 
         const auto texelSize = textureMipmapObject->GetMipmapTexelSize(uploadTarget, static_cast<Uint>(level));
-        const Bool isFullLevelRead = xoffset == 0 && yoffset == 0 && zoffset == 0 &&
+        // On a cube map, z is the FACE axis. A cube map's level is stored per face, so its level
-                                     width == texelSize.x() && height == texelSize.y() &&
+        // size reads z = 1 whichever face named it - but GL 4.6 core 8.11.4 addresses the six faces
-                                     depth == texelSize.z();
+        // of a cube map through zoffset/depth, exactly the six layers a face token names for
+        // glGetTexImage. Without this arm the z range was measured against that 1 and only zoffset 0
+        // (the +X face) was expressible; the other five were rejected as a partial read.
+        //
+        // Only ONE face at a time. depth > 1 would have to concatenate faces into the destination,
+        // which is the same unimplemented multi-image packing the check below still refuses.
+        const Bool isSingleCubeFaceRead = textureObject->GetTarget() == TextureTarget::TextureCubeMap &&
+                                          depth == 1 && zoffset < 6;
+        const Bool isFullLevelRead = xoffset == 0 && yoffset == 0 && width == texelSize.x() &&
+                                     height == texelSize.y() &&
+                                     (isSingleCubeFaceRead || (zoffset == 0 && depth == texelSize.z()));
         if (!isFullLevelRead) {
             MG_State::pGLContext->RecordError(
                 ErrorCode::InvalidOperation,
@@ -6495,7 +6522,17 @@ namespace MobileGL::MG_Impl::GLImpl {
             return;
         }
 
-        GetTextureImage(texture, level, format, type, bufSize, pixels);
+        const TextureUploadTarget readUploadTarget =
+            isSingleCubeFaceRead ? static_cast<TextureUploadTarget>(
+                                       static_cast<Int>(TextureUploadTarget::CubeMapPositiveX) + zoffset)
+                                 : uploadTarget;
+        if (!ValidateTextureImageQuery(textureObject, level, MG_Util::ConvertGLEnumToTextureInputFormat(format),
+                                       MG_Util::ConvertGLEnumToTexturePixelDataType(type), bufSize, pixels, __func__,
+                                       isSingleCubeFaceRead ? 1u : 0u)) {
+            return;
+        }
+        GetTextureImageForUploadTarget(textureObject, readUploadTarget, level, format, type, bufSize, pixels,
+                                       __func__);
     }
 
     // A buffer texture carries none of the sampler or level state these queries report. Reached by