diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index 9dc40996..647f2c07 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -5078,6 +5078,13 @@ 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. + // A cube map's six faces are six independent images, and both readback spellings name one of + // them: glGetTexImage through the TARGET token, glGetTextureSubImage through zoffset. Both then + // have to tell the size checks below that ONE image is coming back, not six. + static Bool IsCubeMapFaceUploadTarget(TextureUploadTarget target) { + return target >= TextureUploadTarget::CubeMapPositiveX && target <= TextureUploadTarget::CubeMapNegativeZ; + } + // `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 @@ -5282,9 +5289,14 @@ namespace MobileGL::MG_Impl::GLImpl { isProxy ? TextureImpl::pProxyTextureManager->GetProxyTextureObject(textureUploadTarget) : bindingSlot.GetBoundObject(); - // glGetTexImage has no bufSize argument: -1 stands for "no client-side limit". + // glGetTexImage has no bufSize argument: -1 stands for "no client-side limit". That skips + // the destination-size branch but NOT the pixel-pack-buffer one, which measures the same + // `required` against the bound PBO's real size - so a cube FACE query has to say it returns + // one image here too, or a PBO sized for the one face this call packs is refused as too + // small while the copy that follows writes exactly that much into it. return ValidateTextureImageQuery(textureObject, level, textureInputFormat, texturePixelDataType, -1, pixels, - "GetTexImage_State"); + "GetTexImage_State", + IsCubeMapFaceUploadTarget(textureUploadTarget) ? 1u : 0u); } // What this helper can and cannot answer. diff --git a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp index 9e26e842..c299c1c2 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.cpp @@ -97,27 +97,31 @@ namespace MobileGL::MG_State::GLState { return m_storageOwner->HasFixedSampleLocations(); } + Uint TextureObjectView::ViewLayerIndex(TextureUploadTarget viewTarget) const { + if (GetTarget() != TextureTarget::TextureCubeMap) { + // One target, one layer: the view's origin is the whole answer. + return m_viewMinLayer; + } + for (Uint i = 0; i < static_cast(m_uploadTargets.size()); ++i) { + if (m_uploadTargets[i] == viewTarget) return m_viewMinLayer + i; + } + return m_viewMinLayer; + } + TextureUploadTarget TextureObjectView::ToOwnerUploadTarget(TextureUploadTarget viewTarget) const { const auto& ownerTargets = m_storageOwner->GetUploadTargets(); MOBILEGL_ASSERT(!ownerTargets.empty(), "TextureObjectView: storage owner has no upload target"); if (ownerTargets.size() == 1) { - // The owner keeps every layer in one blob, so there is nothing to choose. + // The owner keeps every layer in one blob, so there is nothing to choose HERE - which + // is exactly why a cube-map view over such an owner has to have its face carried by + // LayerByteOffset instead. See ViewLayerIndex. return ownerTargets[0]; } - // The owner is a cube map: six independent blobs, one per face, and the view's layer - // index selects among them. A cube-map view of a cube map maps face to face; any other - // view target addresses layers, which for a cube-map owner ARE its faces. + // The owner is a cube map: six independent blobs, one per face, and the layer this view + // target names selects among them. A cube-map view of a cube map maps face to face; any + // other view target addresses layers, which for a cube-map owner ARE its faces. const Uint faceCount = static_cast(ownerTargets.size()); - Uint face = m_viewMinLayer; - if (GetTarget() == TextureTarget::TextureCubeMap) { - for (Uint i = 0; i < m_uploadTargets.size(); ++i) { - if (m_uploadTargets[i] == viewTarget) { - face = m_viewMinLayer + i; - break; - } - } - } - return ownerTargets[std::min(face, faceCount - 1)]; + return ownerTargets[std::min(ViewLayerIndex(viewTarget), faceCount - 1)]; } IntVec3 TextureObjectView::ToViewLevelSize(const IntVec3& ownerLevelSize) const { @@ -151,7 +155,13 @@ namespace MobileGL::MG_State::GLState { } SizeT TextureObjectView::LayerByteOffset(TextureUploadTarget viewTarget, Uint mipmapLevel) const { - if (m_viewMinLayer == 0 || m_ownerMipmap == nullptr) return 0; + if (m_ownerMipmap == nullptr) return 0; + // The FACE is part of this, not just the view's origin: a cube-map view over a layered + // owner (a 2D array or a cube-map ARRAY) has only one blob to address, so the face its + // target token names lives here or nowhere. It used to live nowhere, and all six face + // tokens read the view's first layer-face - silently, with texels from a real layer. + const Uint layerIndex = ViewLayerIndex(viewTarget); + if (layerIndex == 0) 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 @@ -173,23 +183,26 @@ namespace MobileGL::MG_State::GLState { ? 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; + const SizeT offset = static_cast(layerIndex) * layerTexels * bytesPerTexel; return offset < ownerBytes ? offset : 0; } - IntVec3 TextureObjectView::ToOwnerRegionOffset(const IntVec3& viewOffset) const { - if (m_viewMinLayer == 0) return viewOffset; + IntVec3 TextureObjectView::ToOwnerRegionOffset(TextureUploadTarget viewTarget, const IntVec3& viewOffset) const { + const Uint layerIndex = ViewLayerIndex(viewTarget); + if (layerIndex == 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 + // upload path walks - so the layer this view target names 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. + // one moves the bytes, the other tells the owner which of its layers moved. They must agree + // on the layer, which is why both ask ViewLayerIndex rather than reading m_viewMinLayer - + // on a cube-map view the face is half the answer. switch (LayerAxisOf(m_storageOwner->GetTarget())) { case LayerAxis::Y: - offset.y() += static_cast(m_viewMinLayer); + offset.y() += static_cast(layerIndex); break; case LayerAxis::Z: - offset.z() += static_cast(m_viewMinLayer); + offset.z() += static_cast(layerIndex); break; case LayerAxis::None: break; @@ -300,7 +313,7 @@ namespace MobileGL::MG_State::GLState { IntVec3 size) { if (m_ownerMipmap == nullptr) return; m_ownerMipmap->MarkStorageDirtyRegion(ToOwnerUploadTarget(uploadTarget), ToOwnerLevel(mipmapLevel), - ToOwnerRegionOffset(offset), size); + ToOwnerRegionOffset(uploadTarget, 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 c7d12382..9a4d9ecb 100644 --- a/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h +++ b/MobileGL/MG_State/GLState/TextureState/TextureObjectView.h @@ -103,6 +103,18 @@ namespace MobileGL::MG_State::GLState { // target - arrays and cube-map arrays included - keeps all its layers in one blob, so // the mapping is "the owner's only target" unless one of the two sides is a cube map. TextureUploadTarget ToOwnerUploadTarget(TextureUploadTarget viewTarget) const; + // WHICH of the owner's layers a given view-side upload target names, in the owner's layer + // numbering. For every view target but a cube map that is just this view's layer origin - + // one target, one layer. A GL_TEXTURE_CUBE_MAP view addresses SIX of the owner's layers at + // once (GL 4.6 core 8.18), so the face its target token names is an index on top of that + // origin, and this is the only place that can express it when the owner keeps every layer + // in one blob: ToOwnerUploadTarget has a single blob to choose from there, so the face + // would otherwise vanish and all six tokens would read the view's first layer. + // + // Every place that turns this view into owner-side bytes goes through here - the blob + // choice, the byte offset, and the dirty region - so the three cannot disagree about which + // layer a face is. + Uint ViewLayerIndex(TextureUploadTarget viewTarget) const; Uint ToOwnerLevel(Uint viewLevel) const { return m_viewMinLevel + viewLevel; } // The owner's level extent rewritten into this view's shape: the owner's layer axis is // collapsed to one slice and the view's own layer count is imposed on the view's layer @@ -114,8 +126,10 @@ namespace MobileGL::MG_State::GLState { // 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; + // A dirty-region origin moved from the view's layer space into the owner's. Takes the view + // target for the same reason LayerByteOffset does: on a cube-map view the target names the + // face, and the region has to name the same owner layer the bytes were written to. + IntVec3 ToOwnerRegionOffset(TextureUploadTarget viewTarget, 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