diff --git a/MobileGL/MG_Backend/BackendObject.h b/MobileGL/MG_Backend/BackendObject.h index 884a59b2..5c7aafff 100644 --- a/MobileGL/MG_Backend/BackendObject.h +++ b/MobileGL/MG_Backend/BackendObject.h @@ -14,6 +14,7 @@ namespace MobileGL { namespace MG_State::GLState { class FramebufferObject; class ITextureObject; + class RenderbufferObject; } enum class BackendType { @@ -24,6 +25,19 @@ namespace MobileGL { }; namespace MG_Backend { + // One endpoint of a glCopyImageSubData. GL 4.6 core 18.3.2 accepts GL_RENDERBUFFER + // alongside the ten whole-image texture targets, and a renderbuffer name lives in a + // namespace of its own - so an endpoint is a sum type, not an ITextureObject. At most + // one of the two pointers is set; neither is set when the name named nothing, which is + // the INVALID_VALUE the frontend validator reports. + struct CopyImageEndpoint { + SharedPtr Texture; + SharedPtr Renderbuffer; + + Bool IsRenderbuffer() const { return Renderbuffer != nullptr; } + Bool Exists() const { return Texture != nullptr || Renderbuffer != nullptr; } + }; + enum class FormatCapability : Uint64 { Creatable = 1ull << 0, @@ -160,9 +174,9 @@ namespace MobileGL { GLsizei height, GLint border); void (*CopyTexSubImage2D)(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height); - void (*CopyImageSubData)(const SharedPtr& srcTexture, + void (*CopyImageSubData)(const CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth); void (*GenerateMipmap)(GLenum target); diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 72842256..f8ba1dc6 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -5708,27 +5708,83 @@ namespace MobileGL::MG_Backend::DirectGLES { // The 1D-array case is not just a rename: GL addresses its layers with y/height while the // ES 2D array that backs it addresses them with z/depth, so the two axes swap with the // target. + // + // GL_RENDERBUFFER is the exception that must NOT be translated: ES 3.2 core (and + // GL_EXT_copy_image) take it as a srcTarget/dstTarget verbatim, while + // ConvertGLEnumToTextureTarget answers Unknown for it and the translation below would hand + // the driver GL_UNKNOWN_MGL. struct GLESCopyImageEndpoint { GLenum target = GL_TEXTURE_2D; + // Exactly one of the two is set. The backend object is kept rather than its id, because + // the id is only stable until the OTHER endpoint syncs (a sync can re-mint a texture), + // so it is read at the point of use. + SharedPtr texture; + SharedPtr renderbuffer; GLint x = 0; GLint y = 0; GLint z = 0; + + Bool IsRenderbuffer() const { return renderbuffer != nullptr; } + GLuint Name() const { + if (renderbuffer) return renderbuffer->GetBackendRenderbufferId(); + return texture ? texture->GetBackendTextureId() : 0u; + } }; - static GLESCopyImageEndpoint MakeGLESCopyImageEndpoint(GLenum appTarget, GLint x, GLint y, GLint z) { - const TextureTarget stateTarget = MG_Util::ConvertGLEnumToTextureTarget(appTarget); - GLESCopyImageEndpoint endpoint{}; - endpoint.target = TextureImpl::ConvertTextureTargetToBackendGLEnum(stateTarget); - if (stateTarget == TextureTarget::Texture1DArray) { - endpoint.x = x; - endpoint.y = 0; - endpoint.z = y; - return endpoint; + // The renderbuffer twin of TextureImpl::SyncTextureObjectToBackend: the same + // find-or-create-then-sync the framebuffer attachment walk does (see SyncAttachmentObject), + // reachable from a path that has a renderbuffer but no framebuffer. + static SharedPtr SyncRenderbufferObjectToBackend( + const SharedPtr& renderbufferObject) { + if (!renderbufferObject) return nullptr; + SharedPtr backendRenderbufferObject; + if (auto* slot = RenderbufferImpl::g_backendRenderbufferObjects.Find(renderbufferObject.get())) { + backendRenderbufferObject = *slot; + } else { + auto& newSlot = RenderbufferImpl::g_backendRenderbufferObjects.GetOrCreate(renderbufferObject); + if (!newSlot) { + newSlot = MakeShared(); + } + backendRenderbufferObject = newSlot; } - endpoint.x = x; - endpoint.y = y; - endpoint.z = z; - return endpoint; + backendRenderbufferObject->SyncToBackend(renderbufferObject); + return backendRenderbufferObject; + } + + static Bool MakeGLESCopyImageEndpoint(const CopyImageEndpoint& endpoint, GLenum appTarget, GLint x, GLint y, + GLint z, GLESCopyImageEndpoint& out) { + if (endpoint.IsRenderbuffer()) { + out.renderbuffer = SyncRenderbufferObjectToBackend(endpoint.Renderbuffer); + if (!out.renderbuffer) return false; + out.target = GL_RENDERBUFFER; + out.x = x; + out.y = y; + out.z = z; + return true; + } + // BY VALUE, not by reference. SyncTextureObjectToBackend hands back a reference to a + // slot inside the backend texture registry, and the second call mutates that very map: + // GetOrCreate indexes it (an insert relocates entries - by rehashing, and also by + // robin-hood displacement well under the load factor), and Find drops any + // entry whose state object has expired - which, with the map open-addressed and erasing + // by shifting the probe cluster backwards, relocates entries other than the erased one. + // Either way a reference taken by the first call is stale by the time the second returns, + // and it is read four more times below. Copying the SharedPtr costs two refcount bumps on + // a path that is already doing a texture copy. + out.texture = TextureImpl::SyncTextureObjectToBackend(endpoint.Texture); + if (!out.texture) return false; + const TextureTarget stateTarget = MG_Util::ConvertGLEnumToTextureTarget(appTarget); + out.target = TextureImpl::ConvertTextureTargetToBackendGLEnum(stateTarget); + if (stateTarget == TextureTarget::Texture1DArray) { + out.x = x; + out.y = 0; + out.z = y; + return true; + } + out.x = x; + out.y = y; + out.z = z; + return true; } // The region extent swaps the same two axes for a 1D array, and does so for whichever side @@ -5744,85 +5800,86 @@ namespace MobileGL::MG_Backend::DirectGLES { std::swap(height, depth); } - void CopyImageSubData(const SharedPtr& srcTexture, + static TextureInternalFormat GetCopyImageEndpointFormat(const CopyImageEndpoint& endpoint) { + if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->GetInternalFormat(); + return endpoint.Texture ? endpoint.Texture->GetFormat() : TextureInternalFormat::Unknown; + } + + void CopyImageSubData(const CopyImageEndpoint& srcEndpoint, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dstEndpoint, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { - // BY VALUE, not by reference. SyncTextureObjectToBackend hands back a reference to a - // slot inside the backend texture registry, and the second call mutates that very map: - // GetOrCreate indexes it (an insert relocates entries - by rehashing, and also by - // robin-hood displacement well under the load factor), and Find drops any - // entry whose state object has expired - which, with the map open-addressed and erasing - // by shifting the probe cluster backwards, relocates entries other than the erased one. - // Either way a reference taken by the first call is stale by the time the second returns, - // and it is read four more times below. Copying the SharedPtr costs two refcount bumps on - // a path that is already doing a texture copy. - const SharedPtr srcBackendTexture = - TextureImpl::SyncTextureObjectToBackend(srcTexture); - const SharedPtr dstBackendTexture = - TextureImpl::SyncTextureObjectToBackend(dstTexture); + GLESCopyImageEndpoint src{}; + GLESCopyImageEndpoint dst{}; // The DirectVulkan half of this entry point died exactly here, on a texture whose sync // produced nothing - and it died in a release build, where the MOBILEGL_ASSERT that was - // supposed to catch it expands to nothing. The four GetBackendTextureId() calls below - // are the same dereference. The frontend validator is what keeps this unreachable and - // what reports the error the application is owed; declining is only how a future gap up - // there stops being a crash. See the level guard in VulkanRenderer::CopyImageSubData. - if (!srcBackendTexture || !dstBackendTexture) { - MGLOG_E_ONCE("%s: source or destination texture failed to sync; declining the copy", __func__); + // supposed to catch it expands to nothing. The four Name() calls below are the same + // dereference. The frontend validator is what keeps this unreachable and what reports + // the error the application is owed; declining is only how a future gap up there stops + // being a crash. See the level guard in VulkanRenderer::CopyImageSubData. + if (!MakeGLESCopyImageEndpoint(srcEndpoint, srcTarget, srcX, srcY, srcZ, src) || + !MakeGLESCopyImageEndpoint(dstEndpoint, dstTarget, dstX, dstY, dstZ, dst)) { + MGLOG_E_ONCE("%s: source or destination image failed to sync; declining the copy", __func__); return; } - const GLESCopyImageEndpoint src = MakeGLESCopyImageEndpoint(srcTarget, srcX, srcY, srcZ); - const GLESCopyImageEndpoint dst = MakeGLESCopyImageEndpoint(dstTarget, dstX, dstY, dstZ); GLsizei copyHeight = srcHeight; GLsizei copyDepth = srcDepth; ApplyGLESCopyImageExtent(srcTarget, dstTarget, copyHeight, copyDepth); - const Bool srcIsDepth = MG_Util::IsDepthFormatInternalFormat(srcTexture->GetFormat()); - const Bool dstIsDepth = MG_Util::IsDepthFormatInternalFormat(dstTexture->GetFormat()); - const Bool srcStencil = MG_Util::IsStencilFormatInternalFormat(srcTexture->GetFormat()); - const Bool dstStencil = MG_Util::IsStencilFormatInternalFormat(dstTexture->GetFormat()); - if (srcIsDepth || dstIsDepth || srcStencil || dstStencil) { + const TextureInternalFormat srcFormat = GetCopyImageEndpointFormat(srcEndpoint); + const TextureInternalFormat dstFormat = GetCopyImageEndpointFormat(dstEndpoint); + // Both emulation fallbacks below are written against TEXTURE ids and texture targets, so + // an endpoint that is a renderbuffer takes the native ES copy - which accepts + // GL_RENDERBUFFER on both sides - and reports rather than mis-dispatches if the driver + // turns it down. + const Bool anyRenderbuffer = src.IsRenderbuffer() || dst.IsRenderbuffer(); + + const Bool srcIsDepth = MG_Util::IsDepthFormatInternalFormat(srcFormat); + const Bool dstIsDepth = MG_Util::IsDepthFormatInternalFormat(dstFormat); + const Bool srcStencil = MG_Util::IsStencilFormatInternalFormat(srcFormat); + const Bool dstStencil = MG_Util::IsStencilFormatInternalFormat(dstFormat); + if (!anyRenderbuffer && (srcIsDepth || dstIsDepth || srcStencil || dstStencil)) { MOBILEGL_ASSERT(srcIsDepth && dstIsDepth && !srcStencil && !dstStencil, "DirectGLES CopyImageSubData only supports depth-only image copies."); MOBILEGL_ASSERT(src.target == GL_TEXTURE_2D && dst.target == GL_TEXTURE_2D, "DirectGLES depth CopyImageSubData only supports GL_TEXTURE_2D."); MOBILEGL_ASSERT(src.z == 0 && dst.z == 0 && copyDepth == 1, "DirectGLES depth CopyImageSubData only supports single-layer copies."); - BlitDepthTexture2D(srcBackendTexture->GetBackendTextureId(), srcLevel, src.x, src.y, srcWidth, copyHeight, - dstBackendTexture->GetBackendTextureId(), dstLevel, dst.x, dst.y, srcWidth, copyHeight); + BlitDepthTexture2D(src.Name(), srcLevel, src.x, src.y, srcWidth, copyHeight, + dst.Name(), dstLevel, dst.x, dst.y, srcWidth, copyHeight); return; } - if (srcTexture->GetFormat() == TextureInternalFormat::R32F || - dstTexture->GetFormat() == TextureInternalFormat::R32F) { + if (!anyRenderbuffer && + (srcFormat == TextureInternalFormat::R32F || dstFormat == TextureInternalFormat::R32F)) { // The single glGetError below decides the fallback dispatch, and // ErrorLopper::Clear is compiled out at the default log level - drain // with the always-live helper so a stale flag cannot misroute a // succeeded native copy into the 2D-only fallback. ClearGLErrors(); - g_GLESFuncs.glCopyImageSubData(srcBackendTexture->GetBackendTextureId(), src.target, srcLevel, src.x, src.y, src.z, - dstBackendTexture->GetBackendTextureId(), dst.target, dstLevel, dst.x, dst.y, dst.z, + g_GLESFuncs.glCopyImageSubData(src.Name(), src.target, srcLevel, src.x, src.y, src.z, + dst.Name(), dst.target, dstLevel, dst.x, dst.y, dst.z, srcWidth, copyHeight, copyDepth); const GLenum copyImageError = g_GLESFuncs.glGetError(); if (copyImageError == GL_NO_ERROR) { return; } - MOBILEGL_ASSERT(IsColorOnlyFormat(srcTexture->GetFormat()) && IsColorOnlyFormat(dstTexture->GetFormat()), + MOBILEGL_ASSERT(IsColorOnlyFormat(srcFormat) && IsColorOnlyFormat(dstFormat), "DirectGLES CopyImageSubData only supports color-only or depth-only copies."); MOBILEGL_ASSERT(src.target == GL_TEXTURE_2D && dst.target == GL_TEXTURE_2D, "DirectGLES color CopyImageSubData only supports GL_TEXTURE_2D."); MOBILEGL_ASSERT(src.z == 0 && dst.z == 0 && copyDepth == 1, "DirectGLES color CopyImageSubData only supports single-layer copies."); - CopyR32FTexture2D(srcBackendTexture->GetBackendTextureId(), srcLevel, src.x, src.y, srcWidth, copyHeight, - dstBackendTexture->GetBackendTextureId(), dst.target, dstLevel, dst.x, dst.y); + CopyR32FTexture2D(src.Name(), srcLevel, src.x, src.y, srcWidth, copyHeight, + dst.Name(), dst.target, dstLevel, dst.x, dst.y); return; } ClearGLErrors(); - g_GLESFuncs.glCopyImageSubData(srcBackendTexture->GetBackendTextureId(), src.target, srcLevel, src.x, src.y, src.z, - dstBackendTexture->GetBackendTextureId(), dst.target, dstLevel, dst.x, dst.y, dst.z, + g_GLESFuncs.glCopyImageSubData(src.Name(), src.target, srcLevel, src.x, src.y, src.z, + dst.Name(), dst.target, dstLevel, dst.x, dst.y, dst.z, srcWidth, copyHeight, copyDepth); // Every error condition glCopyImageSubData has was already ruled out by the frontend // validator, so a driver error here is an internal invariant violation, not something diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h index 81e2b144..94947965 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.h +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.h @@ -76,9 +76,9 @@ namespace MobileGL::MG_Backend::DirectGLES { GLsizei height, GLint border); void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height); - void CopyImageSubData(const SharedPtr& srcTexture, + void CopyImageSubData(const CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth); void GenerateMipmap(GLenum target); diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index a917113c..4bcf74f4 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -632,15 +632,15 @@ namespace MobileGL::MG_Backend::DirectVulkan { MOBILEGL_ASSERT(MG_State::pGLContext, "DirectVulkan::CopyTexSubImage2D called with null GL context"); pVulkanRenderer->CopyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height); } - void CopyImageSubData(const SharedPtr& srcTexture, + void CopyImageSubData(const CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { MOBILEGL_ASSERT(pVulkanRenderer, "DirectVulkan::CopyImageSubData called with null VulkanRenderer"); MOBILEGL_ASSERT(MG_State::pGLContext, "DirectVulkan::CopyImageSubData called with null GL context"); - pVulkanRenderer->CopyImageSubData(srcTexture, srcTarget, srcLevel, srcX, srcY, srcZ, - dstTexture, dstTarget, dstLevel, dstX, dstY, dstZ, + pVulkanRenderer->CopyImageSubData(src, srcTarget, srcLevel, srcX, srcY, srcZ, + dst, dstTarget, dstLevel, dstX, dstY, dstZ, srcWidth, srcHeight, srcDepth); } void GenerateMipmap(GLenum target) { diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h index 2f992399..74241e81 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.h @@ -82,9 +82,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { GLsizei height, GLint border); void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height); - void CopyImageSubData(const SharedPtr& srcTexture, + void CopyImageSubData(const CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth); void GenerateMipmap(GLenum target); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 4fd23b38..2de8c9d0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -8869,7 +8869,7 @@ void main() { // A mixed 2D-array <-> 3D pair is legal because maintenance1 - core since Vulkan 1.1 - // relaxed the old "layerCounts must match" rule into "the 3D side's extent.depth must // equal the array side's layerCount". - struct CopyImageEndpoint { + struct CopyImageSliceMapping { // True for a VK_IMAGE_TYPE_3D image, i.e. slices ride the z axis, not the layer axis. Bool slicesAreDepth = false; // The GL z offset, kept in whichever field this endpoint's image type reads it from. @@ -8883,13 +8883,35 @@ void main() { Int32 OffsetZ() const { return slicesAreDepth ? static_cast(baseSlice) : 0; } }; - Bool TryResolveCopyImageEndpoint(TextureTarget target, - const VkTextureManager::TextureResource& resource, Uint32 mipLevel, - GLint glZ, GLsizei glDepth, CopyImageEndpoint& outEndpoint) { + // The Vulkan image one glCopyImageSubData endpoint names, after the two object kinds GL + // 4.6 core 18.3.2 allows have been collapsed onto the fields this copy reads. A + // renderbuffer is a single-level, single-layer 2D image, so its shape answers are + // constants rather than a mip walk. `trackedLayout` points AT the owning resource's own + // layout field - both resource maps are node-based, so the pointer survives the further + // lookups the clear materialization below makes. + struct CopyImageVkImage { + Bool isRenderbuffer = false; + VkImage image = VK_NULL_HANDLE; + VkImageLayout* trackedLayout = nullptr; + VkImageAspectFlags aspect = VK_IMAGE_ASPECT_NONE; + Uint32 mipLevels = 1; + VkExtent2D extent = {0, 0}; + Uint32 depth = 1; + Uint32 arrayLayers = 1; + }; + + Bool TryResolveCopyImageSliceMapping(TextureTarget target, const CopyImageVkImage& image, Uint32 mipLevel, + GLint glZ, GLsizei glDepth, CopyImageSliceMapping& outMapping) { if (glZ < 0 || glDepth <= 0) { return false; } const Uint32 baseSlice = static_cast(glZ); + if (image.isRenderbuffer) { + // A renderbuffer holds one 2D image and nothing else; GL still requires the + // z/depth pair and it can only name that one slice. + outMapping = {}; + return baseSlice == 0 && glDepth == 1; + } switch (target) { case TextureTarget::Texture1D: case TextureTarget::Texture2D: @@ -8897,12 +8919,12 @@ void main() { case TextureTarget::Texture2DMultisample: // Not layered at all: GL still requires the z/depth pair, and it can only name the // one slice these targets have. - outEndpoint = {}; + outMapping = {}; return baseSlice == 0 && glDepth == 1; case TextureTarget::Texture3D: - outEndpoint.slicesAreDepth = true; - outEndpoint.baseSlice = baseSlice; - outEndpoint.availableSlices = std::max(1u, resource.depth >> mipLevel); + outMapping.slicesAreDepth = true; + outMapping.baseSlice = baseSlice; + outMapping.availableSlices = std::max(1u, image.depth >> mipLevel); return true; case TextureTarget::Texture2DArray: case TextureTarget::Texture2DMultisampleArray: @@ -8911,9 +8933,9 @@ void main() { // A cube map is an array of six faces here (see TryResolveTextureShapeInfo), and GL // numbers its faces on the same z axis an array texture numbers its layers, so both // arrive as a plain layer range. - outEndpoint.slicesAreDepth = false; - outEndpoint.baseSlice = baseSlice; - outEndpoint.availableSlices = resource.arrayLayers; + outMapping.slicesAreDepth = false; + outMapping.baseSlice = baseSlice; + outMapping.availableSlices = image.arrayLayers; return true; default: // GL_TEXTURE_1D_ARRAY carries its layers on the Y axis (srcY/srcHeight), which @@ -8923,15 +8945,20 @@ void main() { return false; } } + + Uint CopyImageEndpointName(const CopyImageEndpoint& endpoint) { + if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->GetExternalIndex(); + return endpoint.Texture ? endpoint.Texture->GetExternalIndex() : 0u; + } } // namespace - void VulkanRenderer::CopyImageSubData(const SharedPtr& srcTexture, + void VulkanRenderer::CopyImageSubData(const CopyImageEndpoint& srcEndpoint, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dstEndpoint, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { - MOBILEGL_ASSERT(srcTexture != nullptr && dstTexture != nullptr, - "CopyImageSubData requires valid source and destination textures."); + MOBILEGL_ASSERT(srcEndpoint.Exists() && dstEndpoint.Exists(), + "CopyImageSubData requires valid source and destination images."); // The frontend already declines a zero or negative extent, so anything else here is a // caller MobileGL wrote - but it still reaches vkCmdCopyImage in a release build, and a // zero extent.depth is as invalid as a zero width. @@ -8948,9 +8975,9 @@ 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 (srcTexture.get() == dstTexture.get()) { - MGLOG_E_ONCE("%s: in-place copy on textureId=%d is not supported; declining the copy", __func__, - srcTexture->GetExternalIndex()); + if (srcEndpoint.Texture == dstEndpoint.Texture && srcEndpoint.Renderbuffer == dstEndpoint.Renderbuffer) { + MGLOG_E_ONCE("%s: in-place copy on objectId=%u is not supported; declining the copy", __func__, + CopyImageEndpointName(srcEndpoint)); return; } @@ -8963,8 +8990,39 @@ void main() { VkRenderPassManager::EndRenderPass(frame.commandBuffer); } - auto* srcResource = m_textureManager->SyncTextureAndGetDescriptor(*srcTexture); - auto* dstResource = m_textureManager->SyncTextureAndGetDescriptor(*dstTexture); + // One resolver for both object kinds. The texture arm is the same + // SyncTextureAndGetDescriptor the copy always used; the renderbuffer arm goes through the + // render-pass manager, which is where a renderbuffer's VkImage lives. + const auto resolveImage = [this](const CopyImageEndpoint& endpoint, CopyImageVkImage& out) { + if (endpoint.IsRenderbuffer()) { + auto* resource = m_renderPassManager->GetOrCreateRenderbufferResource(endpoint.Renderbuffer); + if (resource == nullptr) return false; + out.isRenderbuffer = true; + out.image = resource->image; + out.trackedLayout = &resource->layout; + out.aspect = resource->aspect; + out.mipLevels = 1; + out.extent = resource->extent; + out.depth = 1; + out.arrayLayers = 1; + return out.image != VK_NULL_HANDLE; + } + auto* resource = m_textureManager->SyncTextureAndGetDescriptor(*endpoint.Texture); + if (resource == nullptr) return false; + out.isRenderbuffer = false; + out.image = resource->image; + out.trackedLayout = &resource->layout; + out.aspect = resource->aspect; + out.mipLevels = resource->mipLevels; + out.extent = resource->extent; + out.depth = resource->depth; + out.arrayLayers = resource->arrayLayers; + return true; + }; + CopyImageVkImage srcImage{}; + CopyImageVkImage dstImage{}; + const Bool srcResolved = resolveImage(srcEndpoint, srcImage); + const Bool dstResolved = resolveImage(dstEndpoint, dstImage); // Real checks, not MOBILEGL_ASSERT: the assertions this replaces compile to nothing in // a release build, which is where both observed failures happened - a null resource // dereferenced right below (lavapipe) and a mip level the VkImage does not have handed @@ -8980,29 +9038,29 @@ void main() { // The frontend validator (ValidateTextureLevelExists) is what produces the // GL_INVALID_VALUE the application is actually owed. This guard exists so the next gap // up there declines a copy instead of taking the process down. - if (srcResource == nullptr || dstResource == nullptr) { - MGLOG_E_ONCE("%s: source or destination texture failed to sync; declining the copy", __func__); + if (!srcResolved || !dstResolved) { + MGLOG_E_ONCE("%s: source or destination image failed to sync; declining the copy", __func__); return; } - if (srcLevel < 0 || dstLevel < 0 || static_cast(srcLevel) >= srcResource->mipLevels || - static_cast(dstLevel) >= dstResource->mipLevels) { + 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__, - srcLevel, srcResource->mipLevels, dstLevel, dstResource->mipLevels); + srcLevel, srcImage.mipLevels, dstLevel, dstImage.mipLevels); return; } const VkImageAspectFlags copyAspectMask = - srcResource->aspect & dstResource->aspect & + srcImage.aspect & dstImage.aspect & (VK_IMAGE_ASPECT_COLOR_BIT | VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT); MOBILEGL_ASSERT(copyAspectMask != 0 && - (srcResource->aspect & copyAspectMask) == srcResource->aspect && - (dstResource->aspect & copyAspectMask) == dstResource->aspect, + (srcImage.aspect & copyAspectMask) == srcImage.aspect && + (dstImage.aspect & copyAspectMask) == dstImage.aspect, "CopyImageSubData source and destination aspects are incompatible."); const Uint32 srcMipLevel = static_cast(srcLevel); const Uint32 dstMipLevel = static_cast(dstLevel); - const Uint32 srcMipWidth = std::max(1u, srcResource->extent.width >> srcMipLevel); - const Uint32 srcMipHeight = std::max(1u, srcResource->extent.height >> srcMipLevel); - const Uint32 dstMipWidth = std::max(1u, dstResource->extent.width >> dstMipLevel); - const Uint32 dstMipHeight = std::max(1u, dstResource->extent.height >> dstMipLevel); + const Uint32 srcMipWidth = std::max(1u, srcImage.extent.width >> srcMipLevel); + const Uint32 srcMipHeight = std::max(1u, srcImage.extent.height >> srcMipLevel); + const Uint32 dstMipWidth = std::max(1u, dstImage.extent.width >> dstMipLevel); + const Uint32 dstMipHeight = std::max(1u, dstImage.extent.height >> dstMipLevel); // Promoted for the same reason as the level range above, and it is the same bug class: // a VkImageCopy whose region runs past the image is an out-of-bounds promise to the // driver, and the frontend does not check the region at all (there is a CTS sibling, @@ -9025,10 +9083,10 @@ void main() { // here: every target whose slices this function can address on one of the two Vulkan axes. // A refusal has to be a real decline, not an assertion - the assertion compiled to nothing // in a release build and the unsupported shape reached vkCmdCopyImage anyway. - CopyImageEndpoint srcEndpoint; - CopyImageEndpoint dstEndpoint; - if (!TryResolveCopyImageEndpoint(srcTextureTarget, *srcResource, srcMipLevel, srcZ, srcDepth, srcEndpoint) || - !TryResolveCopyImageEndpoint(dstTextureTarget, *dstResource, dstMipLevel, dstZ, srcDepth, dstEndpoint)) { + CopyImageSliceMapping srcSlices; + CopyImageSliceMapping dstSlices; + if (!TryResolveCopyImageSliceMapping(srcTextureTarget, srcImage, srcMipLevel, srcZ, srcDepth, srcSlices) || + !TryResolveCopyImageSliceMapping(dstTextureTarget, dstImage, dstMipLevel, dstZ, srcDepth, dstSlices)) { MGLOG_E_ONCE("%s: unsupported target pair src=%s dst=%s (srcZ=%d dstZ=%d depth=%d); declining the copy", __func__, MG_Util::ConvertTextureTargetToString(srcTextureTarget).c_str(), MG_Util::ConvertTextureTargetToString(dstTextureTarget).c_str(), srcZ, dstZ, srcDepth); @@ -9039,25 +9097,31 @@ void main() { // shrinks) and a 3D texture by the selected level's depth (which every level halves), so // both come from the endpoint that resolved them. const Uint32 copySliceCount = static_cast(srcDepth); - if (srcEndpoint.baseSlice + copySliceCount > srcEndpoint.availableSlices || - dstEndpoint.baseSlice + copySliceCount > dstEndpoint.availableSlices) { + if (srcSlices.baseSlice + copySliceCount > srcSlices.availableSlices || + dstSlices.baseSlice + copySliceCount > dstSlices.availableSlices) { MGLOG_E_ONCE("%s: slice range outside image bounds (srcZ=%d of %u, dstZ=%d of %u, depth=%d); " "declining the copy", - __func__, srcZ, srcEndpoint.availableSlices, dstZ, dstEndpoint.availableSlices, srcDepth); + __func__, srcZ, srcSlices.availableSlices, dstZ, dstSlices.availableSlices, srcDepth); return; } - const Bool clearReady = MaterializePendingClearForTexture(frame.commandBuffer, *srcTexture); - MOBILEGL_ASSERT(clearReady, "%s: failed to materialize pending clear for source textureId=%d", - __func__, srcTexture->GetExternalIndex()); + const auto materializeClear = [this, &frame](const CopyImageEndpoint& endpoint) { + if (endpoint.IsRenderbuffer()) { + return MaterializePendingClearForRenderbuffer(frame.commandBuffer, endpoint.Renderbuffer); + } + return MaterializePendingClearForTexture(frame.commandBuffer, *endpoint.Texture); + }; + const Bool clearReady = materializeClear(srcEndpoint); + MOBILEGL_ASSERT(clearReady, "%s: failed to materialize pending clear for source objectId=%u", + __func__, CopyImageEndpointName(srcEndpoint)); // A clear still parked on the destination would otherwise materialize AFTER this copy and // wipe the texels it just wrote. - const Bool dstClearReady = MaterializePendingClearForTexture(frame.commandBuffer, *dstTexture); - MOBILEGL_ASSERT(dstClearReady, "%s: failed to materialize pending clear for destination textureId=%d", - __func__, dstTexture->GetExternalIndex()); + const Bool dstClearReady = materializeClear(dstEndpoint); + MOBILEGL_ASSERT(dstClearReady, "%s: failed to materialize pending clear for destination objectId=%u", + __func__, CopyImageEndpointName(dstEndpoint)); - const VkImageLayout srcOriginalLayout = srcResource->layout; - const VkImageLayout dstOriginalLayout = dstResource->layout; + const VkImageLayout srcOriginalLayout = *srcImage.trackedLayout; + const VkImageLayout dstOriginalLayout = *dstImage.trackedLayout; // A layout of UNDEFINED means nothing has ever been written to the image, which on the // SOURCE side is glTexStorage without an upload: legal GL, and the texels it copies are // undefined by the same spec sentence that lets the application ask. Both sides therefore @@ -9083,15 +9147,15 @@ void main() { // [baseSlice, baseSlice + depth) the slice mapping above hands the copy. if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { Bool srcReady = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, srcResource->image, srcResource->layout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + frame.commandBuffer, srcImage.image, *srcImage.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, - srcResource->aspect, 0, srcResource->mipLevels); + srcImage.aspect, 0, srcImage.mipLevels); MOBILEGL_ASSERT(srcReady, "%s: failed to transition undefined source image", __func__); - srcCopyLayout = srcResource->layout; + srcCopyLayout = *srcImage.trackedLayout; } else { Bool srcReady = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, srcResource->image, srcCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + frame.commandBuffer, srcImage.image, srcCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, srcStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, srcAccessMask, VK_ACCESS_TRANSFER_READ_BIT, copyAspectMask, srcMipLevel, 1); MOBILEGL_ASSERT(srcReady, "%s: failed to transition source image", __func__); @@ -9103,15 +9167,15 @@ void main() { VkImageLayout dstCopyLayout = dstOriginalLayout; if (dstOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { Bool dstReady = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, dstResource->image, dstResource->layout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + frame.commandBuffer, dstImage.image, *dstImage.trackedLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, dstStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, - dstResource->aspect, 0, dstResource->mipLevels); + dstImage.aspect, 0, dstImage.mipLevels); MOBILEGL_ASSERT(dstReady, "%s: failed to transition undefined destination image", __func__); - dstCopyLayout = dstResource->layout; + dstCopyLayout = *dstImage.trackedLayout; } else { Bool dstReady = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, dstResource->image, dstCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + frame.commandBuffer, dstImage.image, dstCopyLayout, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, dstStageMask, VK_PIPELINE_STAGE_TRANSFER_BIT, dstAccessMask, VK_ACCESS_TRANSFER_WRITE_BIT, copyAspectMask, dstMipLevel, 1); MOBILEGL_ASSERT(dstReady, "%s: failed to transition destination image", __func__); @@ -9121,18 +9185,18 @@ void main() { // on extent.depth as soon as either endpoint IS: a 3D image's subresource is always the // single layer (0, 1) and its slices are counted by the depth of the copy extent. With two // non-3D endpoints both layer counts carry it and extent.depth stays 1. - const Bool copyCrossesDepthAxis = srcEndpoint.slicesAreDepth || dstEndpoint.slicesAreDepth; + const Bool copyCrossesDepthAxis = srcSlices.slicesAreDepth || dstSlices.slicesAreDepth; VkImageCopy copyRegion{}; copyRegion.srcSubresource.aspectMask = copyAspectMask; copyRegion.srcSubresource.mipLevel = srcMipLevel; - copyRegion.srcSubresource.baseArrayLayer = srcEndpoint.BaseArrayLayer(); - copyRegion.srcSubresource.layerCount = srcEndpoint.slicesAreDepth ? 1u : copySliceCount; - copyRegion.srcOffset = {srcX, srcY, srcEndpoint.OffsetZ()}; + copyRegion.srcSubresource.baseArrayLayer = srcSlices.BaseArrayLayer(); + copyRegion.srcSubresource.layerCount = srcSlices.slicesAreDepth ? 1u : copySliceCount; + copyRegion.srcOffset = {srcX, srcY, srcSlices.OffsetZ()}; copyRegion.dstSubresource.aspectMask = copyAspectMask; copyRegion.dstSubresource.mipLevel = dstMipLevel; - copyRegion.dstSubresource.baseArrayLayer = dstEndpoint.BaseArrayLayer(); - copyRegion.dstSubresource.layerCount = dstEndpoint.slicesAreDepth ? 1u : copySliceCount; - copyRegion.dstOffset = {dstX, dstY, dstEndpoint.OffsetZ()}; + copyRegion.dstSubresource.baseArrayLayer = dstSlices.BaseArrayLayer(); + copyRegion.dstSubresource.layerCount = dstSlices.slicesAreDepth ? 1u : copySliceCount; + copyRegion.dstOffset = {dstX, dstY, dstSlices.OffsetZ()}; copyRegion.extent = {static_cast(srcWidth), static_cast(srcHeight), copyCrossesDepthAxis ? copySliceCount : 1u}; MGLOG_D("CopyImageSubData: src(target=%s level=%u layer=%u+%u z=%d) -> dst(target=%s level=%u layer=%u+%u " @@ -9143,8 +9207,8 @@ void main() { copyRegion.dstSubresource.baseArrayLayer, copyRegion.dstSubresource.layerCount, copyRegion.dstOffset.z, srcWidth, srcHeight, copyRegion.extent.depth); vkCmdCopyImage(frame.commandBuffer, - srcResource->image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - dstResource->image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + srcImage.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + dstImage.image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, ©Region); VkPipelineStageFlags srcRestoreStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; @@ -9152,14 +9216,14 @@ void main() { GetImageTransitionDestinationState(srcRestoreLayout, srcRestoreStageMask, srcRestoreAccessMask); if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { Bool srcRestored = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, srcResource->image, srcResource->layout, srcRestoreLayout, + frame.commandBuffer, srcImage.image, *srcImage.trackedLayout, srcRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask, VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, - srcResource->aspect, 0, srcResource->mipLevels); + srcImage.aspect, 0, srcImage.mipLevels); MOBILEGL_ASSERT(srcRestored, "%s: failed to restore undefined source image layout", __func__); } else { Bool srcRestored = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, srcResource->image, srcCopyLayout, srcRestoreLayout, + frame.commandBuffer, srcImage.image, srcCopyLayout, srcRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, srcRestoreStageMask, VK_ACCESS_TRANSFER_READ_BIT, srcRestoreAccessMask, copyAspectMask, srcMipLevel, 1); MOBILEGL_ASSERT(srcRestored, "%s: failed to restore source image layout", __func__); @@ -9170,14 +9234,14 @@ void main() { GetImageTransitionDestinationState(dstRestoreLayout, dstRestoreStageMask, dstRestoreAccessMask); if (dstOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { Bool dstRestored = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, dstResource->image, dstResource->layout, dstRestoreLayout, + frame.commandBuffer, dstImage.image, *dstImage.trackedLayout, dstRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, - dstResource->aspect, 0, dstResource->mipLevels); + dstImage.aspect, 0, dstImage.mipLevels); MOBILEGL_ASSERT(dstRestored, "%s: failed to restore undefined destination image layout", __func__); } else { Bool dstRestored = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, dstResource->image, dstCopyLayout, dstRestoreLayout, + frame.commandBuffer, dstImage.image, dstCopyLayout, dstRestoreLayout, VK_PIPELINE_STAGE_TRANSFER_BIT, dstRestoreStageMask, VK_ACCESS_TRANSFER_WRITE_BIT, dstRestoreAccessMask, copyAspectMask, dstMipLevel, 1); MOBILEGL_ASSERT(dstRestored, "%s: failed to restore destination image layout", __func__); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 4df06667..fff16041 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -23,6 +23,7 @@ #include "VkTimerQueryManager.h" #include "MG_Util/Math/VectorTypes.h" #include +#include #include #include "../VkIncludes.h" @@ -197,9 +198,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { GLbitfield mask, GLenum filter); void CopyTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint x, GLint y, GLsizei width, GLsizei height); - void CopyImageSubData(const SharedPtr& srcTexture, + void CopyImageSubData(const CopyImageEndpoint& srcEndpoint, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const CopyImageEndpoint& dstEndpoint, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth); void GenerateMipmap(GLenum target); diff --git a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp index c35b527c..a9539587 100644 --- a/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp +++ b/MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp @@ -3412,9 +3412,9 @@ namespace MobileGL::MG_Impl::GLImpl { MG_Backend::gBackendFunctionsTable.GL.CopyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height); } - void CopyImageSubData_Backend(const SharedPtr& srcTexture, + void CopyImageSubData_Backend(const MG_Backend::CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, + const MG_Backend::CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { auto copyImageSubData = MG_Backend::gBackendFunctionsTable.GL.CopyImageSubData; @@ -3425,7 +3425,7 @@ namespace MobileGL::MG_Impl::GLImpl { "Backend does not support image-to-image copies.")); return; } - copyImageSubData(srcTexture, srcTarget, srcLevel, srcX, srcY, srcZ, dstTexture, dstTarget, dstLevel, dstX, + copyImageSubData(src, srcTarget, srcLevel, srcX, srcY, srcZ, dst, dstTarget, dstLevel, dstX, dstY, dstZ, srcWidth, srcHeight, srcDepth); } @@ -3472,9 +3472,9 @@ namespace MobileGL::MG_Impl::GLImpl { // the ~30 entry points that reach it through a BOUND object (where the name was never // in question and the fault is the binding), so this is a local rule rather than a // change to the helper. - Bool ValidateCopyImageObjectExists(const SharedPtr& textureObject, + Bool ValidateCopyImageObjectExists(const MG_Backend::CopyImageEndpoint& endpoint, const char* endpointName) { - if (textureObject) return true; + if (endpoint.Exists()) return true; MG_State::pGLContext->RecordError( ErrorCode::InvalidValue, MakeUnique( @@ -3498,21 +3498,75 @@ namespace MobileGL::MG_Impl::GLImpl { MG_Util::ConvertTextureTargetToString(textureObject->GetTarget())))); return false; } - } // namespace - Bool ValidateCopyImageSubData_State(const SharedPtr& srcTexture, - GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, - const SharedPtr& dstTexture, - GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, - GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { - if (!ValidateCopyImageObjectExists(srcTexture, "source") || - !ValidateCopyImageObjectExists(dstTexture, "destination")) { + // ---- The questions ValidateCopyImageSubData_State asks of one endpoint. --------------- + // A renderbuffer answers all of them directly: it has exactly one image, no mip chain and + // no sampler state, and it carries its own internal format and extent. + + Int GetCopyImageEndpointSamples(const MG_Backend::CopyImageEndpoint& endpoint) { + if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->GetSamples(); + return endpoint.Texture->GetSamples(); + } + + TextureInternalFormat GetCopyImageEndpointFormat(const MG_Backend::CopyImageEndpoint& endpoint) { + if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->GetInternalFormat(); + return endpoint.Texture->GetFormat(); + } + + // A renderbuffer has level 0 and nothing else, and the failure is the same INVALID_VALUE + // ValidateTextureLevelExists records for a level a texture does not have. + Bool ValidateCopyImageEndpointLevelExists(const MG_Backend::CopyImageEndpoint& endpoint, GLint level, + const char* caller) { + if (!endpoint.IsRenderbuffer()) { + return TextureImpl::ValidateTextureLevelExists(endpoint.Texture, level, caller); + } + if (level == 0) return true; + MG_State::pGLContext->RecordError( + ErrorCode::InvalidValue, + MakeUnique("MG_Impl/GLImpl", caller, "A renderbuffer has only level 0.")); return false; } - const auto srcTextureTarget = MG_Util::ConvertGLEnumToTextureTarget(srcTarget); - const auto dstTextureTarget = MG_Util::ConvertGLEnumToTextureTarget(dstTarget); - if (!TextureImpl::ValidateTextureTarget(srcTextureTarget) || - !TextureImpl::ValidateTextureTarget(dstTextureTarget)) { + + Bool IsCopyImageEndpointComplete(const MG_Backend::CopyImageEndpoint& endpoint) { + // A renderbuffer is complete exactly when it has storage - there is nothing else it + // could be missing. + if (endpoint.IsRenderbuffer()) return endpoint.Renderbuffer->IsAllocated(); + return endpoint.Texture && endpoint.Texture->IsComplete(); + } + + GLenum GetCopyImageEndpointCompressedFormat(const MG_Backend::CopyImageEndpoint& endpoint, + TextureUploadTarget uploadTarget, GLint level) { + if (endpoint.IsRenderbuffer()) return GL_NONE; + return GetCompressedLevelFormat(endpoint.Texture, uploadTarget, level); + } + + IntVec3 GetCopyImageEndpointLevelSize(const MG_Backend::CopyImageEndpoint& endpoint, + TextureUploadTarget uploadTarget, GLint level) { + if (endpoint.IsRenderbuffer()) { + return {endpoint.Renderbuffer->GetWidth(), endpoint.Renderbuffer->GetHeight(), 1}; + } + return GetCopyImageLevelSize(endpoint.Texture, uploadTarget, level); + } + } // namespace + + Bool ValidateCopyImageSubData_State(const MG_Backend::CopyImageEndpoint& src, + GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, + const MG_Backend::CopyImageEndpoint& dst, + GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, + GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { + if (!ValidateCopyImageObjectExists(src, "source") || + !ValidateCopyImageObjectExists(dst, "destination")) { + return false; + } + // GL_RENDERBUFFER has no TextureTarget to convert to, and it needs none: it is its own + // whole-image target, and the endpoint that carries it was resolved from the renderbuffer + // namespace, so it matches its object by construction. + const auto srcTextureTarget = + src.IsRenderbuffer() ? TextureTarget::Unknown : MG_Util::ConvertGLEnumToTextureTarget(srcTarget); + const auto dstTextureTarget = + dst.IsRenderbuffer() ? TextureTarget::Unknown : MG_Util::ConvertGLEnumToTextureTarget(dstTarget); + if ((!src.IsRenderbuffer() && !TextureImpl::ValidateTextureTarget(srcTextureTarget)) || + (!dst.IsRenderbuffer() && !TextureImpl::ValidateTextureTarget(dstTextureTarget))) { return false; } // GL_TEXTURE_BUFFER and the cube FACE enums convert to a target this frontend knows, but @@ -3520,8 +3574,8 @@ namespace MobileGL::MG_Impl::GLImpl { if (!ValidateCopyImageTarget(srcTarget, "source") || !ValidateCopyImageTarget(dstTarget, "destination")) { return false; } - if (!ValidateCopyImageTargetMatchesObject(srcTexture, srcTextureTarget, "source") || - !ValidateCopyImageTargetMatchesObject(dstTexture, dstTextureTarget, "destination")) { + if (!ValidateCopyImageTargetMatchesObject(src.Texture, srcTextureTarget, "source") || + !ValidateCopyImageTargetMatchesObject(dst.Texture, dstTextureTarget, "destination")) { return false; } if (!TextureImpl::ValidateTextureLevelNumber(srcLevel) || @@ -3535,8 +3589,8 @@ namespace MobileGL::MG_Impl::GLImpl { // driver as an out-of-range mip index - on Adreno that is a SIGSEGV inside // vkCmdCopyImage, which is what KHR-GL43.copy_image.non_existent_mipmap used to do to // the whole glcts process. The answer the spec asks for is GL_INVALID_VALUE. - if (!TextureImpl::ValidateTextureLevelExists(srcTexture, srcLevel, __func__) || - !TextureImpl::ValidateTextureLevelExists(dstTexture, dstLevel, __func__)) { + if (!ValidateCopyImageEndpointLevelExists(src, srcLevel, __func__) || + !ValidateCopyImageEndpointLevelExists(dst, dstLevel, __func__)) { return false; } if (srcWidth < 0 || srcHeight < 0 || srcDepth < 0) { @@ -3552,37 +3606,41 @@ namespace MobileGL::MG_Impl::GLImpl { // A multisample image can only be copied to one with the same sample count, and a // single-sample image reports zero - so this one comparison is also what rejects // copying between a multisample target and a non-multisample one. - if (srcTexture->GetSamples() != dstTexture->GetSamples()) { + const Int srcSamples = GetCopyImageEndpointSamples(src); + const Int dstSamples = GetCopyImageEndpointSamples(dst); + if (srcSamples != dstSamples) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeUnique( "MG_Impl/GLImpl", __func__, std::format("The two images have different sample counts ({} vs. {}).", - srcTexture->GetSamples(), dstTexture->GetSamples()))); + srcSamples, dstSamples))); return false; } // 18.3.2: both images must be complete. An incomplete one has no defined texels to copy // and no defined storage to copy into. - if (!srcTexture->IsComplete() || !dstTexture->IsComplete()) { + const Bool srcComplete = IsCopyImageEndpointComplete(src); + const Bool dstComplete = IsCopyImageEndpointComplete(dst); + if (!srcComplete || !dstComplete) { MG_State::pGLContext->RecordError( ErrorCode::InvalidOperation, MakeUnique( "MG_Impl/GLImpl", __func__, std::format("A copied image is incomplete (source complete: {}, destination complete: {}).", - srcTexture->IsComplete(), dstTexture->IsComplete()))); + srcComplete, dstComplete))); return false; } - const auto srcUploadTarget = GetPrimaryUploadTarget(srcTexture); - const auto dstUploadTarget = GetPrimaryUploadTarget(dstTexture); + const auto srcUploadTarget = GetPrimaryUploadTarget(src.Texture); + const auto dstUploadTarget = GetPrimaryUploadTarget(dst.Texture); const auto srcBlock = TextureImpl::ResolveCopyImageTexelBlock( - srcTexture->GetFormat(), GetCompressedLevelFormat(srcTexture, srcUploadTarget, srcLevel)); + GetCopyImageEndpointFormat(src), GetCopyImageEndpointCompressedFormat(src, srcUploadTarget, srcLevel)); const auto dstBlock = TextureImpl::ResolveCopyImageTexelBlock( - dstTexture->GetFormat(), GetCompressedLevelFormat(dstTexture, dstUploadTarget, dstLevel)); + GetCopyImageEndpointFormat(dst), GetCopyImageEndpointCompressedFormat(dst, dstUploadTarget, dstLevel)); if (!TextureImpl::ValidateCopyImageFormatCompatibility(srcBlock, dstBlock)) { return false; } - const IntVec3 srcLevelSize = GetCopyImageLevelSize(srcTexture, srcUploadTarget, srcLevel); - const IntVec3 dstLevelSize = GetCopyImageLevelSize(dstTexture, dstUploadTarget, dstLevel); + const IntVec3 srcLevelSize = GetCopyImageEndpointLevelSize(src, srcUploadTarget, srcLevel); + const IntVec3 dstLevelSize = GetCopyImageEndpointLevelSize(dst, dstUploadTarget, dstLevel); if (!TextureImpl::ValidateCopyImageBlockAlignment(srcBlock, srcX, srcY, srcWidth, srcHeight, srcLevelSize.x(), srcLevelSize.y(), "source") || !TextureImpl::ValidateCopyImageBlockAlignment(dstBlock, dstX, dstY, srcWidth, srcHeight, @@ -5780,17 +5838,29 @@ namespace MobileGL::MG_Impl::GLImpl { GLuint dstName, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { // A missing name is INVALID_VALUE here, where GetTextureObjectByName's own diagnostic is - // INVALID_OPERATION - so resolve through the plain lookup, which answers a null + // INVALID_OPERATION - so resolve through the plain lookups, which answer a null // SharedPtr, and let the validator record the error this entry point owes. - const SharedPtr srcTexture = - MG_State::pGLContext->GetTextureObject(srcName); - const SharedPtr dstTexture = - MG_State::pGLContext->GetTextureObject(dstName); - if (!ValidateCopyImageSubData_State(srcTexture, srcTarget, srcLevel, srcX, srcY, dstTexture, dstTarget, + // + // The TARGET picks the namespace: GL 4.6 core 18.3.2 accepts GL_RENDERBUFFER, and a + // renderbuffer name has nothing to do with a texture name. Resolving both through + // GetTextureObject made every renderbuffer endpoint INVALID_VALUE - or, when the number + // happened to collide with a live texture, INVALID_ENUM from the target check. + const auto resolveEndpoint = [](GLuint name, GLenum target) { + MG_Backend::CopyImageEndpoint endpoint{}; + if (target == GL_RENDERBUFFER) { + endpoint.Renderbuffer = MG_State::pGLContext->GetRenderbufferObject(name); + } else { + endpoint.Texture = MG_State::pGLContext->GetTextureObject(name); + } + return endpoint; + }; + const MG_Backend::CopyImageEndpoint src = resolveEndpoint(srcName, srcTarget); + const MG_Backend::CopyImageEndpoint dst = resolveEndpoint(dstName, dstTarget); + if (!ValidateCopyImageSubData_State(src, srcTarget, srcLevel, srcX, srcY, dst, dstTarget, dstLevel, dstX, dstY, srcWidth, srcHeight, srcDepth)) { return; } - CopyImageSubData_Backend(srcTexture, srcTarget, srcLevel, srcX, srcY, srcZ, dstTexture, dstTarget, dstLevel, + CopyImageSubData_Backend(src, srcTarget, srcLevel, srcX, srcY, srcZ, dst, dstTarget, dstLevel, dstX, dstY, dstZ, srcWidth, srcHeight, srcDepth); } diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 19279b0d..4658a7b3 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -4100,24 +4100,25 @@ namespace { GLint SrcZ = -1; GLint DstZ = -1; GLsizei Depth = -1; + Bool SrcIsRenderbuffer = false; + Bool DstIsRenderbuffer = false; } g_copyImageSubDataCall; - void RecordCopyImageSubData(const SharedPtr& srcTexture, GLenum srcTarget, + void RecordCopyImageSubData(const MG_Backend::CopyImageEndpoint& src, GLenum srcTarget, GLint srcLevel, GLint srcX, GLint srcY, GLint srcZ, - const SharedPtr& dstTexture, GLenum dstTarget, + const MG_Backend::CopyImageEndpoint& dst, GLenum dstTarget, GLint dstLevel, GLint dstX, GLint dstY, GLint dstZ, GLsizei srcWidth, GLsizei srcHeight, GLsizei srcDepth) { - (void)srcTexture; (void)srcLevel; (void)srcX; (void)srcY; - (void)dstTexture; (void)dstLevel; (void)dstX; (void)dstY; (void)srcWidth; (void)srcHeight; - g_copyImageSubDataCall = {true, srcTarget, dstTarget, srcZ, dstZ, srcDepth}; + g_copyImageSubDataCall = {true, srcTarget, dstTarget, srcZ, + dstZ, srcDepth, src.IsRenderbuffer(), dst.IsRenderbuffer()}; } // Two storage-backed 2D textures of the requested formats, so a copy between them is a legal @@ -4388,3 +4389,102 @@ TEST_F(TextureTest, CopyImageSubDataPassesTheRectangleTargetThroughUntranslated) EXPECT_EQ(g_copyImageSubDataCall.DstTarget, static_cast(GL_TEXTURE_RECTANGLE)); EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); } + +// GL 4.6 core 18.3.2 accepts GL_RENDERBUFFER as an endpoint target, and a renderbuffer name lives +// in its own namespace. Resolving BOTH names through the texture namespace answered a null object +// for every renderbuffer endpoint, so all 74 conformance cases that name one - the whole +// texture<->renderbuffer half of KHR-GL43.copy_image, plus its smoke test - reported +// GL_INVALID_VALUE. The endpoint is a sum type now; the target picks the namespace. +TEST_F(TextureTest, CopyImageSubDataResolvesARenderbufferEndpointInTheRenderbufferNamespace) { + const ScopedTextureBackendFunctionsOverride backendGuard; + MG_Backend::gBackendFunctionsTable.GL.CopyImageSubData = RecordCopyImageSubData; + g_copyImageSubDataCall = {}; + + GLuint texture = 0; + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &texture); + MG_Impl::GLImpl::TextureStorage2D(texture, 1, GL_RGBA8, 8, 8); + GLuint renderbuffer = 0; + MG_Impl::GLImpl::CreateRenderbuffers(1, &renderbuffer); + MG_Impl::GLImpl::NamedRenderbufferStorage(renderbuffer, GL_RGBA8, 8, 8); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(texture, GL_TEXTURE_2D, 0, 0, 0, 0, renderbuffer, GL_RENDERBUFFER, 0, 0, 0, 0, + 4, 4, 1); + EXPECT_TRUE(g_copyImageSubDataCall.Called); + EXPECT_FALSE(g_copyImageSubDataCall.SrcIsRenderbuffer); + EXPECT_TRUE(g_copyImageSubDataCall.DstIsRenderbuffer); + EXPECT_EQ(g_copyImageSubDataCall.DstTarget, static_cast(GL_RENDERBUFFER)); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + // ...and back the other way, which is the second half of the conformance case's two-copy + // shape (texture -> renderbuffer -> texture). + g_copyImageSubDataCall = {}; + MG_Impl::GLImpl::CopyImageSubData(renderbuffer, GL_RENDERBUFFER, 0, 0, 0, 0, texture, GL_TEXTURE_2D, 0, 0, 0, 0, + 4, 4, 1); + EXPECT_TRUE(g_copyImageSubDataCall.Called); + EXPECT_TRUE(g_copyImageSubDataCall.SrcIsRenderbuffer); + EXPECT_FALSE(g_copyImageSubDataCall.DstIsRenderbuffer); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + +// Renderbuffer to renderbuffer, the shape neither endpoint could take before, plus the negative +// that pins which table was consulted: with GL_RENDERBUFFER named, a number that is not a live +// RENDERBUFFER is INVALID_VALUE - the texture table is never asked. +TEST_F(TextureTest, CopyImageSubDataKeepsTheTwoNameNamespacesApart) { + const ScopedTextureBackendFunctionsOverride backendGuard; + MG_Backend::gBackendFunctionsTable.GL.CopyImageSubData = RecordCopyImageSubData; + g_copyImageSubDataCall = {}; + + GLuint srcRenderbuffer = 0; + GLuint dstRenderbuffer = 0; + MG_Impl::GLImpl::CreateRenderbuffers(1, &srcRenderbuffer); + MG_Impl::GLImpl::CreateRenderbuffers(1, &dstRenderbuffer); + MG_Impl::GLImpl::NamedRenderbufferStorage(srcRenderbuffer, GL_RGBA8, 8, 8); + MG_Impl::GLImpl::NamedRenderbufferStorage(dstRenderbuffer, GL_RGBA8, 8, 8); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(srcRenderbuffer, GL_RENDERBUFFER, 0, 0, 0, 0, dstRenderbuffer, + GL_RENDERBUFFER, 0, 0, 0, 0, 4, 4, 1); + EXPECT_TRUE(g_copyImageSubDataCall.Called); + EXPECT_TRUE(g_copyImageSubDataCall.SrcIsRenderbuffer); + EXPECT_TRUE(g_copyImageSubDataCall.DstIsRenderbuffer); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + g_copyImageSubDataCall = {}; + MG_Impl::GLImpl::CopyImageSubData(srcRenderbuffer, GL_RENDERBUFFER, 0, 0, 0, 0, 4243, GL_RENDERBUFFER, 0, 0, 0, + 0, 4, 4, 1); + EXPECT_FALSE(g_copyImageSubDataCall.Called); + ExpectSingleGlError(GL_INVALID_VALUE); +} + +// A renderbuffer has exactly one image, so any level above zero is the same INVALID_VALUE a +// texture gets for a level it does not have - and an unallocated one is an incomplete image, +// which 18.3.2 spells INVALID_OPERATION. +TEST_F(TextureTest, CopyImageSubDataChecksARenderbufferLevelAndStorage) { + const ScopedTextureBackendFunctionsOverride backendGuard; + MG_Backend::gBackendFunctionsTable.GL.CopyImageSubData = RecordCopyImageSubData; + g_copyImageSubDataCall = {}; + + GLuint texture = 0; + MG_Impl::GLImpl::CreateTextures(GL_TEXTURE_2D, 1, &texture); + MG_Impl::GLImpl::TextureStorage2D(texture, 1, GL_RGBA8, 8, 8); + GLuint renderbuffer = 0; + MG_Impl::GLImpl::CreateRenderbuffers(1, &renderbuffer); + MG_Impl::GLImpl::NamedRenderbufferStorage(renderbuffer, GL_RGBA8, 8, 8); + ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + MG_Impl::GLImpl::CopyImageSubData(texture, GL_TEXTURE_2D, 0, 0, 0, 0, renderbuffer, GL_RENDERBUFFER, 1, 0, 0, 0, + 4, 4, 1); + EXPECT_FALSE(g_copyImageSubDataCall.Called); + ExpectSingleGlError(GL_INVALID_VALUE); + + g_copyImageSubDataCall = {}; + GLuint emptyRenderbuffer = 0; + MG_Impl::GLImpl::CreateRenderbuffers(1, &emptyRenderbuffer); + DrainPendingGlErrors(); + + MG_Impl::GLImpl::CopyImageSubData(texture, GL_TEXTURE_2D, 0, 0, 0, 0, emptyRenderbuffer, GL_RENDERBUFFER, 0, 0, + 0, 0, 4, 4, 1); + EXPECT_FALSE(g_copyImageSubDataCall.Called); + ExpectSingleGlError(GL_INVALID_OPERATION); +}