[Fix, Test] (DirectVulkan, GLState): apply a texture view level and layer window at every subresource boundary

MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp

@@ -1038,8 +1038,15 @@ namespace MobileGL::MG_Backend::DirectVulkan {
                         binding);
         const VkFormat reflectedFormat = programObj.storageImageFormatByBinding[binding];
         const Bool useBindingFormat = programObj.storageImageUsesBindingFormatByBinding[binding];
+        // The storage's own VkFormat is the wrong reference for a GL texture view: the view
+        // reinterprets it (GL 4.6 core table 8.21), and it is the VIEW's format the shader's
+        // image declaration was written against. Same correction the sampled path makes above.
+        const VkFormat storageImageSourceFormat =
+            imageBinding.Texture->IsTextureView()
+                ? m_textureManager->ResolveTextureViewWindow(*imageBinding.Texture, *resource).format
+                : resource->format;
         const VkFormat viewFormat = ResolveStorageImageViewFormat(
-            reflectedFormat, imageBinding.Format, resource->format, useBindingFormat);
+            reflectedFormat, imageBinding.Format, storageImageSourceFormat, useBindingFormat);
         if (viewFormat == VK_FORMAT_UNDEFINED) {
             MGLOG_E_ONCE("ResolveStorageImageDescriptor: unsupported glBindImageTexture format=0x%x "
                     "for binding=%u imageUnit=%d textureId=%d bindingPolicy=%s",

MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp

@@ -67,14 +67,23 @@ namespace MobileGL::MG_Backend::DirectVulkan {
     }
 
     static Uint32 ResolveAttachmentBaseArrayLayer(const MG_State::GLState::FramebufferAttachmentObject& attachment) {
+        // Every branch has to go through ToStorageArrayLayer, including the two that name layer 0
+        // implicitly: a layered attachment of a texture VIEW starts at the view's first layer, not
+        // at the image's, and a cube FACE index is a layer index like any other. Leaving either
+        // unshifted made the render pass write layers [0, n) while the clear key, the blit, the
+        // copy and the readback for the same attachment all addressed [minLayer, minLayer + n) -
+        // they resolve the layer through their own copies of this helper, which do shift.
+        const auto* texture = attachment.GetTexture().get();
         if (attachment.IsLayered()) {
-            return 0;
+            return ToStorageArrayLayer(texture, 0);
         }
         const TextureUploadTarget uploadTarget = attachment.GetTextureUploadTarget();
         if (!IsCubeMapFaceUploadTarget(uploadTarget)) {
-            return ToStorageArrayLayer(attachment.GetTexture().get(), attachment.GetTextureLayer());
+            return ToStorageArrayLayer(texture, attachment.GetTextureLayer());
         }
-        return static_cast<Uint32>(uploadTarget) - static_cast<Uint32>(TextureUploadTarget::CubeMapPositiveX);
+        const Int face =
+            static_cast<Int>(uploadTarget) - static_cast<Int>(TextureUploadTarget::CubeMapPositiveX);
+        return ToStorageArrayLayer(texture, face);
     }
 
     static Uint32 ResolveAttachmentLayerCount(const MG_State::GLState::FramebufferAttachmentObject& attachment) {

MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp

@@ -711,6 +711,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
 
     void VkTextureManager::EraseTrackedTexture(const TextureIdentity& identity) {
         m_viewRequestedImageFlags.erase(identity);
+        m_viewRequestedFormats.erase(identity);
         auto resourceIt = m_textureResources.find(identity);
         if (resourceIt != m_textureResources.end()) {
             DeferResourceRelease(Move(resourceIt->second));
@@ -1046,6 +1047,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             .viewType = window.viewType,
             .format = window.format,
             .aspect = window.sampledAspect,
+            .componentSwizzle = PackComponentSwizzle(window.components),
         };
         const auto existing = resource.alternateSampledViews.find(key);
         if (existing != resource.alternateSampledViews.end()) {
@@ -1061,12 +1063,10 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             return VK_NULL_HANDLE;
         }
 
-        const TextureFormatInfo formatInfo = ResolveTextureFormatInfo(texture.GetFormat());
-        const VkComponentMapping sampledComponents = ResolveSampledViewComponents(texture, formatInfo);
         const VkImageView view =
             CreateImageView(resource.image, window.format, window.sampledAspect, window.viewType,
                             window.baseMipLevel, window.levelCount, window.baseArrayLayer, window.layerCount,
-                            &sampledComponents);
+                            &window.components);
         if (view == VK_NULL_HANDLE) {
             MGLOG_E_ONCE("%s: failed to create sampled view for textureId=%d format=%d aspect=0x%x "
                          "mips=[%u,%u) layers=[%u,%u)",
@@ -1126,6 +1126,8 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             .viewType = resource->viewType,
             .format = format,
             .aspect = VK_IMAGE_ASPECT_COLOR_BIT,
+            .componentSwizzle = PackComponentSwizzle(
+                ResolveSampledViewComponents(texture, ResolveTextureFormatInfo(texture.GetFormat()))),
         };
         const auto existing = resource->alternateSampledViews.find(key);
         if (existing != resource->alternateSampledViews.end()) {
@@ -1192,8 +1194,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             return VK_NULL_HANDLE;
         }
 
-        Uint32 baseArrayLayer = 0;
+        // A GL texture view opens onto a WINDOW of the storage's layers; a layered image
-        Uint32 layerCount = resource->arrayLayers;
+        // binding of it must not reach past that window into the parent's other layers.
+        Uint32 baseArrayLayer = ToStorageArrayLayer(&texture, 0);
+        Uint32 layerCount = texture.IsTextureView()
+                                ? std::min(static_cast<Uint32>(texture.GetViewNumLayers()),
+                                           resource->arrayLayers - baseArrayLayer)
+                                : resource->arrayLayers;
         VkImageViewType viewType = resource->viewType;
         if (!layered) {
             switch (resource->viewType) {
@@ -1226,7 +1233,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
 
         const Bool isFullResourceView = baseArrayLayer == 0 && layerCount == resource->arrayLayers &&
                                         viewType == resource->viewType;
-        if (format == resource->format && isFullResourceView) {
+        if (format == resource->format && isFullResourceView && !texture.IsTextureView()) {
             return GetOrCreateViewAtMipLevel(texture, mipLevel);
         }
 
@@ -1755,7 +1762,14 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         // Same shape for a GL texture view's demands on the image (MUTABLE_FORMAT for a
         // format-reinterpreting view, CUBE_COMPATIBLE for a cube view of an array texture):
         // nothing about the texture itself changed, but the live image cannot carry the view.
-        const VkImageCreateFlags requestedViewFlags = GetViewRequestedImageFlags(texture);
+        // Masked by what this format can actually be given: MUTABLE_FORMAT is deliberately
+        // withheld from formats the driver already refused it for (see SyncTextureResource), and
+        // without this mask the "upgrade still pending" test below could never come true again -
+        // costing every later sync of that texture the whole slow path, forever.
+        VkImageCreateFlags requestedViewFlags = GetViewRequestedImageFlags(texture);
+        if (m_mutableFormatUnsupported.find(outResource.format) != m_mutableFormatUnsupported.end()) {
+            requestedViewFlags &= ~VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
+        }
         const Bool viewFlagUpgradePending =
             (outResource.imageCreateFlags & requestedViewFlags) != requestedViewFlags;
         if (outResource.image != VK_NULL_HANDLE && !storageUpgradePending && !viewFlagUpgradePending &&
@@ -2122,6 +2136,11 @@ namespace MobileGL::MG_Backend::DirectVulkan {
                     viewFormats.push_back(viewFormat);
                 }
             }
+            // ...plus every format a glTextureView over this storage reinterprets it as. Those
+            // are NOT enumerable from ResolveSampledImageViewFormat - an application may name any
+            // member of the format's view class (GL 4.6 core table 8.21) - so without this the
+            // list would forbid the very view the MUTABLE_FORMAT bit was requested for.
+            AppendViewRequestedFormats(texture, viewFormats);
             formatListInfo.sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_LIST_CREATE_INFO;
             formatListInfo.viewFormatCount = static_cast<Uint32>(viewFormats.size());
             formatListInfo.pViewFormats = viewFormats.data();
@@ -2589,6 +2608,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         window.layerCount = resource.arrayLayers;
         window.sampledAspect =
             ResolveSampledImageViewAspectMask(resource.aspect, texture.GetDepthStencilTextureMode());
+        window.components = ResolveSampledViewComponents(texture, ResolveTextureFormatInfo(texture.GetFormat()));
         ResolveViewMipRange(texture, resource.mipLevels, window.baseMipLevel, window.levelCount);
         if (!texture.IsTextureView()) {
             return window;
@@ -2642,21 +2662,44 @@ namespace MobileGL::MG_Backend::DirectVulkan {
     // legally be built over, instead of handing back VK_NULL_HANDLE for a frame.
     void VkTextureManager::NoteTextureViewImageRequirements(MG_State::GLState::ITextureObject& viewTexture,
                                                             MG_State::GLState::ITextureObject& storageTexture) {
+        const TextureIdentity storageIdentity = MakeTextureIdentity(&storageTexture);
         VkImageCreateFlags required = 0;
         const VkFormat viewFormat = ResolveTextureFormatInfo(viewTexture.GetFormat()).format;
         const VkFormat storageFormat = ResolveTextureFormatInfo(storageTexture.GetFormat()).format;
         if (viewFormat != VK_FORMAT_UNDEFINED && storageFormat != VK_FORMAT_UNDEFINED &&
             viewFormat != storageFormat) {
             required |= VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
+            // The image may be created with a NARROWED format list (see SyncTextureResource), and
+            // that list is a promise about every format the image will ever be viewed as. Record
+            // this one so the promise stays true.
+            m_viewRequestedFormats[storageIdentity].insert(viewFormat);
         }
         const TextureTarget viewTarget = viewTexture.GetTarget();
         if (viewTarget == TextureTarget::TextureCubeMap || viewTarget == TextureTarget::TextureCubeMapArray) {
+            // Only when the storage could legally carry the bit. VK_IMAGE_CREATE_CUBE_COMPATIBLE
+            // demands a 2D image with square levels and at least six array layers
+            // (VUID-VkImageCreateInfo-flags-00954), and asking for it on a storage that has fewer
+            // would fail vkCreateImage - which, because SyncTextureResource has already released
+            // the old resource by then, would leave the PARENT texture with no image at all. A
+            // degenerate view must not be able to destroy the texture it views; let its own view
+            // creation fail instead.
+            const IntVec3 storageSize = storageTexture.GetBaseSize();
+            const Bool storageCanBeCube = storageSize.x() == storageSize.y() &&
+                                          storageTexture.GetViewNumLayers() >= 6 &&
+                                          storageTexture.GetTarget() != TextureTarget::Texture3D;
+            if (storageCanBeCube) {
                 required |= VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT;
+            } else {
+                MGLOG_W_ONCE("Texture view %d wants a cube view of texture %d, whose storage is %dx%d with %u "
+                             "layers and cannot be cube-compatible; the view will have no image view.",
+                             viewTexture.GetExternalIndex(), storageTexture.GetExternalIndex(), storageSize.x(),
+                             storageSize.y(), storageTexture.GetViewNumLayers());
+            }
         }
         if (required == 0) {
             return;
         }
-        VkImageCreateFlags& stored = m_viewRequestedImageFlags[MakeTextureIdentity(&storageTexture)];
+        VkImageCreateFlags& stored = m_viewRequestedImageFlags[storageIdentity];
         stored |= required;
     }
 
@@ -2667,6 +2710,20 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         return it == m_viewRequestedImageFlags.end() ? 0 : it->second;
     }
 
+    void VkTextureManager::AppendViewRequestedFormats(const MG_State::GLState::ITextureObject& storageTexture,
+                                                      Vector<VkFormat>& outFormats) const {
+        const auto it = m_viewRequestedFormats.find(
+            MakeTextureIdentity(const_cast<MG_State::GLState::ITextureObject*>(&storageTexture)));
+        if (it == m_viewRequestedFormats.end()) {
+            return;
+        }
+        for (const VkFormat viewFormat : it->second) {
+            if (std::find(outFormats.begin(), outFormats.end(), viewFormat) == outFormats.end()) {
+                outFormats.push_back(viewFormat);
+            }
+        }
+    }
+
     Bool VkTextureManager::SyncTextureViews(const MG_State::GLState::ITextureObject& texture, TextureResource& resource) {
         MOBILEGL_ASSERT(resource.image != VK_NULL_HANDLE, "SyncTextureViews: image == VK_NULL_HANDLE");
 

MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h

@@ -173,6 +173,12 @@ public:
             VkImageViewType viewType = VK_IMAGE_VIEW_TYPE_2D;
             VkFormat format = VK_FORMAT_UNDEFINED;
             VkImageAspectFlags aspect = VK_IMAGE_ASPECT_COLOR_BIT;
+            // GL_TEXTURE_SWIZZLE_* is per-texture state, so two views over one storage with the
+            // same window but different swizzles are different views. Baked into the key because
+            // a GL texture view's ONLY sampled view lives in this cache: unlike the storage
+            // texture's own sampledView, which SyncTextureViews rebuilds whenever the params
+            // version moves, nothing else would ever notice a swizzle change on a view.
+            Uint32 componentSwizzle = 0;
 
             Bool operator==(const SampledImageViewKey& other) const {
                 return baseMipLevel == other.baseMipLevel &&
@@ -181,7 +187,8 @@ public:
                        layerCount == other.layerCount &&
                        viewType == other.viewType &&
                        format == other.format &&
-                       aspect == other.aspect;
+                       aspect == other.aspect &&
+                       componentSwizzle == other.componentSwizzle;
             }
         };
 
@@ -197,6 +204,7 @@ public:
                         0x9e3779b9u + (hash << 6) + (hash >> 2);
                 hash ^= std::hash<Uint32>{}(static_cast<Uint32>(key.aspect)) +
                         0x9e3779b9u + (hash << 6) + (hash >> 2);
+                hash ^= std::hash<Uint32>{}(key.componentSwizzle) + 0x9e3779b9u + (hash << 6) + (hash >> 2);
                 return hash;
             }
         };
@@ -418,8 +426,17 @@ public:
         VkFormat format = VK_FORMAT_UNDEFINED;
         VkImageViewType viewType = VK_IMAGE_VIEW_TYPE_2D;
         VkImageAspectFlags sampledAspect = VK_IMAGE_ASPECT_COLOR_BIT;
+        VkComponentMapping components{VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_B,
+                                      VK_COMPONENT_SWIZZLE_A};
         Bool isTextureView = false;
     };
+
+    // The four component swizzles packed into one value, for the sampled-view cache key.
+    static Uint32 PackComponentSwizzle(const VkComponentMapping& components) {
+        return (static_cast<Uint32>(components.r) & 0xFFu) | ((static_cast<Uint32>(components.g) & 0xFFu) << 8) |
+               ((static_cast<Uint32>(components.b) & 0xFFu) << 16) |
+               ((static_cast<Uint32>(components.a) & 0xFFu) << 24);
+    }
     TextureViewWindow ResolveTextureViewWindow(MG_State::GLState::ITextureObject& texture,
                                                const TextureResource& resource) const;
     // Records what a GL texture view needs of the image it views, so the next sync of the
@@ -428,6 +445,10 @@ public:
     void NoteTextureViewImageRequirements(MG_State::GLState::ITextureObject& viewTexture,
                                           MG_State::GLState::ITextureObject& storageTexture);
     VkImageCreateFlags GetViewRequestedImageFlags(const MG_State::GLState::ITextureObject& storageTexture) const;
+    // Appends every format a GL texture view reinterprets this storage as, for the narrowed
+    // VkImageFormatListCreateInfo the image is created with.
+    void AppendViewRequestedFormats(const MG_State::GLState::ITextureObject& storageTexture,
+                                    Vector<VkFormat>& outFormats) const;
     // Builds (and caches, keyed by the whole window) one sampled VkImageView over a storage
     // image. Shared back end of every GL-texture-view sampled path.
     VkImageView GetOrCreateWindowedSampledView(MG_State::GLState::ITextureObject& texture,
@@ -655,6 +676,11 @@ private:
     // feature almost none of them use. A SAME-format view - which is the common case, and the
     // Better Clouds case - needs no flag at all and therefore costs nothing.
     std::unordered_map<TextureIdentity, VkImageCreateFlags, TextureIdentityHash> m_viewRequestedImageFlags;
+    // Every VkFormat a GL texture view has asked to reinterpret this storage as. The narrowed
+    // VkImageFormatListCreateInfo the image is created with must name them: the list is a promise
+    // that NO other format will ever be viewed, and building a view outside it is
+    // VUID-VkImageViewCreateInfo-pNext-01585. Keyed, like the flags above, by the STORAGE texture.
+    std::unordered_map<TextureIdentity, std::unordered_set<VkFormat>, TextureIdentityHash> m_viewRequestedFormats;
     // Supported multisample counts per format, so repeat texture syncs do not
     // re-query vkGetPhysicalDeviceImageFormatProperties.
     std::unordered_map<VkFormat, VkSampleCountFlags> m_multisampleCountsByFormat;

MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp

@@ -9025,7 +9025,15 @@ void main() {
         // and an overlap check). Refused outright, and refused for real rather than through an
         // assertion the release build drops: recording the pair anyway is a validation error and,
         // on a tiler, a copy whose source has already been overwritten.
-        if (srcEndpoint.Texture == dstEndpoint.Texture && srcEndpoint.Renderbuffer == dstEndpoint.Renderbuffer) {
+        // Compared by STORAGE, not by GL object: a texture view and the texture it views are two
+        // different objects over one VkImage (ARB_texture_view), and GL 4.6 core 8.18 explicitly
+        // permits copying between them - so an object-identity test would let exactly the case
+        // this guard exists for through.
+        const auto* srcStorageTexture =
+            srcEndpoint.Texture ? &VkTextureManager::StorageTextureOf(*srcEndpoint.Texture) : nullptr;
+        const auto* dstStorageTexture =
+            dstEndpoint.Texture ? &VkTextureManager::StorageTextureOf(*dstEndpoint.Texture) : nullptr;
+        if (srcStorageTexture == dstStorageTexture && srcEndpoint.Renderbuffer == dstEndpoint.Renderbuffer) {
             MGLOG_E_ONCE("%s: in-place copy on objectId=%u is not supported; declining the copy", __func__,
                          CopyImageEndpointName(srcEndpoint));
             return;
@@ -9095,6 +9103,15 @@ void main() {
             MGLOG_E_ONCE("%s: source or destination image failed to sync; declining the copy", __func__);
             return;
         }
+        // Storage space from here down. srcImage/dstImage are the STORAGE textures' resources
+        // (SyncTextureAndGetDescriptor resolves a view to the texture it views), while srcLevel /
+        // dstLevel and the z origins below arrived relative to whichever name the application
+        // passed - so a view's level 0 has to become the parent level it opened onto before it
+        // can index a subresource, exactly as at every other attachment boundary.
+        srcLevel = static_cast<GLint>(ToStorageMipLevel(srcEndpoint.Texture.get(), srcLevel));
+        dstLevel = static_cast<GLint>(ToStorageMipLevel(dstEndpoint.Texture.get(), dstLevel));
+        srcZ = static_cast<GLint>(ToStorageArrayLayer(srcEndpoint.Texture.get(), srcZ));
+        dstZ = static_cast<GLint>(ToStorageArrayLayer(dstEndpoint.Texture.get(), dstZ));
         if (srcLevel < 0 || dstLevel < 0 || static_cast<Uint32>(srcLevel) >= srcImage.mipLevels ||
             static_cast<Uint32>(dstLevel) >= dstImage.mipLevels) {
             MGLOG_E_ONCE("%s: mip level out of range (src %d of %u, dst %d of %u); declining the copy", __func__,
@@ -10197,10 +10214,14 @@ void main() {
                     textureMipmapObject->GetMipmapTexelSize(textureUploadTarget, static_cast<Uint>(level));
                 const Bool isCubeFace = textureUploadTarget >= TextureUploadTarget::CubeMapPositiveX &&
                     textureUploadTarget <= TextureUploadTarget::CubeMapNegativeZ;
-                const Uint32 arrayLayer = isCubeFace
+                // Storage space: `resource` is the storage texture's, so a view's level and
-                    ? static_cast<Uint32>(textureUploadTarget) -
+                // layer have to be shifted into its numbering (see ToStorageMipLevel).
-                        static_cast<Uint32>(TextureUploadTarget::CubeMapPositiveX)
+                const Int glArrayLayer = isCubeFace
+                    ? 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
                 // rows of layer 0, so the read has to be told which of the two it is looking at.
@@ -10209,7 +10230,7 @@ void main() {
                         ? static_cast<Uint32>(std::max<Int>(levelSize.y(), 1))
                         : 1u;
                 ReadDepthStencilImageToClient(resource->image, resource->format, &resource->layout, resource->aspect,
-                                              static_cast<Uint32>(level), arrayLayer, 0, 0, levelSize.x(),
+                                              storageLevel, arrayLayer, 0, 0, levelSize.x(),
                                               levelSize.y(), format, type, pixels,
                                               /*defaultFramebufferOrientation=*/false, sourceLayers);
             } else {
@@ -10286,13 +10307,15 @@ void main() {
             frame.commandBuffer, resource->image, resource->layout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL,
             srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT,
             srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, resource->aspect,
-            static_cast<Uint32>(level), 1);
+            ToStorageMipLevel(textureObject.get(), level), 1);
         MOBILEGL_ASSERT(ok, "%s: failed to transition texture image", __func__);
 
         VkBufferImageCopy copyRegion{};
         copyRegion.imageSubresource.aspectMask = resource->aspect;
-        copyRegion.imageSubresource.mipLevel = static_cast<Uint32>(level);
+        // Storage space, as above: a texture view reads its own level 0 out of whichever level
-        copyRegion.imageSubresource.baseArrayLayer = 0;
+        // and layer of the parent it opened onto.
+        copyRegion.imageSubresource.mipLevel = ToStorageMipLevel(textureObject.get(), level);
+        copyRegion.imageSubresource.baseArrayLayer = ToStorageArrayLayer(textureObject.get(), 0);
         copyRegion.imageSubresource.layerCount = static_cast<Uint32>(arrayLayers);
         copyRegion.imageExtent = {static_cast<Uint32>(width),
                                   is1dArrayImage ? 1u : static_cast<Uint32>(height),
@@ -10307,7 +10330,7 @@ void main() {
             frame.commandBuffer, resource->image, resource->layout, originalLayout,
             VK_PIPELINE_STAGE_TRANSFER_BIT, restoreStageMask,
             VK_ACCESS_TRANSFER_READ_BIT, restoreAccessMask, resource->aspect,
-            static_cast<Uint32>(level), 1);
+            ToStorageMipLevel(textureObject.get(), level), 1);
         MOBILEGL_ASSERT(ok, "%s: failed to restore texture image layout", __func__);
 
         if (!SubmitReadbackCommandsAndWait(frame)) {

MobileGL/MG_IntegrationTest/Scenarios/TextureViewScenario.cpp

@@ -691,6 +691,115 @@ void main() {
                          "a single-layer 2D view of an array must address the layer it named");
         }
 
+        // ------------------------------------------------------------------------------------
+        // Writing THROUGH a layer-sliced view. The read direction is covered above; this is the
+        // write direction, and it is the one that can corrupt the parent rather than merely
+        // return the wrong pixels - a view whose texel path forgot its layer origin writes over
+        // the parent's layer 0 while the application believes it addressed layer minLayer.
+        // ------------------------------------------------------------------------------------
+        TEST_F(TextureViewScenario, WritingThroughALayerSlicedViewLandsOnItsOwnLayers) {
+            if (!Ready() || IsSkipped()) return;
+
+            constexpr int kLayers = 4;
+            constexpr int kViewMinLayer = 2;
+            const GLuint storage = MakeTexture();
+            glBindTexture(GL_TEXTURE_2D_ARRAY, storage);
+            glTexStorage3D(GL_TEXTURE_2D_ARRAY, 1, GL_RGBA8, kSize, kSize, kLayers);
+            glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+            glTexParameteri(GL_TEXTURE_2D_ARRAY, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+            ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
+
+            const auto layerFill = [](int layer) {
+                return Rgba8{static_cast<std::uint8_t>(10 + layer * 20),
+                             static_cast<std::uint8_t>(200 - layer * 20), 30, 255};
+            };
+            // Seeded by CPU sub-image rather than by rendering, deliberately: this scenario is
+            // about the view's LAYER ORIGIN, and seeding through the GPU would additionally
+            // depend on a CPU sub-image reaching a layer whose content the GPU wrote - which
+            // DirectVulkan does not currently do even for a plain array texture (no view
+            // involved), and which would make a failure here unattributable.
+            const auto uploadLayer = [&](GLuint texture, int layer, Rgba8 colour) {
+                std::vector<std::uint8_t> texels(static_cast<std::size_t>(kSize) * kSize * 4);
+                for (std::size_t i = 0; i < texels.size(); i += 4) {
+                    texels[i + 0] = colour.r;
+                    texels[i + 1] = colour.g;
+                    texels[i + 2] = colour.b;
+                    texels[i + 3] = colour.a;
+                }
+                glBindTexture(GL_TEXTURE_2D_ARRAY, texture);
+                glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
+                glTexSubImage3D(GL_TEXTURE_2D_ARRAY, 0, 0, 0, layer, kSize, kSize, 1, GL_RGBA, GL_UNSIGNED_BYTE,
+                                texels.data());
+                glBindTexture(GL_TEXTURE_2D_ARRAY, 0);
+            };
+            for (int layer = 0; layer < kLayers; ++layer) {
+                uploadLayer(storage, layer, layerFill(layer));
+            }
+            const GLuint fbo = MakeFbo();
+            glDisable(GL_DEPTH_TEST);
+            glDisable(GL_STENCIL_TEST);
+            ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR)) << "seeding the layers raised an error";
+
+            // A two-layer window starting at layer 2, so a lost offset lands on layer 0 - which
+            // the assertions below would see as an untouched layer that moved.
+            const GLuint view = MakeTexture();
+            glTextureView(view, GL_TEXTURE_2D_ARRAY, storage, GL_RGBA8, 0, 1, kViewMinLayer, 2);
+            ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR));
+
+            // Write the view's OWN layer 0, i.e. the storage's layer 2.
+            constexpr Rgba8 kPainted{255, 0, 255, 255};
+            std::vector<std::uint8_t> texels(static_cast<std::size_t>(kSize) * kSize * 4);
+            for (std::size_t i = 0; i < texels.size(); i += 4) {
+                texels[i + 0] = kPainted.r;
+                texels[i + 1] = kPainted.g;
+                texels[i + 2] = kPainted.b;
+                texels[i + 3] = kPainted.a;
+            }
+            glBindTexture(GL_TEXTURE_2D_ARRAY, view);
+            glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
+            glTexSubImage3D(GL_TEXTURE_2D_ARRAY, 0, 0, 0, 0, kSize, kSize, 1, GL_RGBA, GL_UNSIGNED_BYTE,
+                            texels.data());
+            glBindTexture(GL_TEXTURE_2D_ARRAY, 0);
+            ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR)) << "writing through the view raised an error";
+
+            // POSITIVE CONTROL, through the parent's own name and into a layer outside the view's
+            // window. It makes the assertions below able to tell "the view lost its layer origin"
+            // from "a CPU sub-image into this array does not reach the GPU at all", which is a
+            // different question and not one a texture view can answer.
+            constexpr Rgba8 kControl{0, 0, 255, 255};
+            std::vector<std::uint8_t> controlTexels(texels.size());
+            for (std::size_t i = 0; i < controlTexels.size(); i += 4) {
+                controlTexels[i + 0] = kControl.r;
+                controlTexels[i + 1] = kControl.g;
+                controlTexels[i + 2] = kControl.b;
+                controlTexels[i + 3] = kControl.a;
+            }
+            glBindTexture(GL_TEXTURE_2D_ARRAY, storage);
+            glTexSubImage3D(GL_TEXTURE_2D_ARRAY, 0, 0, 0, 1, kSize, kSize, 1, GL_RGBA, GL_UNSIGNED_BYTE,
+                            controlTexels.data());
+            glBindTexture(GL_TEXTURE_2D_ARRAY, 0);
+            ASSERT_EQ(glGetError(), static_cast<GLenum>(GL_NO_ERROR)) << "the control write raised an error";
+
+            // Read every layer of the PARENT back: only the one the view's layer 0 maps to may
+            // have changed.
+            for (int layer = 0; layer < kLayers; ++layer) {
+                glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+                glFramebufferTextureLayer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, storage, 0, layer);
+                glReadBuffer(GL_COLOR_ATTACHMENT0);
+                const Image image = ReadPixels(kSize, kSize);
+                Rgba8 expected = layerFill(layer);
+                const char* what = "a layer outside the view's window must not have been written";
+                if (layer == kViewMinLayer) {
+                    expected = kPainted;
+                    what = "the view's layer 0 must be the storage layer it named";
+                } else if (layer == 1) {
+                    expected = kControl;
+                    what = "control: a sub-image written through the PARENT must reach its layer";
+                }
+                ExpectRegion(image, 0, kSize - 1, 0, kSize - 1, expected, 2, what);
+            }
+        }
+
         // ------------------------------------------------------------------------------------
         // Views of views compose; the composed view still reaches the ROOT storage.
         // ------------------------------------------------------------------------------------

MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp

@@ -9,6 +9,7 @@
 #include "TextureObjectView.h"
 
 #include <algorithm>
+#include <cstring>
 
 namespace MobileGL::MG_State::GLState {
     namespace {
@@ -149,6 +150,53 @@ namespace MobileGL::MG_State::GLState {
         return size;
     }
 
+    SizeT TextureObjectView::LayerByteOffset(TextureUploadTarget viewTarget, Uint mipmapLevel) const {
+        if (m_viewMinLayer == 0 || m_ownerMipmap == nullptr) return 0;
+        const LayerAxis ownerAxis = LayerAxisOf(m_storageOwner->GetTarget());
+        if (ownerAxis == LayerAxis::None) {
+            // A cube-map owner keeps each face in its OWN blob, and ToOwnerUploadTarget already
+            // picked the right one; a 3D or plain 2D owner has no layers to skip.
+            return 0;
+        }
+        const TextureUploadTarget ownerTarget = ToOwnerUploadTarget(viewTarget);
+        const Uint ownerLevel = ToOwnerLevel(mipmapLevel);
+        const IntVec3 ownerSize = m_ownerMipmap->GetMipmapTexelSize(ownerTarget, ownerLevel);
+        const SizeT ownerBytes = m_ownerMipmap->GetMipmapByteSize(ownerTarget, ownerLevel);
+        const SizeT ownerTexels = static_cast<SizeT>(std::max(ownerSize.x(), 0)) *
+                                  static_cast<SizeT>(std::max(ownerSize.y(), 0)) *
+                                  static_cast<SizeT>(std::max(ownerSize.z(), 1));
+        if (ownerTexels == 0 || ownerBytes == 0) return 0;
+        const SizeT bytesPerTexel = ownerBytes / ownerTexels;
+        // One "layer" is a whole x*y slice for a 2D/cube array, and a single row of `width`
+        // texels for a 1D array (whose layer count lives in the state-side height).
+        const SizeT layerTexels = ownerAxis == LayerAxis::Y
+                                      ? static_cast<SizeT>(std::max(ownerSize.x(), 0))
+                                      : static_cast<SizeT>(std::max(ownerSize.x(), 0)) *
+                                            static_cast<SizeT>(std::max(ownerSize.y(), 0));
+        const SizeT offset = static_cast<SizeT>(m_viewMinLayer) * layerTexels * bytesPerTexel;
+        return offset < ownerBytes ? offset : 0;
+    }
+
+    IntVec3 TextureObjectView::ToOwnerRegionOffset(const IntVec3& viewOffset) const {
+        if (m_viewMinLayer == 0) return viewOffset;
+        IntVec3 offset = viewOffset;
+        // The dirty region is recorded in the OWNER's blob coordinates - that is the space its
+        // upload path walks - so the view's layer origin has to be added here even though
+        // MapMipmapData hands back an already-shifted POINTER. The two are not double-counting:
+        // one moves the bytes, the other tells the owner which of its layers moved.
+        switch (LayerAxisOf(m_storageOwner->GetTarget())) {
+        case LayerAxis::Y:
+            offset.y() += static_cast<Int>(m_viewMinLayer);
+            break;
+        case LayerAxis::Z:
+            offset.z() += static_cast<Int>(m_viewMinLayer);
+            break;
+        case LayerAxis::None:
+            break;
+        }
+        return offset;
+    }
+
     Uint TextureObjectView::GetMipmapLevelCount() const {
         if (m_ownerMipmap == nullptr) return 0;
         const Uint ownerLevels = m_ownerMipmap->GetMipmapLevelCount();
@@ -180,7 +228,14 @@ namespace MobileGL::MG_State::GLState {
         const SizeT viewTexels = static_cast<SizeT>(std::max(viewSize.x(), 0)) *
                                  static_cast<SizeT>(std::max(viewSize.y(), 0)) *
                                  static_cast<SizeT>(std::max(viewSize.z(), 1));
-        return (ownerBytes / ownerTexels) * viewTexels;
+        const SizeT viewBytes = (ownerBytes / ownerTexels) * viewTexels;
+        // Clamped against what remains of the owner's blob past this view's layer origin. A view
+        // whose layer window the shadow cannot lay out contiguously - several faces of a cube-map
+        // owner, which are separate blobs - would otherwise advertise more bytes than
+        // MapMipmapData can hand back, and a caller sizing a copy off this would overrun.
+        const SizeT layerOffset = LayerByteOffset(target, mipmapLevel);
+        const SizeT available = layerOffset < ownerBytes ? ownerBytes - layerOffset : 0;
+        return std::min(viewBytes, available);
     }
 
     void TextureObjectView::AllocateStorage(TextureUploadTarget uploadTarget, Uint mipmapLevel, MipmapInput input) {
@@ -199,12 +254,36 @@ namespace MobileGL::MG_State::GLState {
 
     void TextureObjectView::UpdateMipmapSubData(TextureUploadTarget uploadTarget, Uint mipmapLevel, DataPtr input) {
         if (m_ownerMipmap == nullptr) return;
-        m_ownerMipmap->UpdateMipmapSubData(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel), input);
+        const TextureUploadTarget ownerTarget = ToOwnerUploadTarget(uploadTarget);
+        const Uint ownerLevel = ToOwnerLevel(mipmapLevel);
+        const SizeT layerOffset = LayerByteOffset(uploadTarget, mipmapLevel);
+        if (layerOffset == 0) {
+            m_ownerMipmap->UpdateMipmapSubData(ownerTarget, ownerLevel, input);
+            return;
+        }
+        // The owner's whole-level write starts at ITS level origin, which for a layer-sliced view
+        // is the wrong place: writing there would silently overwrite the parent's layers 0..n
+        // instead of the window this view opened. Write through the shifted pointer instead, and
+        // mark exactly the layers that moved.
+        auto* destination = static_cast<Uint8*>(m_ownerMipmap->MapMipmapData(ownerTarget, ownerLevel));
+        if (destination == nullptr || input.data == nullptr || input.size == 0) return;
+        const SizeT capacity = GetMipmapByteSize(uploadTarget, mipmapLevel);
+        std::memcpy(destination + layerOffset, input.data, std::min(input.size, capacity));
+        const IntVec3 viewSize = GetMipmapTexelSize(uploadTarget, mipmapLevel);
+        MarkStorageDirtyRegion(uploadTarget, mipmapLevel, IntVec3{0, 0, 0},
+                               IntVec3{viewSize.x(), viewSize.y(), std::max(viewSize.z(), 1)});
     }
 
     void* TextureObjectView::MapMipmapData(TextureUploadTarget uploadTarget, Uint mipmapLevel) {
         if (m_ownerMipmap == nullptr) return nullptr;
-        return m_ownerMipmap->MapMipmapData(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel));
+        auto* data = static_cast<Uint8*>(
+            m_ownerMipmap->MapMipmapData(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel)));
+        if (data == nullptr) return nullptr;
+        // Shifted to the view's first LAYER, so that a caller which maps this pointer and then
+        // offsets into it using the extents GetMipmapTexelSize reports - which is what every
+        // glTexSubImage*/glGetTexImage path does - lands on the layers this view addresses rather
+        // than on the parent's first ones.
+        return data + LayerByteOffset(uploadTarget, mipmapLevel);
     }
 
     void TextureObjectView::MarkStorageDirty(TextureUploadTarget uploadTarget, Uint mipmapLevel, Bool dirty) {
@@ -220,8 +299,8 @@ namespace MobileGL::MG_State::GLState {
     void TextureObjectView::MarkStorageDirtyRegion(TextureUploadTarget uploadTarget, Uint mipmapLevel, IntVec3 offset,
                                                    IntVec3 size) {
         if (m_ownerMipmap == nullptr) return;
-        m_ownerMipmap->MarkStorageDirtyRegion(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel), offset,
+        m_ownerMipmap->MarkStorageDirtyRegion(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel),
-                                              size);
+                                              ToOwnerRegionOffset(offset), size);
     }
 
     MipmapDirtyRegion TextureObjectView::GetStorageDirtyRegion(TextureUploadTarget uploadTarget,

MobileGL/MG_State/GLState/TextureState/TextureObjectView.h

@@ -30,13 +30,17 @@ namespace MobileGL::MG_State::GLState {
     // "<minlevel> plus the value of TEXTURE_VIEW_MIN_LEVEL from the original texture" rule
     // means; one hop therefore always reaches real storage and no recursion is possible.
     //
-    // LAYER offsets are deliberately NOT applied here. The TextureObjectMipmap interface
+    // LEVEL offsets are applied by shifting the level index; LAYER offsets cannot be, because the
-    // addresses storage as (upload target, level) and a layer lives INSIDE a level's blob, so a
+    // TextureObjectMipmap interface addresses storage as (upload target, level) and a layer lives
-    // layer offset is not expressible at this boundary. The entry points that move texels for a
+    // INSIDE a level's blob. They are applied two other ways instead, and the pair is what keeps
-    // view (glTexSubImage*, glGetTexImage) therefore redirect to the owner themselves and add
+    // a layer-sliced view from corrupting its parent:
-    // GetViewMinLayer() to the z coordinate there, where it can be said. What this class does
+    //   * MapMipmapData returns a pointer already advanced to the view's first layer, so a caller
-    // apply is the view's layer COUNT, because the level extents it reports are what both
+    //     that maps it and then offsets using the extents GetMipmapTexelSize reports - which is
-    // backends size their images and views from.
+    //     what every glTexSubImage*/glGetTexImage path does - writes the layers it meant to; and
+    //   * MarkStorageDirtyRegion moves the region's origin into the OWNER's layer space, which is
+    //     the space its upload path walks.
+    // Those two are not double-counting: one moves the bytes, the other names which of the
+    // owner's layers moved.
     class TextureObjectView : public TextureObjectMipmap {
     public:
         TextureObjectView(Uint externalIndex, TextureTarget target, SharedPtr<ITextureObject> storageOwner,
@@ -45,6 +49,13 @@ namespace MobileGL::MG_State::GLState {
         const SharedPtr<ITextureObject>& GetViewStorageOwner() const override { return m_storageOwner; }
         const Vector<TextureUploadTarget>& GetUploadTargets() const override { return m_uploadTargets; }
 
+        // A view is immutable from birth (GL 4.6 core 8.18 sets its TEXTURE_IMMUTABLE_FORMAT), and
+        // unconditionally so: the base class infers immutability from a non-zero level count, and
+        // a degenerate view - one the spec's min() composition narrowed to zero levels - would
+        // otherwise report GL_FALSE, walk straight past ValidateTextureMutable and let
+        // glTexImage2D respecify the PARENT's immutable storage through AllocateStorage.
+        Bool IsImmutable() const override { return true; }
+
         // GL 4.6 core 8.18: "TEXTURE_IMMUTABLE_LEVELS is set to the value of
         // TEXTURE_IMMUTABLE_LEVELS from the original texture" - NOT to <numlevels>. Kept as a
         // forward rather than in m_immutableLevels so that the base class's level-range clamp
@@ -98,6 +109,13 @@ namespace MobileGL::MG_State::GLState {
         // axis. A GL 1D array carries its layer count in the state-side HEIGHT while every other
         // layered target carries it in z, so the axis is target-dependent.
         IntVec3 ToViewLevelSize(const IntVec3& ownerLevelSize) const;
+        // Where this view's first LAYER starts inside the owner's level blob. The layer axis a
+        // level's bytes are laid out along is the OWNER's, so this is a slice for a 2D/cube array
+        // and a single row for a 1D array; a cube-map owner returns 0 because its faces are
+        // separate blobs that ToOwnerUploadTarget already selects between.
+        SizeT LayerByteOffset(TextureUploadTarget viewTarget, Uint mipmapLevel) const;
+        // A dirty-region origin moved from the view's layer space into the owner's.
+        IntVec3 ToOwnerRegionOffset(const IntVec3& viewOffset) const;
 
         SharedPtr<ITextureObject> m_storageOwner;
         // Non-owning; m_storageOwner keeps it alive and is never a view, so this is set once in