From 6a80a82dd3df478abfe7354f54c3fa2c451d8b11 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 13 Aug 2026 02:41:09 -0400 Subject: [PATCH] [Fix, Test] (MG_Util, MG_Backend/DirectGLES, MG_Backend/DirectVulkan, MG_Test): a packed texel whose client type already spells its storage word must cross glGetTexImage unencoded, or the RGB9_E5 shared exponent gets canonicalized --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 16 ++ MobileGL/MG_Backend/DirectGLES/Utils.cpp | 162 ++++++++------- MobileGL/MG_Backend/DirectGLES/Utils.h | 10 + .../DirectVulkan/Renderer/VulkanRenderer.cpp | 31 +++ MobileGL/MG_Test/Texture/TextureTest.cpp | 187 ++++++++++++++++++ .../MG_Util/Texture/PixelStoreProcessor.cpp | 39 +++- .../MG_Util/Texture/PixelStoreProcessor.h | 15 ++ 7 files changed, 385 insertions(+), 75 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 8e720833..cee98ea6 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -7177,6 +7177,22 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } + // glGetTexImage returns the stored texels, and for a packed internal format read with the + // matching client type the shadow word already IS the client word. Decoding it to float and + // re-encoding would canonicalize an RGB9_E5 shared exponent (0xf8fc0000 -> 0xe7e00000: the + // same value, different bits), so those pairs copy the words straight through. + if (MG_Util::PixelStoreProcessor::IsRawPackedPixelTransfer( + textureMipmapObject->GetFormat(), MG_Util::ConvertGLEnumToTextureInputFormat(format), + MG_Util::ConvertGLEnumToTexturePixelDataType(type))) { + if (!ReadbackImpl::StorePackedWordsToClient(static_cast(shadow), width, sliceHeight, + sliceCount, type, pixels, applyPackImageParams)) { + return false; + } + MGLOG_D("GetTexImage: copied %s/%s verbatim from the CPU shadow copy", + MG_Util::ConvertGLEnumToString(format).c_str(), MG_Util::ConvertGLEnumToString(type).c_str()); + return true; + } + Vector wide; Bool isInteger = false; Bool isSigned = false; diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 47c97902..92820c34 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -1596,88 +1596,71 @@ namespace MobileGL::MG_Backend::DirectGLES { return (rowBytes + align - 1) / align * align; } - // Repacks wide RGBA(_INTEGER) rows into the client's (format, type) layout, honoring the - // client-side PACK parameters and the bound pixel-pack buffer. `wide` holds - // `sliceHeight * sliceCount` rows of `width` texels (slice-major, tightly stacked), - // 4 components x GetReadbackComponentSize(wideType) bytes each. + // Walks the client-side destination the PACK parameters describe and hands each row to + // `fillRow(slice, row, dstRow)`, which writes width * dstPixelBytes bytes of finished client + // texels. Shared by the converting and the raw-word stores so both address the destination - + // and feed the bound pixel-pack buffer - identically. // applyPackImageParams: GL_PACK_IMAGE_HEIGHT / GL_PACK_SKIP_IMAGES apply only to GetTexImage // of 3D/array images; ReadPixels and 2D GetTexImage ignore them (GL 3.3 sections 4.3.1, 6.1.4). // Per the GL addressing rules, slice k row j lands at // SKIP_IMAGES*imageStride + SKIP_ROWS*rowStride + SKIP_PIXELS*pixelBytes // + k*imageStride + j*rowStride, with imageStride = max(IMAGE_HEIGHT, sliceHeight)*rowStride. - Bool StoreWideRowsToClient(const Uint8* wide, GLenum wideType, GLsizei width, GLsizei sliceHeight, - GLsizei sliceCount, const ReadbackChannelMapping& mapping, GLenum type, - void* pixels, Bool applyPackImageParams) { - const SizeT dstPixelBytes = GetReadbackDstPixelSize(mapping, type); - if (dstPixelBytes == 0) { - return false; - } - PackedReadbackLayout packedLayout{}; - const Bool isPackedType = GetPackedReadbackLayout(type, packedLayout); - const SizeT dstComponentSize = GetReadbackComponentSize(type); + template + static Bool StoreClientRows(SizeT dstPixelBytes, SizeT swapGroupSize, GLsizei width, GLsizei sliceHeight, + GLsizei sliceCount, void* pixels, Bool applyPackImageParams, FillRow&& fillRow) { + const auto& pixelPackBufferObject = + MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject(); - const auto& pixelPackBufferObject = - MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::PixelPack).GetBoundObject(); + // Destination layout is computed from the client-side PACK parameters; only the actual pixel + // rows are written so skip regions of the destination stay untouched. + const auto packParams = MG_State::pGLContext->GetPixelStoreParameters(false); + const SizeT rowPixels = static_cast(packParams.RowLength > 0 ? packParams.RowLength : width); + const SizeT dstRowStride = AlignReadbackRow(rowPixels * dstPixelBytes, packParams.Alignment); + const SizeT imageRows = + applyPackImageParams && packParams.ImageHeight > 0 + ? static_cast(packParams.ImageHeight) + : static_cast(sliceHeight); + const SizeT dstImageStride = imageRows * dstRowStride; + const SizeT skipImages = + applyPackImageParams ? static_cast(std::max(packParams.SkipImages, 0)) : SizeT{0}; + const SizeT dstSkipOffset = skipImages * dstImageStride + + static_cast(std::max(packParams.SkipRows, 0)) * dstRowStride + + static_cast(std::max(packParams.SkipPixels, 0)) * dstPixelBytes; + const SizeT dstRowBytes = static_cast(width) * dstPixelBytes; - // Destination layout is computed from the client-side PACK parameters; only the actual pixel - // rows are written so skip regions of the destination stay untouched. - const auto packParams = MG_State::pGLContext->GetPixelStoreParameters(false); - const SizeT rowPixels = static_cast(packParams.RowLength > 0 ? packParams.RowLength : width); - const SizeT dstRowStride = AlignReadbackRow(rowPixels * dstPixelBytes, packParams.Alignment); - const SizeT imageRows = - applyPackImageParams && packParams.ImageHeight > 0 - ? static_cast(packParams.ImageHeight) - : static_cast(sliceHeight); - const SizeT dstImageStride = imageRows * dstRowStride; - const SizeT skipImages = - applyPackImageParams ? static_cast(std::max(packParams.SkipImages, 0)) : SizeT{0}; - const SizeT dstSkipOffset = skipImages * dstImageStride + - static_cast(std::max(packParams.SkipRows, 0)) * dstRowStride + - static_cast(std::max(packParams.SkipPixels, 0)) * dstPixelBytes; - const SizeT dstRowBytes = static_cast(width) * dstPixelBytes; - - const SizeT pboBaseOffset = reinterpret_cast(pixels); // with a PBO, `pixels` is an offset - if (pixelPackBufferObject) { - const SizeT requiredSize = pboBaseOffset + dstSkipOffset + - static_cast(sliceCount - 1) * dstImageStride + - static_cast(sliceHeight - 1) * dstRowStride + dstRowBytes; - if (requiredSize > pixelPackBufferObject->GetSize()) { - MGLOG_E_ONCE("Readback conversion: pixel pack buffer is too small"); - return true; + const SizeT pboBaseOffset = reinterpret_cast(pixels); // with a PBO, `pixels` is an offset + if (pixelPackBufferObject) { + const SizeT requiredSize = pboBaseOffset + dstSkipOffset + + static_cast(sliceCount - 1) * dstImageStride + + static_cast(sliceHeight - 1) * dstRowStride + dstRowBytes; + if (requiredSize > pixelPackBufferObject->GetSize()) { + MGLOG_E_ONCE("Readback conversion: pixel pack buffer is too small"); + return true; + } } - } - const SizeT srcComponentSize = GetReadbackComponentSize(wideType); - const SizeT srcPixelBytes = 4 * srcComponentSize; - Vector convertedRow(dstRowBytes); + Vector convertedRow(dstRowBytes); - for (GLsizei slice = 0; slice < sliceCount; ++slice) { - for (GLsizei row = 0; row < sliceHeight; ++row) { - const SizeT flatRow = static_cast(slice) * static_cast(sliceHeight) + - static_cast(row); - const Uint8* srcRow = wide + flatRow * static_cast(width) * srcPixelBytes; - ConvertWideReadbackRow(srcRow, convertedRow.data(), static_cast(width), wideType, - mapping, type); + for (GLsizei slice = 0; slice < sliceCount; ++slice) { + for (GLsizei row = 0; row < sliceHeight; ++row) { + fillRow(slice, row, convertedRow.data()); - if (packParams.SwapBytes) { - const SizeT groupSize = isPackedType ? packedLayout.byteSize : dstComponentSize; - if (groupSize > 1) { - for (SizeT offset = 0; offset + groupSize <= dstRowBytes; offset += groupSize) { - std::reverse(convertedRow.data() + offset, convertedRow.data() + offset + groupSize); + if (packParams.SwapBytes && swapGroupSize > 1) { + for (SizeT offset = 0; offset + swapGroupSize <= dstRowBytes; offset += swapGroupSize) { + std::reverse(convertedRow.data() + offset, convertedRow.data() + offset + swapGroupSize); } } - } - const SizeT dstOffset = dstSkipOffset + static_cast(slice) * dstImageStride + - static_cast(row) * dstRowStride; - if (pixelPackBufferObject) { - pixelPackBufferObject->WritebackFromBackend({convertedRow.data(), dstRowBytes}, - pboBaseOffset + dstOffset); - } else { - Memcpy(static_cast(pixels) + dstOffset, convertedRow.data(), dstRowBytes); + const SizeT dstOffset = dstSkipOffset + static_cast(slice) * dstImageStride + + static_cast(row) * dstRowStride; + if (pixelPackBufferObject) { + pixelPackBufferObject->WritebackFromBackend({convertedRow.data(), dstRowBytes}, + pboBaseOffset + dstOffset); + } else { + Memcpy(static_cast(pixels) + dstOffset, convertedRow.data(), dstRowBytes); + } } } - } if (pixelPackBufferObject) { // WritebackFromBackend bumps change serials with no backend op; re-open // the buffer draw-clean memos (once for the whole row loop). @@ -1685,5 +1668,52 @@ namespace MobileGL::MG_Backend::DirectGLES { } return true; } + + // Repacks wide RGBA(_INTEGER) rows into the client's (format, type) layout, honoring the + // client-side PACK parameters and the bound pixel-pack buffer. `wide` holds + // `sliceHeight * sliceCount` rows of `width` texels (slice-major, tightly stacked), + // 4 components x GetReadbackComponentSize(wideType) bytes each. + Bool StoreWideRowsToClient(const Uint8* wide, GLenum wideType, GLsizei width, GLsizei sliceHeight, + GLsizei sliceCount, const ReadbackChannelMapping& mapping, GLenum type, + void* pixels, Bool applyPackImageParams) { + const SizeT dstPixelBytes = GetReadbackDstPixelSize(mapping, type); + if (dstPixelBytes == 0) { + return false; + } + PackedReadbackLayout packedLayout{}; + const Bool isPackedType = GetPackedReadbackLayout(type, packedLayout); + const SizeT swapGroupSize = isPackedType ? packedLayout.byteSize : GetReadbackComponentSize(type); + const SizeT srcPixelBytes = 4 * GetReadbackComponentSize(wideType); + + return StoreClientRows(dstPixelBytes, swapGroupSize, width, sliceHeight, sliceCount, pixels, + applyPackImageParams, + [&](GLsizei slice, GLsizei row, Uint8* dstRow) { + const SizeT flatRow = static_cast(slice) * + static_cast(sliceHeight) + + static_cast(row); + const Uint8* srcRow = + wide + flatRow * static_cast(width) * srcPixelBytes; + ConvertWideReadbackRow(srcRow, dstRow, static_cast(width), wideType, + mapping, type); + }); + } + + Bool StorePackedWordsToClient(const Uint8* srcWords, GLsizei width, GLsizei sliceHeight, GLsizei sliceCount, + GLenum type, void* pixels, Bool applyPackImageParams) { + PackedReadbackLayout packedLayout{}; + if (!GetPackedReadbackLayout(type, packedLayout) || packedLayout.byteSize != 4) { + return false; + } + const SizeT srcRowBytes = static_cast(width) * 4; + + return StoreClientRows(4, packedLayout.byteSize, width, sliceHeight, sliceCount, pixels, + applyPackImageParams, + [&](GLsizei slice, GLsizei row, Uint8* dstRow) { + const SizeT flatRow = static_cast(slice) * + static_cast(sliceHeight) + + static_cast(row); + Memcpy(dstRow, srcWords + flatRow * srcRowBytes, srcRowBytes); + }); + } } // namespace ReadbackImpl } // namespace MobileGL::MG_Backend::DirectGLES diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index d44b3c67..a679a4aa 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -115,6 +115,16 @@ namespace MobileGL::MG_Backend::DirectGLES { Bool StoreWideRowsToClient(const Uint8* wide, GLenum wideType, GLsizei width, GLsizei sliceHeight, GLsizei sliceCount, const ReadbackChannelMapping& mapping, GLenum type, void* pixels, Bool applyPackImageParams); + + // Stores packed 32-bit source words verbatim, with the same destination addressing, PACK + // parameters and pixel-pack-buffer handling as StoreWideRowsToClient. For the sources whose + // storage word already IS the client word (MG_Util::IsRawPackedPixelTransfer): routing those + // through the wide float intermediate re-encodes them, and the RGB9_E5 encoder canonicalizes + // the shared exponent, so glGetTexImage would answer with different bits than were stored. + // `srcWords` holds sliceHeight * sliceCount tightly stacked rows of `width` 32-bit words. + // False when `type` is not a 4-byte packed type. + Bool StorePackedWordsToClient(const Uint8* srcWords, GLsizei width, GLsizei sliceHeight, GLsizei sliceCount, + GLenum type, void* pixels, Bool applyPackImageParams); } // namespace ReadbackImpl namespace PrgramImpl { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 2d03a4e1..ee4d3c74 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -26,6 +26,7 @@ #include "MG_Util/Converters/MGToVk/TextureEnumConverter.h" #include "MG_Util/Math/HalfFloat.h" #include "MG_Util/Metrics/TextureMetrics.h" +#include "MG_Util/Texture/PixelStoreProcessor.h" #include #include #include @@ -2621,6 +2622,24 @@ void main() { } } + // The GL internal format a packed VkFormat stores, for the raw-word readback test below. + // Only the packed 32-bit layouts MobileGL keeps natively need an entry; anything else takes + // the wide decode path. + static TextureInternalFormat GetPackedReadbackInternalFormat(VkFormat format) { + switch (format) { + case VK_FORMAT_E5B9G9R9_UFLOAT_PACK32: + return TextureInternalFormat::RGB9E5; + case VK_FORMAT_B10G11R11_UFLOAT_PACK32: + return TextureInternalFormat::R11FG11FB10F; + case VK_FORMAT_A2B10G10R10_UNORM_PACK32: + return TextureInternalFormat::RGB10A2; + case VK_FORMAT_A2B10G10R10_UINT_PACK32: + return TextureInternalFormat::RGB10A2UI; + default: + return TextureInternalFormat::Unknown; + } + } + static Bool PackReadbackToClientOrPbo(const Uint8* srcPixels, VkFormat srcFormat, GLsizei width, GLsizei sliceHeight, GLsizei sliceCount, GLenum format, GLenum type, void* pixels, Bool applyPackImageParams, @@ -2636,6 +2655,18 @@ void main() { return false; } + // A packed image read with the matching client type hands back its own words: the + // decode-to-float / re-encode round trip is lossy in the bits (it canonicalizes an + // RGB9_E5 shared exponent), which glGetTexImage must not do. Left to the wide path when + // GL_CLAMP_READ_COLOR may still have to act, i.e. for glReadPixels. + if (!applyReadColorClamp && + MG_Util::PixelStoreProcessor::IsRawPackedPixelTransfer( + GetPackedReadbackInternalFormat(srcFormat), MG_Util::ConvertGLEnumToTextureInputFormat(format), + MG_Util::ConvertGLEnumToTexturePixelDataType(type))) { + return DirectGLES::ReadbackImpl::StorePackedWordsToClient(srcPixels, width, sliceHeight, sliceCount, + type, pixels, applyPackImageParams); + } + Vector wide; GLenum wideType = GL_FLOAT; if (!DecodeReadbackRowsToWide(srcPixels, srcFormat, width, diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index 4c8a86fe..993cec8e 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -3179,6 +3179,193 @@ TEST_F(TextureTest, DecodeShadowDataToWideRGBACoversComponentAndPackedLayouts) { } } +// ---- GL_RGB9_E5 raw-preserving transfer -------------------------------------------------------- +// RGB9_E5 packs three 9-bit mantissas against one shared 5-bit exponent, so a value has several +// legal encodings (shift the exponent up, shift every mantissa down). The spec's encode algorithm +// (GL 4.6 8.5.2) always emits the canonical one, which makes decode-to-float / re-encode +// value-preserving but NOT bit-preserving. glTexImage followed by glGetTexImage has to hand the +// application its own bits back, so a client (format, type) whose word already IS the storage word +// must move verbatim. GL CTS KHR-GL43.copy_image caught the round trip turning the uploaded +// 0xf8fc0000 into 0xe7e00000 ("CopyImageSubData modified contents of source image") and a copied-in +// 0x60000000 into 0x00000000 ("CopyImageSubData stored invalid data in copied region"). + +namespace { + Uint32 RoundTripSharedExponentWord(Uint32 word) { + Float rgb[3]; + MG_Util::DecodeSharedExponentRGB9E5(word, rgb); + return MG_Util::EncodeSharedExponentRGB9E5(rgb); + } +} // namespace + +TEST(SharedExponentRGB9E5Test, EncodeReproducesCanonicalWordsExactly) { + // Canonical encodings - the ones the spec algorithm emits - must survive a decode/encode round + // trip untouched, or every conversion INTO RGB9_E5 would be off as well. + const Uint32 canonical[] = { + 0x00000000u, // all zero + 0x0FFFFFFFu, // exponent 1, every mantissa saturated (smallest normalized exponent in use) + 0x000003FFu, // exponent 0: the denormal range, mantissas 511 / 1 / 0 + 0x81010100u, // (1.0, 0.5, 0.25) + 0xE7E00000u, // (0, 0, 8064) - what the CTS round trip produced + 0xFFFFFFFFu, // exponent 31 with saturated mantissas = the largest representable texel + }; + for (const Uint32 word : canonical) { + EXPECT_EQ(RoundTripSharedExponentWord(word), word) << "word 0x" << std::hex << word; + // Encoding is idempotent: a second pass may not drift either. + EXPECT_EQ(RoundTripSharedExponentWord(RoundTripSharedExponentWord(word)), word); + } +} + +TEST(SharedExponentRGB9E5Test, EncodeCanonicalizesRedundantWords) { + // The exact QPA signatures. Both pairs hold the same value, so the encoder is not wrong - which + // is why the fix has to be a raw path rather than an encoder change. + Float observed[3]; + MG_Util::DecodeSharedExponentRGB9E5(0xF8FC0000u, observed); + Float canonical[3]; + MG_Util::DecodeSharedExponentRGB9E5(0xE7E00000u, canonical); + EXPECT_EQ(observed[2], 8064.0f); + EXPECT_EQ(canonical[2], 8064.0f); + EXPECT_EQ(RoundTripSharedExponentWord(0xF8FC0000u), 0xE7E00000u); + + // Exponent 12 with all-zero mantissas is still the value zero, and canonicalizes to the + // all-zero word. + EXPECT_EQ(RoundTripSharedExponentWord(0x60000000u), 0x00000000u); + // Mantissa 1 at exponent 1 renormalizes down into the denormal range. + EXPECT_EQ(RoundTripSharedExponentWord(0x08000001u), 0x00000002u); +} + +TEST(SharedExponentRGB9E5Test, RawPackedPixelTransferCoversOnlyIdenticalLayouts) { + using MG_Util::PixelStoreProcessor::IsRawPackedPixelTransfer; + + // The four pairs whose client word is bit-identical to the packed storage word. + EXPECT_TRUE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB9E5, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt5999Rev)); + EXPECT_TRUE(IsRawPackedPixelTransfer(TextureInternalFormat::R11FG11FB10F, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt101111Rev)); + EXPECT_TRUE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB10A2, TextureInputFormat::RGBA, + TexturePixelDataType::UnsignedInt2101010Rev)); + EXPECT_TRUE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB10A2UI, TextureInputFormat::RGBAInteger, + TexturePixelDataType::UnsignedInt2101010Rev)); + + // A different packed float layout of the same width is still a conversion. + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB9E5, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt101111Rev)); + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::R11FG11FB10F, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt5999Rev)); + // So is a component client type, or the same word against a component internal format. + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB9E5, TextureInputFormat::RGB, + TexturePixelDataType::Float)); + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB8, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt5999Rev)); + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGBA32F, TextureInputFormat::RGBA, + TexturePixelDataType::UnsignedInt2101010Rev)); + // Integerness has to line up too: the normalized and integer 10/10/10/2 words are not the + // same client layout even though they are the same bit field. + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB10A2, TextureInputFormat::RGBAInteger, + TexturePixelDataType::UnsignedInt2101010Rev)); + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::RGB10A2UI, TextureInputFormat::RGBA, + TexturePixelDataType::UnsignedInt2101010Rev)); + EXPECT_FALSE(IsRawPackedPixelTransfer(TextureInternalFormat::Unknown, TextureInputFormat::RGB, + TexturePixelDataType::UnsignedInt5999Rev)); +} + +TEST_F(TextureTest, TexImage2DRGB9E5KeepsNonCanonicalClientWords) { + // Upload direction: GL_RGB / GL_UNSIGNED_INT_5_9_9_9_REV into GL_RGB9_E5 stores the client + // words untouched, including the redundant encodings the CTS generates. + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + + const Uint32 words[] = {0xF8FC0000u, 0x60000000u, 0x08000001u, 0x0FFFFFFFu}; + MG_Impl::GLImpl::PixelStorei(GL_UNPACK_ALIGNMENT, 1); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGB9_E5, 4, 1, 0, GL_RGB, GL_UNSIGNED_INT_5_9_9_9_REV, words); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + const auto* stored = GetBoundTexture2DLevelBytes(texture); + ASSERT_NE(stored, nullptr); + Uint32 readBack[4] = {}; + std::memcpy(readBack, stored, sizeof(readBack)); + for (Int i = 0; i < 4; ++i) { + EXPECT_EQ(readBack[i], words[i]) << "texel " << i; + } + + MG_Impl::GLImpl::PixelStorei(GL_UNPACK_ALIGNMENT, 4); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0); +} + +TEST_F(TextureTest, TexImage2DRGB9E5FromOtherPackedFloatTypeStillConverts) { + // Negative control for the raw path: a genuinely different client layout keeps the + // decode-to-float / re-encode conversion. + GLuint texture = 0; + MG_Impl::GLImpl::GenTextures(1, &texture); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture); + + // 10F_11F_11F_REV word holding (1.0, 0.5, 0.25) - see the packed readback encode tests. + const Uint32 packedFloatWord = 0x681C03C0u; + MG_Impl::GLImpl::PixelStorei(GL_UNPACK_ALIGNMENT, 1); + MG_Impl::GLImpl::TexImage2D(GL_TEXTURE_2D, 0, GL_RGB9_E5, 1, 1, 0, GL_RGB, GL_UNSIGNED_INT_10F_11F_11F_REV, + &packedFloatWord); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); + + const auto* stored = GetBoundTexture2DLevelBytes(texture); + ASSERT_NE(stored, nullptr); + Uint32 word = 0; + std::memcpy(&word, stored, sizeof(word)); + const Float rgb[3] = {1.0f, 0.5f, 0.25f}; + EXPECT_EQ(word, MG_Util::EncodeSharedExponentRGB9E5(rgb)); + EXPECT_NE(word, packedFloatWord) << "the raw path must not swallow a real conversion"; + + MG_Impl::GLImpl::PixelStorei(GL_UNPACK_ALIGNMENT, 4); + MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, 0); +} + +TEST_F(TextureTest, StorePackedWordsToClientCopiesWordsVerbatimUnderPackParams) { + // Readback direction: the raw store copies the words bit-for-bit while still honoring the + // client-side PACK addressing (alignment, skip rows/pixels) and GL_PACK_SWAP_BYTES. + namespace ReadbackImpl = MG_Backend::DirectGLES::ReadbackImpl; + + const Uint32 source[] = {0xF8FC0000u, 0x60000000u, 0x08000001u, // row 0 + 0x0FFFFFFFu, 0xFFFFFFFFu, 0x00000000u}; // row 1 + constexpr Uint32 kFill = 0xDEADBEEFu; + Uint32 destination[16]; + std::fill(std::begin(destination), std::end(destination), kFill); + + MG_Impl::GLImpl::PixelStorei(GL_PACK_ALIGNMENT, 8); // rows of 3 words (12 B) pad to 16 B + MG_Impl::GLImpl::PixelStorei(GL_PACK_SKIP_ROWS, 1); + MG_Impl::GLImpl::PixelStorei(GL_PACK_SKIP_PIXELS, 1); + ASSERT_TRUE(ReadbackImpl::StorePackedWordsToClient(reinterpret_cast(source), /*width=*/3, + /*sliceHeight=*/2, /*sliceCount=*/1, + GL_UNSIGNED_INT_5_9_9_9_REV, destination, + /*applyPackImageParams=*/false)); + // Row 0 lands at SKIP_ROWS * 16 + SKIP_PIXELS * 4 = 20 bytes = word 5; row 1 one 16-byte + // stride further along, at word 9. + for (Int i = 0; i < 3; ++i) { + EXPECT_EQ(destination[5 + i], source[i]) << "row 0 texel " << i; + EXPECT_EQ(destination[9 + i], source[3 + i]) << "row 1 texel " << i; + } + // The skipped region and the row padding stay untouched. + EXPECT_EQ(destination[0], kFill); + EXPECT_EQ(destination[4], kFill); + EXPECT_EQ(destination[8], kFill); + EXPECT_EQ(destination[12], kFill); + + // GL_PACK_SWAP_BYTES reverses each 4-byte word. + std::fill(std::begin(destination), std::end(destination), kFill); + MG_Impl::GLImpl::PixelStorei(GL_PACK_SKIP_ROWS, 0); + MG_Impl::GLImpl::PixelStorei(GL_PACK_SKIP_PIXELS, 0); + MG_Impl::GLImpl::PixelStorei(GL_PACK_ALIGNMENT, 1); + MG_Impl::GLImpl::PixelStorei(GL_PACK_SWAP_BYTES, GL_TRUE); + ASSERT_TRUE(ReadbackImpl::StorePackedWordsToClient(reinterpret_cast(source), /*width=*/3, + /*sliceHeight=*/1, /*sliceCount=*/1, + GL_UNSIGNED_INT_5_9_9_9_REV, destination, + /*applyPackImageParams=*/false)); + EXPECT_EQ(destination[0], 0x0000FCF8u); // byte-reversed 0xF8FC0000 + EXPECT_EQ(destination[1], 0x00000060u); + + MG_Impl::GLImpl::PixelStorei(GL_PACK_SWAP_BYTES, GL_FALSE); + MG_Impl::GLImpl::PixelStorei(GL_PACK_ALIGNMENT, 4); + EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR); +} + // GL 4.6 core table 23.18: GL_TEXTURE_COMPARE_FUNC takes the whole eight-function depth-compare // range. The validator used to start it at GL_LEQUAL, which sits in the middle of the contiguous // GL_NEVER..GL_ALWAYS block, so NEVER/LESS/EQUAL were rejected while GREATER/NOTEQUAL/GEQUAL only diff --git a/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp b/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp index 60e9906f..066a0822 100644 --- a/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp +++ b/MobileGL/MG_Util/Texture/PixelStoreProcessor.cpp @@ -259,6 +259,25 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { } } + // The one client (format, type) pair whose word is bit-identical to the packed internal + // word, if any. Everything else has to go through the decode/encode conversion. + Bool IsRawPackedPixelPair(PackedInternalKind kind, TextureInputFormat format, + TexturePixelDataType type) { + switch (kind) { + case PackedInternalKind::UNorm2101010Rev: + return format == TextureInputFormat::RGBA && type == TexturePixelDataType::UnsignedInt2101010Rev; + case PackedInternalKind::UInt2101010Rev: + return format == TextureInputFormat::RGBAInteger && + type == TexturePixelDataType::UnsignedInt2101010Rev; + case PackedInternalKind::FloatR11G11B10: + return format == TextureInputFormat::RGB && type == TexturePixelDataType::UnsignedInt101111Rev; + case PackedInternalKind::FloatRGB9E5: + return format == TextureInputFormat::RGB && type == TexturePixelDataType::UnsignedInt5999Rev; + default: + return false; + } + } + Uint32 EncodePackedInternalWordFloat(PackedInternalKind kind, const Float rgba[4]) { switch (kind) { case PackedInternalKind::UNorm2101010Rev: { @@ -431,10 +450,7 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { return false; } // The client word already equals the packed internal word (memcpy fast path). - if (hasPackedInternal && type == TexturePixelDataType::UnsignedInt2101010Rev && - (format == TextureInputFormat::RGBA || format == TextureInputFormat::RGBAInteger) && - (packedInternal.kind == PackedInternalKind::UNorm2101010Rev || - packedInternal.kind == PackedInternalKind::UInt2101010Rev)) { + if (hasPackedInternal && IsRawPackedPixelPair(packedInternal.kind, format, type)) { return false; } } else { @@ -458,11 +474,7 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { // GL_RGB, which the state layer already enforces. if (mapping.isInteger || mapping.channelCount != 3) return false; // The client word already equals the packed internal word. - if (hasPackedInternal && - ((packedInternal.kind == PackedInternalKind::FloatRGB9E5 && - type == TexturePixelDataType::UnsignedInt5999Rev) || - (packedInternal.kind == PackedInternalKind::FloatR11G11B10 && - type == TexturePixelDataType::UnsignedInt101111Rev))) { + if (hasPackedInternal && IsRawPackedPixelPair(packedInternal.kind, format, type)) { return false; } break; @@ -765,6 +777,15 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { } } // namespace + Bool IsRawPackedPixelTransfer(TextureInternalFormat internalFormat, TextureInputFormat clientFormat, + TexturePixelDataType clientType) { + InternalPackedLayout packedInternal{}; + if (!GetInternalPackedLayout(internalFormat, packedInternal)) { + return false; + } + return IsRawPackedPixelPair(packedInternal.kind, clientFormat, clientType); + } + // assume 8 bit per channel // swizzle.size() == channel count void ProcessColorSwizzle(void* data, SizeT pixelCount, const Vector& swizzle) { diff --git a/MobileGL/MG_Util/Texture/PixelStoreProcessor.h b/MobileGL/MG_Util/Texture/PixelStoreProcessor.h index 04dcf437..7d3d683d 100644 --- a/MobileGL/MG_Util/Texture/PixelStoreProcessor.h +++ b/MobileGL/MG_Util/Texture/PixelStoreProcessor.h @@ -23,6 +23,21 @@ namespace MobileGL::MG_Util::PixelStoreProcessor { IntVec3 dimension, Bool isBitmap, SizeT& outSize); void ProcessColorSwizzle(void* data, SizeT pixelCount, const Vector& swizzle); + // True when a packed internal format's 32-bit storage word IS the client (format, type) word, + // so the transfer has to move the words verbatim in both directions. + // + // Decoding such a texel to float and re-encoding it is NOT a no-op: RGB9_E5 stores a shared + // exponent with redundant encodings, and the spec's encode algorithm (GL 4.6 8.5.2) always + // emits the canonical one - 0xf8fc0000 and 0xe7e00000 are the same value 8064, but only the + // latter is canonical. glTexImage followed by glGetTexImage must hand back the bits the + // application uploaded, which is what GL CTS KHR-GL43.copy_image compares + // ("CopyImageSubData modified contents of source image"). + // + // Only the pairs whose bit layouts are identical qualify; a genuinely different client format + // or type still needs the decode/encode conversion. + Bool IsRawPackedPixelTransfer(TextureInternalFormat internalFormat, TextureInputFormat clientFormat, + TexturePixelDataType clientType); + // Decodes the canonical shadow-mip storage of `internalFormat` into wide RGBA texels for CPU // readback (GetTexImage of non-renderable formats). Non-integer formats fill outWide with // 4 Floats per texel; integer formats fill it with 4 Uint32/Int32 per texel and set