diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp index 26b7d6b9..629dc488 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.cpp @@ -17,6 +17,45 @@ namespace MobileGL::MG_Backend::DirectVulkan { target <= TextureUploadTarget::CubeMapNegativeZ; } + VkClearColorValue MakeVkClearColorValue(const ClearAttachmentPayload& payload, Bool formatLacksAlpha) { + VkClearColorValue clearValue{}; + switch (payload.colorEncoding) { + case ClearColorEncoding::Int: + clearValue.int32[0] = payload.colorInt.x(); + clearValue.int32[1] = payload.colorInt.y(); + clearValue.int32[2] = payload.colorInt.z(); + clearValue.int32[3] = formatLacksAlpha ? 1 : payload.colorInt.w(); + break; + case ClearColorEncoding::Uint: + clearValue.uint32[0] = payload.colorUint.x(); + clearValue.uint32[1] = payload.colorUint.y(); + clearValue.uint32[2] = payload.colorUint.z(); + clearValue.uint32[3] = formatLacksAlpha ? 1u : payload.colorUint.w(); + break; + case ClearColorEncoding::Float: + clearValue.float32[0] = payload.color.x(); + clearValue.float32[1] = payload.color.y(); + clearValue.float32[2] = payload.color.z(); + clearValue.float32[3] = formatLacksAlpha ? 1.0f : payload.color.w(); + break; + } + return clearValue; + } + + void ForceOpaqueClearAlpha(ClearAttachmentPayload& payload) { + switch (payload.colorEncoding) { + case ClearColorEncoding::Int: + payload.colorInt = IntVec4(payload.colorInt.x(), payload.colorInt.y(), payload.colorInt.z(), 1); + break; + case ClearColorEncoding::Uint: + payload.colorUint = UintVec4(payload.colorUint.x(), payload.colorUint.y(), payload.colorUint.z(), 1u); + break; + case ClearColorEncoding::Float: + payload.color = FloatVec4(payload.color.x(), payload.color.y(), payload.color.z(), 1.0f); + break; + } + } + static Bool PendingClearMatchesTextureIdentity(const PendingClearKey& key, const TextureIdentity& identity) { return key.texture == identity.texture && key.textureLifetimeId == identity.lifetimeId; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.h index 3d4b8c2e..bdcd7d8a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkClearManager.h @@ -24,13 +24,32 @@ namespace MobileGL::MG_Backend::DirectVulkan { Uint32 stencil{}; }; + // A colour clear reaches us from one of glClear/ClearBufferfv, ClearBufferiv or + // ClearBufferuiv, and Vulkan reads VkClearColorValue's union according to the destination + // image's format rather than converting between the members - a float written where an + // integer format is expected is reinterpreted bit for bit, not rounded. Remember which entry + // point supplied the value so the member written when the clear is materialized matches. + enum class ClearColorEncoding : Uint8 { Float, Int, Uint }; + struct ClearAttachmentPayload { GLbitfield mask = 0; FloatVec4 color = FloatVec4(0.0f, 0.0f, 0.0f, 0.0f); + ClearColorEncoding colorEncoding = ClearColorEncoding::Float; + IntVec4 colorInt = IntVec4(0, 0, 0, 0); + UintVec4 colorUint = UintVec4(0u, 0u, 0u, 0u); Float depth = 1.0f; Uint32 stencil = 0; }; + // Builds the clear value for `payload` in the union member its encoding calls for. + // `formatLacksAlpha` applies GL's rule that a format without an alpha channel reads as one, + // expressed in whichever type matches (GL 4.6 core 15.2.3). + VkClearColorValue MakeVkClearColorValue(const ClearAttachmentPayload& payload, Bool formatLacksAlpha); + + // Applies that same rule in place, for the paths that have to bake it into the payload before + // the destination is known. + void ForceOpaqueClearAlpha(ClearAttachmentPayload& payload); + struct PendingClearKey { MG_State::GLState::ITextureObject* texture = nullptr; Uint64 textureLifetimeId = 0; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp index 4d13b8bf..3a2bcea0 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkRenderPassManager.cpp @@ -50,7 +50,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - static Float ResolveColorClearAlpha(const MG_State::GLState::ITextureObject* texture, Float requestedAlpha) { + static Bool ColorFormatLacksAlpha(const MG_State::GLState::ITextureObject* texture) { + return texture != nullptr && MG_Util::GetBaseInternalFormatComponentCount(texture->GetFormat()) == 3; + } + + [[maybe_unused]] static Float ResolveColorClearAlpha(const MG_State::GLState::ITextureObject* texture, Float requestedAlpha) { if (texture != nullptr && MG_Util::GetBaseInternalFormatComponentCount(texture->GetFormat()) == 3) { return 1.0f; } @@ -526,7 +530,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { pending.renderbuffer = renderbuffer; pending.payload.mask |= clearPayload.mask; if ((clearPayload.mask & GL_COLOR_BUFFER_BIT) != 0) { + // The whole colour description, not just the float vector: an integer clear keeps its + // value in colorInt/colorUint, and dropping the encoding here would leave the pending + // clear reading as an all-zero float one. pending.payload.color = clearPayload.color; + pending.payload.colorEncoding = clearPayload.colorEncoding; + pending.payload.colorInt = clearPayload.colorInt; + pending.payload.colorUint = clearPayload.colorUint; } if ((clearPayload.mask & GL_DEPTH_BUFFER_BIT) != 0) { pending.payload.depth = clearPayload.depth; @@ -924,9 +934,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (rbHasClear && MG_Util::GetBaseInternalFormatComponentCount(renderbuffer->GetInternalFormat()) == 3) { // RGB renderbuffers are backed by an RGBA image; the missing alpha reads as 1. - rbClearPayload.color = - FloatVec4(rbClearPayload.color.x(), rbClearPayload.color.y(), - rbClearPayload.color.z(), 1.0f); + ForceOpaqueClearAlpha(rbClearPayload); } const VkImageLayout trackedRbLayout = rbResource->layout; @@ -1496,12 +1504,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } if ((clearPayload.mask & GL_COLOR_BUFFER_BIT) != 0) { - clearValues[pending.attachmentIndex].color = { - clearPayload.color.x(), - clearPayload.color.y(), - clearPayload.color.z(), - ResolveColorClearAlpha(liveTexture.get(), clearPayload.color.w()) - }; + clearValues[pending.attachmentIndex].color = + MakeVkClearColorValue(clearPayload, ColorFormatLacksAlpha(liveTexture.get())); } if ((clearPayload.mask & GL_DEPTH_BUFFER_BIT) != 0) { clearValues[pending.attachmentIndex].depthStencil.depth = clearPayload.depth; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index af7207fd..b6c0baa2 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -190,11 +190,11 @@ namespace MobileGL::MG_Backend::DirectVulkan { } } - static Float ResolveColorClearAlpha(const MG_State::GLState::ITextureObject* texture, Float requestedAlpha) { - if (texture != nullptr && MG_Util::GetBaseInternalFormatComponentCount(texture->GetFormat()) == 3) { - return 1.0f; - } - return requestedAlpha; + // GL 4.6 core 15.2.3: a colour format with no alpha channel reads as if alpha were one. + // The substitution has to happen in the clear value's own type, so this reports the condition + // and MakeVkClearColorValue applies it to whichever union member the encoding selects. + static Bool ColorFormatLacksAlpha(const MG_State::GLState::ITextureObject* texture) { + return texture != nullptr && MG_Util::GetBaseInternalFormatComponentCount(texture->GetFormat()) == 3; } static Bool IsQuarterTurnPreTransform(VkSurfaceTransformFlagBitsKHR preTransform) { @@ -5401,9 +5401,11 @@ void main() { VkClearAttachment clearAttachment{}; clearAttachment.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; clearAttachment.colorAttachment = drawBufferIndex; + // glClear only ever supplies float values (ClearFramebufferPayload has no + // other form), so the float member is always the right one here. clearAttachment.clearValue.color = { payload.color.x(), payload.color.y(), payload.color.z(), - ResolveColorClearAlpha(colorTexture, payload.color.w()) + ColorFormatLacksAlpha(colorTexture) ? 1.0f : payload.color.w() }; clearAttachments[clearAttachmentCount++] = clearAttachment; } @@ -5680,10 +5682,8 @@ void main() { } clearAttachment.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; clearAttachment.colorAttachment = static_cast(drawbuffer); - clearAttachment.clearValue.color = { - clearPayload.color.x(), clearPayload.color.y(), clearPayload.color.z(), - ResolveColorClearAlpha(colorTexture, clearPayload.color.w()) - }; + clearAttachment.clearValue.color = + MakeVkClearColorValue(clearPayload, ColorFormatLacksAlpha(colorTexture)); } else { VkImageAspectFlags aspects = 0; if ((clearPayload.mask & GL_DEPTH_BUFFER_BIT) != 0 && MG_State::pGLContext->GetDepthMask() && @@ -5783,8 +5783,8 @@ void main() { switch (buffer) { case GL_COLOR: payload.mask = GL_COLOR_BUFFER_BIT; - payload.color = FloatVec4(static_cast(value[0]), static_cast(value[1]), - static_cast(value[2]), static_cast(value[3])); + payload.colorEncoding = ClearColorEncoding::Int; + payload.colorInt = IntVec4(value[0], value[1], value[2], value[3]); break; case GL_STENCIL: payload.mask = GL_STENCIL_BUFFER_BIT; @@ -5805,8 +5805,8 @@ void main() { ClearAttachmentPayload payload{}; if (buffer == GL_COLOR) { payload.mask = GL_COLOR_BUFFER_BIT; - payload.color = FloatVec4(static_cast(value[0]), static_cast(value[1]), - static_cast(value[2]), static_cast(value[3])); + payload.colorEncoding = ClearColorEncoding::Uint; + payload.colorUint = UintVec4(value[0], value[1], value[2], value[3]); } QueueClearBufferPayloadForFramebuffer(*framebuffer, buffer, drawbuffer, payload); } @@ -5833,8 +5833,8 @@ void main() { switch (buffer) { case GL_COLOR: payload.mask = GL_COLOR_BUFFER_BIT; - payload.color = FloatVec4(static_cast(value[0]), static_cast(value[1]), - static_cast(value[2]), static_cast(value[3])); + payload.colorEncoding = ClearColorEncoding::Uint; + payload.colorUint = UintVec4(value[0], value[1], value[2], value[3]); break; case GL_STENCIL: payload.mask = GL_STENCIL_BUFFER_BIT; @@ -5854,8 +5854,8 @@ void main() { switch (buffer) { case GL_COLOR: payload.mask = GL_COLOR_BUFFER_BIT; - payload.color = FloatVec4(static_cast(value[0]), static_cast(value[1]), - static_cast(value[2]), static_cast(value[3])); + payload.colorEncoding = ClearColorEncoding::Int; + payload.colorInt = IntVec4(value[0], value[1], value[2], value[3]); break; case GL_STENCIL: payload.mask = GL_STENCIL_BUFFER_BIT; @@ -5917,11 +5917,8 @@ void main() { const auto& clearPayload = pendingClear.payload; if ((resource->aspect & VK_IMAGE_ASPECT_COLOR_BIT) != 0) { subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - VkClearColorValue clearValue{}; - clearValue.float32[0] = clearPayload.color.x(); - clearValue.float32[1] = clearPayload.color.y(); - clearValue.float32[2] = clearPayload.color.z(); - clearValue.float32[3] = ResolveColorClearAlpha(&texture, clearPayload.color.w()); + const VkClearColorValue clearValue = + MakeVkClearColorValue(clearPayload, ColorFormatLacksAlpha(&texture)); vkCmdClearColorImage(commandBuffer, resource->image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &clearValue, 1, &subresourceRange); } else { @@ -6003,14 +6000,10 @@ void main() { VkImageLayout steadyLayout; if ((resource->aspect & VK_IMAGE_ASPECT_COLOR_BIT) != 0) { subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - VkClearColorValue clearValue{}; - clearValue.float32[0] = clearPayload.color.x(); - clearValue.float32[1] = clearPayload.color.y(); - clearValue.float32[2] = clearPayload.color.z(); // RGB renderbuffers are backed by an RGBA image; the missing alpha reads as 1. - clearValue.float32[3] = - MG_Util::GetBaseInternalFormatComponentCount(renderbuffer->GetInternalFormat()) == 3 ? - 1.0f : clearPayload.color.w(); + const VkClearColorValue clearValue = MakeVkClearColorValue( + clearPayload, + MG_Util::GetBaseInternalFormatComponentCount(renderbuffer->GetInternalFormat()) == 3); vkCmdClearColorImage(commandBuffer, resource->image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, &clearValue, 1, &subresourceRange); steadyLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; @@ -10662,12 +10655,8 @@ void main() { // VkClearAttachment::colorAttachment indexes the subpass pColorAttachments (draw-buffer // slot space, with UNUSED holes), not the compacted attachment descriptions. clearAttachment.colorAttachment = pending.colorAttachmentSlot; - clearAttachment.clearValue.color = { - clearPayload.color.x(), - clearPayload.color.y(), - clearPayload.color.z(), - ResolveColorClearAlpha(liveTexture.get(), clearPayload.color.w()) - }; + clearAttachment.clearValue.color = + MakeVkClearColorValue(clearPayload, ColorFormatLacksAlpha(liveTexture.get())); } else { if ((clearPayload.mask & GL_DEPTH_BUFFER_BIT) != 0) { clearAttachment.aspectMask |= VK_IMAGE_ASPECT_DEPTH_BIT;