diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 4b1755bb..7038a258 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/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(textureUploadTarget) - static_cast(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(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 - ? static_cast(textureUploadTarget) - - static_cast(TextureUploadTarget::CubeMapPositiveX) - : 0; - const Uint32 arrayLayer = ToStorageArrayLayer(textureObject.get(), glArrayLayer); + const Uint32 arrayLayer = ToStorageArrayLayer(textureObject.get(), glCubeFaceLayer); 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(arrayLayers); copyRegion.imageExtent = {static_cast(width), is1dArrayImage ? 1u : static_cast(height), diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index dfbec45d..9dc40996 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/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& 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 - // six. Pack pixel-store state only ever grows this, so a request rejected here - // could not have fit under any packing. + // Tightly packed, and summed over every face because a whole-level cube map query + // returns all six - unless the caller named a single face, which is what a non-zero + // 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(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& 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); - 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, - __func__); + GetTextureImageForUploadTarget(textureObject, GetPrimaryUploadTarget(textureObject), 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(level)); - const Bool isFullLevelRead = xoffset == 0 && yoffset == 0 && zoffset == 0 && - width == texelSize.x() && height == texelSize.y() && - depth == texelSize.z(); + // On a cube map, z is the FACE axis. A cube map's level is stored per face, so its level + // size reads z = 1 whichever face named it - but GL 4.6 core 8.11.4 addresses the six faces + // 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( + static_cast(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