diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index f1f40f51..f01a29f7 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/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", diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp index 1d829cf6..ad14bceb 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp +++ b/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(uploadTarget) - static_cast(TextureUploadTarget::CubeMapPositiveX); + const Int face = + static_cast(uploadTarget) - static_cast(TextureUploadTarget::CubeMapPositiveX); + return ToStorageArrayLayer(texture, face); } static Uint32 ResolveAttachmentLayerCount(const MG_State::GLState::FramebufferAttachmentObject& attachment) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp index ad14a451..29bc3d9c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp +++ b/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; - Uint32 layerCount = resource->arrayLayers; + // A GL texture view opens onto a WINDOW of the storage's layers; a layered image + // 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(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(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) { - required |= VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT; + // 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& outFormats) const { + const auto it = m_viewRequestedFormats.find( + MakeTextureIdentity(const_cast(&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"); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h index 5b4b013b..8379989e 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.h +++ b/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{}(static_cast(key.aspect)) + 0x9e3779b9u + (hash << 6) + (hash >> 2); + hash ^= std::hash{}(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(components.r) & 0xFFu) | ((static_cast(components.g) & 0xFFu) << 8) | + ((static_cast(components.b) & 0xFFu) << 16) | + ((static_cast(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& 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 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, TextureIdentityHash> m_viewRequestedFormats; // Supported multisample counts per format, so repeat texture syncs do not // re-query vkGetPhysicalDeviceImageFormatProperties. std::unordered_map m_multisampleCountsByFormat; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 03f7ca79..c7c4fe58 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/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(ToStorageMipLevel(srcEndpoint.Texture.get(), srcLevel)); + dstLevel = static_cast(ToStorageMipLevel(dstEndpoint.Texture.get(), dstLevel)); + srcZ = static_cast(ToStorageArrayLayer(srcEndpoint.Texture.get(), srcZ)); + dstZ = static_cast(ToStorageArrayLayer(dstEndpoint.Texture.get(), dstZ)); if (srcLevel < 0 || dstLevel < 0 || static_cast(srcLevel) >= srcImage.mipLevels || static_cast(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(level)); const Bool isCubeFace = textureUploadTarget >= TextureUploadTarget::CubeMapPositiveX && textureUploadTarget <= TextureUploadTarget::CubeMapNegativeZ; - const Uint32 arrayLayer = isCubeFace - ? static_cast(textureUploadTarget) - - static_cast(TextureUploadTarget::CubeMapPositiveX) + // 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 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(std::max(levelSize.y(), 1)) : 1u; ReadDepthStencilImageToClient(resource->image, resource->format, &resource->layout, resource->aspect, - static_cast(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(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(level); - copyRegion.imageSubresource.baseArrayLayer = 0; + // 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); copyRegion.imageSubresource.layerCount = static_cast(arrayLayers); copyRegion.imageExtent = {static_cast(width), is1dArrayImage ? 1u : static_cast(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(level), 1); + ToStorageMipLevel(textureObject.get(), level), 1); MOBILEGL_ASSERT(ok, "%s: failed to restore texture image layout", __func__); if (!SubmitReadbackCommandsAndWait(frame)) { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/TextureViewScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/TextureViewScenario.cpp index 1f6d6373..e84fd14e 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/TextureViewScenario.cpp +++ b/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(GL_NO_ERROR)); + + const auto layerFill = [](int layer) { + return Rgba8{static_cast(10 + layer * 20), + static_cast(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 texels(static_cast(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(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(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 texels(static_cast(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(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 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(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. // ------------------------------------------------------------------------------------ diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp index ae2ed2e9..9e26e842 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp @@ -9,6 +9,7 @@ #include "TextureObjectView.h" #include +#include 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(std::max(ownerSize.x(), 0)) * + static_cast(std::max(ownerSize.y(), 0)) * + static_cast(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(std::max(ownerSize.x(), 0)) + : static_cast(std::max(ownerSize.x(), 0)) * + static_cast(std::max(ownerSize.y(), 0)); + const SizeT offset = static_cast(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(m_viewMinLayer); + break; + case LayerAxis::Z: + offset.z() += static_cast(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(std::max(viewSize.x(), 0)) * static_cast(std::max(viewSize.y(), 0)) * static_cast(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(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( + 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, - size); + m_ownerMipmap->MarkStorageDirtyRegion(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel), + ToOwnerRegionOffset(offset), size); } MipmapDirtyRegion TextureObjectView::GetStorageDirtyRegion(TextureUploadTarget uploadTarget, diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h index 0c71a2c9..c7d12382 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h @@ -30,13 +30,17 @@ namespace MobileGL::MG_State::GLState { // " 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 - // addresses storage as (upload target, level) and a layer lives INSIDE a level's blob, so a - // layer offset is not expressible at this boundary. The entry points that move texels for a - // view (glTexSubImage*, glGetTexImage) therefore redirect to the owner themselves and add - // GetViewMinLayer() to the z coordinate there, where it can be said. What this class does - // apply is the view's layer COUNT, because the level extents it reports are what both - // backends size their images and views from. + // LEVEL offsets are applied by shifting the level index; LAYER offsets cannot be, because the + // TextureObjectMipmap interface addresses storage as (upload target, level) and a layer lives + // INSIDE a level's blob. They are applied two other ways instead, and the pair is what keeps + // a layer-sliced view from corrupting its parent: + // * MapMipmapData returns a pointer already advanced to the view's first layer, so a caller + // that maps it and then offsets using the extents GetMipmapTexelSize reports - which is + // 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 storageOwner, @@ -45,6 +49,13 @@ namespace MobileGL::MG_State::GLState { const SharedPtr& GetViewStorageOwner() const override { return m_storageOwner; } const Vector& 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 . 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 m_storageOwner; // Non-owning; m_storageOwner keeps it alive and is never a view, so this is set once in