[Fix] (DirectVulkan): resolve renderbuffer VkFormats through the shared texture table and police copy size-compatibility

This commit is contained in:
2026-08-27 08:35:01 -04:00
parent 0dbb4ceba8
commit 88ee75be0e
4 changed files with 67 additions and 49 deletions
@@ -355,47 +355,18 @@ namespace MobileGL::MG_Backend::DirectVulkan {
}
const auto internalFormat = renderbuffer->GetInternalFormat();
// Three-channel color formats widen to their RGBA twin exactly like textures do
// (VkTextureManager::ResolveTextureFormatInfo): blits/resolves between a
// renderbuffer and a texture of the same GL format then see one VkFormat.
const VkFormat format = [&]() -> VkFormat {
switch (internalFormat) {
case TextureInternalFormat::RGB:
case TextureInternalFormat::RGB8:
case TextureInternalFormat::R3G3B2:
case TextureInternalFormat::RGB4:
case TextureInternalFormat::RGB5:
return VK_FORMAT_R8G8B8A8_UNORM;
case TextureInternalFormat::SRGB8:
return VK_FORMAT_R8G8B8A8_SRGB;
case TextureInternalFormat::RGB8Snorm:
return VK_FORMAT_R8G8B8A8_SNORM;
case TextureInternalFormat::RGB10:
case TextureInternalFormat::RGB12:
case TextureInternalFormat::RGB16:
return VK_FORMAT_R16G16B16A16_UNORM;
case TextureInternalFormat::RGB16Snorm:
return VK_FORMAT_R16G16B16A16_SNORM;
case TextureInternalFormat::RGB16F:
return VK_FORMAT_R16G16B16A16_SFLOAT;
case TextureInternalFormat::RGB32F:
return VK_FORMAT_R32G32B32A32_SFLOAT;
case TextureInternalFormat::RGB8I:
return VK_FORMAT_R8G8B8A8_SINT;
case TextureInternalFormat::RGB8UI:
return VK_FORMAT_R8G8B8A8_UINT;
case TextureInternalFormat::RGB16I:
return VK_FORMAT_R16G16B16A16_SINT;
case TextureInternalFormat::RGB16UI:
return VK_FORMAT_R16G16B16A16_UINT;
case TextureInternalFormat::RGB32I:
return VK_FORMAT_R32G32B32A32_SINT;
case TextureInternalFormat::RGB32UI:
return VK_FORMAT_R32G32B32A32_UINT;
default:
return MG_Util::ConvertTextureInternalFormatToVkEnum(internalFormat);
}
}();
// ONE resolver, shared with textures (VkTextureManager::ResolveTextureFormatInfo), so a
// renderbuffer and a texture of the same GL format cannot disagree about their VkFormat.
// `expandRgbToRgba` / `componentByteCount` / `alphaBytes` describe how to reshape a SHADOW
// UPLOAD, and a renderbuffer has none, so only `.format` is taken.
//
// This used to be a hand-maintained second copy of that table, and it was missing exactly
// four rows: RGBA2 and RGBA12 fell through to ConvertTextureInternalFormatToVkEnum's
// VK_FORMAT_UNDEFINED (no image at all - bound as a draw buffer the attachment became
// VK_ATTACHMENT_UNUSED and every draw into it was dropped), while RGBA4 and RGB5A1 fell
// through to the 16-bit packed formats and then faced 32-bit R8G8B8A8_UNORM textures across
// a size-incompatible vkCmdCopyImage.
const VkFormat format = ResolveTextureFormatInfo(internalFormat).format;
const VkImageAspectFlags aspect = ResolveImageAspectMaskForFormat(format);
// Renderbuffers are never sampled (GL has no way to bind one to a sampler), so the
// usage set is attachment + transfer: transfer covers readback (vkCmdCopyImageToBuffer),
@@ -46,13 +46,6 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return mipLevelCount;
}
struct TextureFormatInfo {
VkFormat format = VK_FORMAT_UNDEFINED;
Bool expandRgbToRgba = false;
Uint32 componentByteCount = 0;
Array<Uint8, 4> alphaBytes = {0, 0, 0, 0};
};
struct TextureShapeInfo {
VkImageType imageType = VK_IMAGE_TYPE_2D;
VkImageViewType viewType = VK_IMAGE_VIEW_TYPE_2D;
@@ -380,7 +373,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
return true;
}
static TextureFormatInfo ResolveTextureFormatInfo(TextureInternalFormat format) {
TextureFormatInfo ResolveTextureFormatInfo(TextureInternalFormat format) {
switch (format) {
case TextureInternalFormat::RGB:
case TextureInternalFormat::RGB8:
@@ -24,6 +24,31 @@ class ITextureObject;
namespace MobileGL::MG_Backend::DirectVulkan {
enum class SamplerNumericDomain : Uint8;
// What VkFormat a GL internal format is BACKED with, and how a shadow upload has to be reshaped to
// fit it. This is not the same question as "is there an exact VkFormat for this GL format", which is
// what ConvertTextureInternalFormatToVkEnum answers: several GL formats have no Vulkan twin at all
// (RGBA2, RGBA12) and several three-channel ones are deliberately widened to their four-channel twin
// because Vulkan devices rarely support the 3-channel layouts.
//
// SHARED, and it must stay the only answer to that question. A renderbuffer and a texture of the
// same GL format have to resolve to the SAME VkFormat or every blit, resolve and glCopyImageSubData
// between them crosses a size-incompatible pair, which vkCmdCopyImage leaves undefined
// (VUID-vkCmdCopyImage-srcImage-01548). The renderbuffer path used to carry a hand-maintained second
// copy of this table that was missing four rows - RGBA2, RGBA4, RGB5A1 and RGBA12 - so those four
// renderbuffer formats either got no image at all or a 16-bit-packed one facing a 32-bit texture.
struct TextureFormatInfo {
VkFormat format = VK_FORMAT_UNDEFINED;
// The GL format has three channels and is carried in a four-channel image; a shadow upload has
// to be expanded, inserting `alphaBytes` after every `componentByteCount * 3` source bytes.
Bool expandRgbToRgba = false;
Uint32 componentByteCount = 0;
Array<Uint8, 4> alphaBytes = {0, 0, 0, 0};
};
// Callers that only need the backing VkFormat (a renderbuffer has no shadow upload to reshape) take
// `.format` and ignore the rest.
TextureFormatInfo ResolveTextureFormatInfo(TextureInternalFormat format);
// A GL 1D-ARRAY level keeps its LAYER COUNT in the state-side HEIGHT: that is what
// glTexImage2D(GL_TEXTURE_1D_ARRAY, width, layers) means, and the frontend records the level
// as {width, layers, 1} (see GL_Texture.cpp's AllocateStorage and the completeness walk in
@@ -9136,6 +9136,12 @@ void main() {
VkExtent2D extent = {0, 0};
Uint32 depth = 1;
Uint32 arrayLayers = 1;
// Both resources carry a format; this copy used to decline to read it, which is why a
// four-row drift between the texture and renderbuffer format tables turned into
// corrupted texels with nothing in the log. vkCmdCopyImage requires size-compatible
// formats whenever they differ (VUID-vkCmdCopyImage-srcImage-01548) and there is no
// downstream check - a mismatched pair is a promise the driver takes at face value.
VkFormat format = VK_FORMAT_UNDEFINED;
};
Bool TryResolveCopyImageSliceMapping(TextureTarget target, const CopyImageVkImage& image, Uint32 mipLevel,
@@ -9255,6 +9261,7 @@ void main() {
out.extent = resource->extent;
out.depth = 1;
out.arrayLayers = 1;
out.format = resource->format;
return out.image != VK_NULL_HANDLE;
}
// An endpoint that named nothing is the frontend validator's INVALID_VALUE and never
@@ -9270,6 +9277,7 @@ void main() {
out.extent = resource->extent;
out.depth = resource->depth;
out.arrayLayers = resource->arrayLayers;
out.format = resource->format;
return true;
};
CopyImageVkImage srcImage{};
@@ -9310,6 +9318,27 @@ void main() {
srcLevel, srcImage.mipLevels, dstLevel, dstImage.mipLevels);
return;
}
// Size compatibility, the guard whose absence let a table drift two files away reach the
// driver as a promise. glCopyImageSubData is a raw texel-block move (GL 4.6 core 18.3.2), and
// Vulkan says as much: when the two formats differ they must be size-compatible - the same
// texel block size - or vkCmdCopyImage is undefined (VUID-vkCmdCopyImage-srcImage-01548).
// Nothing else on this path asks: the three checks around it cover the mip range, the region
// bounds and the slice range, and none of them ever looked at a format.
//
// A decline rather than a MOBILEGL_ASSERT, for the reason the neighbouring guards spell out:
// assertions compile out of the release build that the CTS and shipping both run, which is
// exactly where the corruption was observed.
if (srcImage.format != dstImage.format) {
const Uint32 srcBlockSize = vkuGetFormatInfo(srcImage.format).texel_block_size;
const Uint32 dstBlockSize = vkuGetFormatInfo(dstImage.format).texel_block_size;
if (srcBlockSize == 0 || dstBlockSize == 0 || srcBlockSize != dstBlockSize) {
MGLOG_E_ONCE("%s: source format %d and destination format %d are not size-compatible "
"(%u vs %u bytes per texel block); declining the copy",
__func__, static_cast<Int>(srcImage.format), static_cast<Int>(dstImage.format),
srcBlockSize, dstBlockSize);
return;
}
}
const VkImageAspectFlags copyAspectMask =
srcImage.aspect & dstImage.aspect &
(VK_IMAGE_ASPECT_COLOR_BIT | VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);