diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 74cf1cd5..b34e3cf3 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -2901,19 +2901,63 @@ namespace MobileGL::MG_Backend::DirectGLES { return widenedData.data(); } + // The rgb10_a2 / rgb10_a2ui shadow split into the four GL_UNSIGNED_SHORT channel CODES its + // GL_RGBA16UI carrier is uploaded as. GL_UNSIGNED_INT_2_10_10_10_REV puts the FIRST + // component in the LOW bits (that is what REV means), so red is bits 0-9, green 10-19, + // blue 20-29 and alpha 30-31. + // + // The same split serves both formats: an rgb10_a2ui channel's code IS its value, and an + // rgb10_a2 channel's code is the numerator of value = code / (2^b - 1) that the shader-side + // unpack divides out. Neither is scaled here - the carrier holds the format's own bits. + // + // Sized from the LEVEL, not the source, for the reason PrepareChannelWidenedUpload is: the + // driver reads a full width*height*depth*4 shorts for the transfer it was handed. + const void* PreparePackedIntWidenedUpload(const IntVec3& texelSize, const void* data, + SizeT byteSize, Vector& widenedData) { + constexpr SizeT kSourceTexelBytes = sizeof(Uint32); + if (data == nullptr || byteSize < kSourceTexelBytes) { + return data; + } + const SizeT texelCount = static_cast(std::max(texelSize.x(), 0)) * + static_cast(std::max(texelSize.y(), 0)) * + static_cast(std::max(texelSize.z(), 1)); + if (texelCount == 0) { + return data; + } + const SizeT copyTexelCount = std::min(texelCount, byteSize / kSourceTexelBytes); + + widenedData.assign(texelCount * 4u * sizeof(Uint16), 0); + const auto* src = static_cast(data); + auto* dst = reinterpret_cast(widenedData.data()); + for (SizeT i = 0; i < texelCount; ++i, dst += 4) { + Uint32 packed = 0; + if (i < copyTexelCount) { + // Through a memcpy rather than a Uint32 read of `src`: the shadow is a byte + // buffer with no alignment promise of its own. + Memcpy(&packed, src + i * kSourceTexelBytes, sizeof(packed)); + } + dst[0] = static_cast(packed & 0x3FFu); + dst[1] = static_cast((packed >> 10u) & 0x3FFu); + dst[2] = static_cast((packed >> 20u) & 0x3FFu); + dst[3] = static_cast((packed >> 30u) & 0x3u); + } + return widenedData.data(); + } + // The transfer half of the image-format widening: an image-bindable texture whose ES // storage was widened to a core carrier is described to the driver as a four-component // transfer, so its narrower client data has to be repacked the same way the three-channel // colour-renderable widening repacks its own. // - // Two shapes, because the carriers come in two kinds. Seventeen of the eighteen keep the - // frontend format's component TYPE and only add channels, so padding the shadow out to - // four components is the whole conversion. r11f_g11f_b10f does not: its shadow is one - // PACKED 32-bit word per texel and its carrier is GL_RGBA16F, so the word has to be - // DECODED into four floats. Reading it as three components of the carrier's type - what - // the repack below would do - would take twelve bytes from a four-byte texel and shear - // the level, which is what the allFormats LOAD walkers see and the STORE ones do not (a - // store overwrites every texel the upload got wrong). + // Three shapes, because the carriers come in three kinds. Most of them keep the frontend + // format's component TYPE and only add channels, so padding the shadow out to four + // components is the whole conversion. The two PACKED formats do not: their shadow is one + // 32-bit word per texel, so the word has to be split - into four floats for + // r11f_g11f_b10f's GL_RGBA16F, into four shorts for rgb10_a2ui's GL_RGBA16UI. Reading such + // a word as components of the carrier's type - what the repack below would do - takes + // twelve or sixteen bytes from a four-byte texel and shears the level, which is what the + // allFormats LOAD walkers see and the STORE ones do not (a store overwrites every texel + // the upload got wrong). // // Composes with PrepareFallbackUpload rather than replacing it, and the composition is a // no-op by construction: none of the widened formats is one GetWidenableClientComponentCount @@ -2926,8 +2970,13 @@ namespace MobileGL::MG_Backend::DirectGLES { if (!widening || widening.SourceChannels == 0 || widening.SourceChannels > 4) { return data; } - if (widening.PackedFloatSource) { + switch (widening.SourceEncoding) { + case TextureImpl::ImageWidenSourceEncoding::PackedFloat11f11f10f: return PreparePackedFloatWidenedUpload(texelSize, data, byteSize, widenedData); + case TextureImpl::ImageWidenSourceEncoding::PackedInt2101010Rev: + return PreparePackedIntWidenedUpload(texelSize, data, byteSize, widenedData); + case TextureImpl::ImageWidenSourceEncoding::Components: + break; } if (widening.SourceChannels == 4) { return data; @@ -5395,6 +5444,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // here whose carrier has a different per-channel layout. See // WidenImageFormatsPass.h. case glslang::ElfR11fG11fB10f: return 0x8C3A; // GL_R11F_G11F_B10F + // 10/10/10/2 unsigned INTEGER channels in an rgba16ui: same component type, same + // channel count, every value representable. Only the transfer is re-encoded. + case glslang::ElfRgb10a2ui: return 0x906F; // GL_RGB10_A2UI default: return 0; } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index c33a138a..4cfcf886 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -749,6 +749,14 @@ namespace MobileGL::MG_Backend::DirectGLES { SizeT byteSize, GLenum uploadType, Vector& widenedData, Bool integerData = false); + // Splits a GL_UNSIGNED_INT_2_10_10_10_REV shadow (rgb10_a2, rgb10_a2ui) into the four + // GL_UNSIGNED_SHORT channel CODES its GL_RGBA16UI image carrier is uploaded as: red in + // bits 0-9, green 10-19, blue 20-29, alpha 30-31. Pure CPU and context-free so a unit test + // can pin the exact fields; `widenedData` is the caller's scratch and has to outlive the + // returned pointer. + const void* PreparePackedIntWidenedUpload(const IntVec3& texelSize, const void* data, SizeT byteSize, + Vector& widenedData); + struct StateTextureBasicInfo { // Used for tracking texture state changes TextureInternalFormat internalFormat = TextureInternalFormat::Unknown; SizeT width = 0; diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 2d3ba4c1..4f8ecdcb 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -277,18 +277,27 @@ namespace MobileGL::MG_Backend::DirectGLES { // its own; this call is only here to spell the transfer pair that describes it. MG_Util::TextureFormatProcessor::NormalizePixelFormat(carrier, Flags{}, nullptr, &widening.Format, &widening.Type); - // r11f_g11f_b10f is the one carrier that is not a channel widening, and the transfer - // pair has to say so. Every other entry keeps the frontend format's own component - // type - a GL_RG16F shadow is halves and so is its GL_RGBA16F carrier, so padding the - // channels is the whole conversion. This shadow is a PACKED 32-bit word (GL_RGB with - // GL_UNSIGNED_INT_10F_11F_11F_REV, TextureFormatProcessor::NormalizePixelFormat), and - // no ES driver accepts that type for a GL_RGBA16F level. GL_FLOAT is asked for - // instead - legal for GL_RGBA16F, and the type the unpack in - // PrepareImageWidenedUpload writes - so the two sides name the same layout. - if (internalFormat == TextureInternalFormat::R11FG11FB10F) { + // The two carriers that are not channel widenings, whose transfer pair has to say so. + // Every other entry keeps the frontend format's own component type - a GL_RG16F shadow + // is halves and so is its GL_RGBA16F carrier, so padding the channels is the whole + // conversion. These two shadows are a PACKED 32-bit word per texel + // (TextureFormatProcessor::NormalizePixelFormat), and no ES driver accepts either + // packed type for the carrier's level, so the transfer names the carrier's own layout + // and PrepareImageWidenedUpload splits the word into it. + switch (internalFormat) { + case TextureInternalFormat::R11FG11FB10F: + // GL_UNSIGNED_INT_10F_11F_11F_REV -> GL_RGBA / GL_FLOAT, legal for GL_RGBA16F. widening.Format = GL_RGBA; widening.Type = GL_FLOAT; - widening.PackedFloatSource = true; + widening.SourceEncoding = ImageWidenSourceEncoding::PackedFloat11f11f10f; + break; + case TextureInternalFormat::RGB10A2UI: + // GL_UNSIGNED_INT_2_10_10_10_REV -> the GL_RGBA_INTEGER / GL_UNSIGNED_SHORT the + // GL_RGBA16UI carrier already asked for above; only the split is new. + widening.SourceEncoding = ImageWidenSourceEncoding::PackedInt2101010Rev; + break; + default: + break; } return widening; } diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index b062671c..8ba33ff5 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -102,6 +102,22 @@ namespace MobileGL::MG_Backend::DirectGLES { // would see them. Closing it needs the per-draw-buffer colour mask the three-channel // widening already carries (FramebufferImpl::g_alphaWidenedDrawBufferMask) generalized // from "alpha" to a channel count, which is its own change. + // How the FRONTEND's CPU shadow for a widened format is laid out relative to the carrier's + // transfer, i.e. what the upload has to do to it. Almost every entry is `Components`: the + // shadow already holds SourceChannels components of exactly the carrier's own type, so + // padding it out to four is the whole conversion. The packed entries do not - their shadow + // is ONE 32-bit word per texel - and reading such a word as components of the carrier's + // type takes twelve or sixteen bytes out of four and shears the level. + enum class ImageWidenSourceEncoding : Uint8 { + Components = 0, + // r11f_g11f_b10f: GL_UNSIGNED_INT_10F_11F_11F_REV -> four GL_FLOATs of an rgba16f. + PackedFloat11f11f10f, + // rgb10_a2 and rgb10_a2ui: GL_UNSIGNED_INT_2_10_10_10_REV -> four GL_UNSIGNED_SHORT + // channel CODES of an rgba16ui. The same split serves both: the two formats differ + // only in what the codes MEAN, which is the shader's business and not the transfer's. + PackedInt2101010Rev, + }; + struct ImageBindableStorageWidening { GLenum InternalFormat = GL_UNKNOWN_MGL; GLenum Format = GL_UNKNOWN_MGL; @@ -113,13 +129,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // transfer type cannot tell the two apart (GL_UNSIGNED_BYTE serves both RG8 and // RG8UI), so the carrier decides. Bool IntegerData = false; - // The frontend shadow is a PACKED word rather than SourceChannels separate components - // of the carrier's own type, so the upload has to DECODE it instead of padding it out - // (PrepareImageWidenedUpload). True only for r11f_g11f_b10f, whose shadow is one - // GL_UNSIGNED_INT_10F_11F_11F_REV per texel and whose carrier is GL_RGBA16F: the - // channel repack every other entry uses would read three floats out of a four-byte - // texel and shear the level. - Bool PackedFloatSource = false; + // What the upload has to do to the frontend shadow before it describes the level to + // the driver (PrepareImageWidenedUpload). + ImageWidenSourceEncoding SourceEncoding = ImageWidenSourceEncoding::Components; explicit operator Bool() const { return InternalFormat != GL_UNKNOWN_MGL; } }; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp index 53e52209..550fb6fb 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp @@ -351,6 +351,93 @@ void main() } } + // GL_RGB10_A2UI, the format all four allFormats walkers stop at once r11f_g11f_b10f is + // carried - and the only widening whose carrier has as MANY channels as the original, so + // GL leaves nothing to pin and neither access is rewritten. What it does need is the other + // packed transfer: its shadow is one GL_UNSIGNED_INT_2_10_10_10_REV word per texel, which + // the GL_RGBA16UI carrier is uploaded as four shorts. + // + // The seed is checked through an imageLoad BEFORE anything is stored, for the reason the + // r11f case is: a sheared split still produces plausible integers, and a store would + // overwrite every texel the upload got wrong. Every channel of every texel is distinct, + // and the alpha values walk the whole 0..3 a two-bit channel has - a widening that pinned + // alpha to GL's "1" the way a three-channel one must would pass for texel 1 alone. + TEST_F(NonCoreImageFormatScenario, PackedIntegerImageSplitsItsUploadAndKeepsAllFourChannels) { + if (!Ready()) GTEST_SKIP() << "no GL context"; + if (!ImagesAreUsable()) GTEST_SKIP() << "no image load/store on this driver"; + + constexpr int kTexels = kExtent * kExtent; + std::vector seed(static_cast(kTexels), 0u); + std::vector expected(static_cast(kTexels) * 4u, 0u); + for (int texel = 0; texel < kTexels; ++texel) { + const GLuint r = static_cast(texel) * 7u; // 0 .. 105 + const GLuint g = 1023u - static_cast(texel) * 11u; // 1023 .. 858 + const GLuint b = 512u + static_cast(texel); // 512 .. 527 + const GLuint a = static_cast(texel) % 4u; // the whole 0..3 + seed[texel] = r | (g << 10) | (b << 20) | (a << 30); + expected[texel * 4 + 0] = r; + expected[texel * 4 + 1] = g; + expected[texel * 4 + 2] = b; + expected[texel * 4 + 3] = a; + } + const std::vector wideSeed(static_cast(kTexels) * 4u, 999u); + const GLuint narrow = + MakeTexture(GL_RGB10_A2UI, GL_RGBA_INTEGER, GL_UNSIGNED_INT_2_10_10_10_REV, seed.data()); + const GLuint wide = MakeTexture(GL_RGBA32UI, GL_RGBA_INTEGER, GL_UNSIGNED_INT, wideSeed.data()); + if (narrow == 0 || wide == 0) return; + + const GLuint loadProgram = MakeComputeProgram(R"(#version 430 core + +layout (local_size_x = 1, local_size_y = 1, local_size_z = 1) in; + +layout (rgb10_a2ui, binding = 0) readonly uniform uimage2D narrow; +layout (rgba32ui, binding = 1) writeonly uniform uimage2D wide; + +void main() +{ + ivec2 coord = ivec2(gl_GlobalInvocationID.xy); + imageStore(wide, coord, imageLoad(narrow, coord)); +} +)"); + const GLuint storeProgram = MakeComputeProgram(R"(#version 430 core + +layout (local_size_x = 1, local_size_y = 1, local_size_z = 1) in; + +layout (rgb10_a2ui, binding = 0) writeonly uniform uimage2D narrow; + +void main() +{ + imageStore(narrow, ivec2(gl_GlobalInvocationID.xy), uvec4(11u, 22u, 33u, 2u)); +} +)"); + if (loadProgram == 0 || storeProgram == 0) return; + + BindImage(kNarrowUnit, narrow, GL_RGB10_A2UI, GL_READ_ONLY); + BindImage(kWideUnit, wide, GL_RGBA32UI, GL_WRITE_ONLY); + Dispatch(loadProgram); + + const std::vector loaded = ReadUints(wide, GL_RGBA_INTEGER, 4); + for (int texel = 0; texel < kTexels; ++texel) { + EXPECT_EQ(loaded[texel * 4 + 0], expected[texel * 4 + 0]) << "texel " << texel << " red"; + EXPECT_EQ(loaded[texel * 4 + 1], expected[texel * 4 + 1]) << "texel " << texel << " green"; + EXPECT_EQ(loaded[texel * 4 + 2], expected[texel * 4 + 2]) << "texel " << texel << " blue"; + EXPECT_EQ(loaded[texel * 4 + 3], expected[texel * 4 + 3]) << "texel " << texel << " alpha"; + } + + // THE STORE. All four channels survive - this is the one widened format where GL drops + // nothing, so a mask here would be a bug rather than the emulation. + BindImage(kNarrowUnit, narrow, GL_RGB10_A2UI, GL_WRITE_ONLY); + Dispatch(storeProgram); + + const std::vector stored = ReadUints(narrow, GL_RGBA_INTEGER, 4); + for (int texel = 0; texel < kTexels; ++texel) { + EXPECT_EQ(stored[texel * 4 + 0], 11u) << "texel " << texel << " red"; + EXPECT_EQ(stored[texel * 4 + 1], 22u) << "texel " << texel << " green"; + EXPECT_EQ(stored[texel * 4 + 2], 33u) << "texel " << texel << " blue"; + EXPECT_EQ(stored[texel * 4 + 3], 2u) << "texel " << texel << " alpha"; + } + } + // GL_R8UI: the only format KHR-GL43.shader_image_load_store.single-byte_data_alignment // declares, and one SPIRV-Cross refuses to print for ESSL at all, so before the emulation // no text was produced for the stage and the dispatch could not run. diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index a188f3cc..33b137dc 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -3999,6 +3999,7 @@ namespace { constexpr Uint kGlR8ui = 0x8232; constexpr Uint kGlR32f = 0x822E; constexpr Uint kGlRgb10A2ui = 0x906F; + constexpr Uint kGlRgb10A2 = 0x8059; } // namespace // The KHR-GL4x.packed_depth_stencil.stencil_texturing compute shader, reduced: one format-less @@ -4049,15 +4050,20 @@ void main() { imageStore(uni_image, ivec2(gl_GlobalInvocationID.xy), uvec4(15u, // pass on the ESSL chain and re-declares them in a core carrier SPIRV-Cross does print, so for // those the module is the right place and the text completion would put back the narrow token no // ES driver accepts. r8ui - which the stencil half of the packed_depth_stencil case binds - is -// one of the rescued ones; rgb10_a2ui, whose 10/10/10/2 channel widths no core format has, is not. +// one of the rescued ones; rgb10_a2, whose 10/10/10/2 NORMALIZED channels no core format has, is +// not. TEST_F(ProgramUtilTest, BakeImageFormatsLeavesOnlyTheFormatsNoCoreCarrierRescues) { using namespace MG_Util::ShaderTranspiler; ASSERT_FALSE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(kGlR8ui)) << "if SPIRV-Cross ever learns to print r8ui for ES, this route can go"; ASSERT_NE(ShaderCompiler::WidenedCoreEsslImageFormat(kGlR8ui), 0u); + // rgb10_a2ui is unprintable too and IS rescued: its channels are unsigned INTEGER, so an + // rgba16ui holds all four of them outright. ASSERT_FALSE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(kGlRgb10A2ui)); - ASSERT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(kGlRgb10A2ui), 0u); + ASSERT_NE(ShaderCompiler::WidenedCoreEsslImageFormat(kGlRgb10A2ui), 0u); + ASSERT_FALSE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(kGlRgb10A2)); + ASSERT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(kGlRgb10A2), 0u); ASSERT_TRUE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(kGlR32ui)); EXPECT_EQ(ShaderCompiler::EsslImageFormatSpelling(kGlR8ui), "r8ui"); EXPECT_EQ(ShaderCompiler::EsslImageFormatSpelling(0x8051 /*GL_RGB8*/), ""); @@ -4072,7 +4078,7 @@ void main() { imageStore(uni_image, ivec2(0), uvec4(15u)); } { // Unprintable AND uncarriable: declined, module untouched, and the stage still transpiles. Vector baked; - ASSERT_TRUE(ShaderCompiler::BakeImageFormatsForEssl(spirv, {{"uni_image", kGlRgb10A2ui}}, baked)); + ASSERT_TRUE(ShaderCompiler::BakeImageFormatsForEssl(spirv, {{"uni_image", kGlRgb10A2}}, baked)); EXPECT_EQ(baked, spirv) << "a format nothing can carry must leave the module untouched"; EXPECT_FALSE(DecompileToEssl(baked).empty()); } diff --git a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp index efc85822..a6b98723 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp @@ -255,7 +255,21 @@ void main() { } )"; - // rg16 is one of the EIGHT with no core carrier at all - core ESSL has no 16-bit normalized + // rgb10_a2ui: FOUR unsigned-integer channels of 10, 10, 10 and 2 bits, carried in an rgba16ui + // that gives each of them sixteen. The only widening whose carrier has as many channels as the + // original, so it is the only one where GL leaves NOTHING to pin and both accesses must come + // out exactly as glslang emitted them. + const char* const kRgb10A2uiLoadStore = R"(#version 430 core +layout(rgb10_a2ui, binding = 0) uniform uimage2D img; +out vec4 fragColor; +void main() { + uvec4 texel = imageLoad(img, ivec2(gl_FragCoord.xy)); + imageStore(img, ivec2(gl_FragCoord.xy), uvec4(7u, 8u, 9u, 3u)); + fragColor = vec4(texel); +} +)"; + + // rg16 is one of the SEVEN with no core carrier at all - core ESSL has no 16-bit normalized // format, so every candidate loses range or changes the component type the texture presents. // It must be left alone and keep the honest "no GLSL ES spelling" diagnostic instead. const char* const kRg16LoadStore = R"(#version 430 core @@ -273,7 +287,7 @@ void main() { // the shader rewrite, the ES texture storage and the glBindImageTexture argument. If it drifts // the three stop agreeing, and a narrow texture read through a wide image goes out of bounds // silently on every driver tested. -TEST(WidenImageFormats, EighteenNonCoreFormatsHaveALosslessCoreCarrier) { +TEST(WidenImageFormats, NineteenNonCoreFormatsHaveALosslessCoreCarrier) { struct Case { Uint requested; Uint carrier; @@ -302,6 +316,9 @@ TEST(WidenImageFormats, EighteenNonCoreFormatsHaveALosslessCoreCarrier) { // is e5m5 against a half's s1e5m10, so the carrier is still lossless - and three channels, // so the mask has to pin only alpha. {0x8C3A, 0x881A, 3, "GL_R11F_G11F_B10F -> GL_RGBA16F"}, + // FOUR channels: 10, 10, 10 and 2 bits of unsigned integer all fit in sixteen, so nothing + // is masked at all and only the packed TRANSFER is re-encoded. + {0x906F, 0x8D76, 4, "GL_RGB10_A2UI -> GL_RGBA16UI"}, }; for (const Case& testCase : cases) { EXPECT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(testCase.requested), testCase.carrier) @@ -317,7 +334,7 @@ TEST(WidenImageFormats, EighteenNonCoreFormatsHaveALosslessCoreCarrier) { } } -TEST(WidenImageFormats, CoreFormatsAndTheEightWithoutALosslessCarrierAreRefused) { +TEST(WidenImageFormats, CoreFormatsAndTheSevenWithoutALosslessCarrierAreRefused) { // The thirteen GLSL ES already has: nothing to carry. for (const Uint coreFormat : {0x8814u /*RGBA32F*/, 0x881Au /*RGBA16F*/, 0x822Eu /*R32F*/, 0x8058u /*RGBA8*/, 0x8F97u /*RGBA8_SNORM*/, 0x8D82u /*RGBA32I*/, @@ -327,13 +344,12 @@ TEST(WidenImageFormats, CoreFormatsAndTheEightWithoutALosslessCarrierAreRefused) EXPECT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(coreFormat), 0u) << "core format 0x" << std::hex << coreFormat; } - // The eight with no LOSSLESS core carrier: core ESSL has no 16-bit normalized format and no + // The seven with no LOSSLESS core carrier: core ESSL has no 16-bit normalized format and no // 10-bit one, so every candidate for these either loses range or changes the component type // the texture presents to anything that samples it. Deliberately left to the honest - // diagnostic. r11f_g11f_b10f is NOT among them - rgba16f holds every value it can, so it is - // carried above. - for (const Uint hardFormat : {0x8059u /*RGB10_A2*/, - 0x906Fu /*RGB10_A2UI*/, 0x805Bu /*RGBA16*/, 0x822Cu /*RG16*/, + // diagnostic. r11f_g11f_b10f is NOT among them - rgba16f holds every value it can - and + // neither is rgb10_a2ui, whose channels are INTEGER and fit an rgba16ui outright. + for (const Uint hardFormat : {0x8059u /*RGB10_A2*/, 0x805Bu /*RGBA16*/, 0x822Cu /*RG16*/, 0x822Au /*R16*/, 0x8F9Bu /*RGBA16_SNORM*/, 0x8F99u /*RG16_SNORM*/, 0x8F98u /*R16_SNORM*/}) { EXPECT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(hardFormat), 0u) @@ -429,6 +445,38 @@ TEST(WidenImageFormats, ThreeChannelPackedFloatImageBecomesRgba16fWithOnlyAlphaP EXPECT_TRUE(HasComponents(*loadMask, {0u, 1u, 2u, 7u})); } +// The four-channel case, which is the whole of rgb10_a2ui's shader-side emulation: the carrier has +// as many channels as the original, every value of every channel fits, and GL therefore defines +// NOTHING about a surplus channel because there is none. So both accesses have to come out +// untouched - a pass that masked here would replace the alpha the application stored (0..3 of a +// two-bit channel, which the CTS walker writes as 3) with the constant 1 and drop blue outright. +TEST(WidenImageFormats, FourChannelIntegerImageBecomesRgba16uiWithNeitherAccessMasked) { + const Vector spirv = CompileFragment(kRgb10A2uiLoadStore); + ASSERT_FALSE(spirv.empty()); + ASSERT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); + + const auto beforeTypes = CollectStorageImageTypes(spirv); + ASSERT_EQ(beforeTypes.size(), 1u); + EXPECT_EQ(beforeTypes.front().format, static_cast(spv::ImageFormat::Rgb10a2ui)); + + Vector widened; + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true)); + ASSERT_FALSE(widened.empty()); + EXPECT_TRUE(Validates(widened)); + EXPECT_FALSE(ShaderCompiler::DeclaresWidenableImageFormat(widened)); + + const auto afterTypes = CollectStorageImageTypes(widened); + ASSERT_EQ(afterTypes.size(), 1u); + EXPECT_EQ(afterTypes.front().format, static_cast(spv::ImageFormat::Rgba16ui)); + + // The declaration moved and nothing else did. + EXPECT_EQ(CollectVectorShuffles(widened).size(), CollectVectorShuffles(spirv).size()) + << "a carrier with as many channels as the original must add no mask"; + EXPECT_EQ(CollectImageReadResultIds(widened).size(), CollectImageReadResultIds(spirv).size()) + << "the imageLoad was duplicated for a rewrite that has nothing to rewrite"; +} + // ...and the same module through the emitter, which is where the failure actually showed: ESSL has // no `r11f_g11f_b10f` token, SPIRV-Cross throws for it, and the throw took every image uniform // declared in the same stage with it. diff --git a/MobileGL/MG_Test/Texture/TextureTest.cpp b/MobileGL/MG_Test/Texture/TextureTest.cpp index a638602f..40534417 100644 --- a/MobileGL/MG_Test/Texture/TextureTest.cpp +++ b/MobileGL/MG_Test/Texture/TextureTest.cpp @@ -5286,3 +5286,53 @@ TEST_F(TextureTest, ImageWidenedUploadExpandsOneAndTwoChannelDataWithGLsMissingC EXPECT_TRUE(widened.empty()); } } + +// The OTHER transfer shape the image widening needs, and the one a channel repack cannot serve: +// GL_RGB10_A2UI's shadow is ONE 32-bit word per texel, not four components of the GL_RGBA16UI +// carrier's own type. Repacking it as components would take sixteen bytes out of a four-byte texel +// and shear the level - which only a LOAD notices, because a store overwrites whatever the upload +// got wrong. +// +// GL_UNSIGNED_INT_2_10_10_10_REV puts the FIRST component in the LOW bits, which is the whole +// content of the word "REV" and the single thing this can get backwards, so every field here is a +// different value and the boundary codes (0, the 10-bit maximum, the 2-bit maximum) are pinned +// exactly rather than compared with a tolerance. +TEST_F(TextureTest, ImageWidenedUploadSplitsAPacked2101010RevShadowIntoFourChannelCodes) { + using MobileGL::MG_Backend::DirectGLES::TextureImpl::PreparePackedIntWidenedUpload; + + const IntVec3 texelSize(3, 1, 1); + // r=1, g=2, b=3, a=1 | r=1023, g=0, b=1023, a=3 | r=0, g=1023, b=0, a=0 + const Uint32 source[] = { + 1u | (2u << 10) | (3u << 20) | (1u << 30), + 1023u | (0u << 10) | (1023u << 20) | (3u << 30), + 0u | (1023u << 10) | (0u << 20) | (0u << 30), + }; + Vector widened; + const auto* result = static_cast( + PreparePackedIntWidenedUpload(texelSize, source, sizeof(source), widened)); + ASSERT_NE(result, static_cast(source)); + ASSERT_EQ(widened.size(), 12 * sizeof(Uint16)); + const Uint16 expected[] = {1, 2, 3, 1, 1023, 0, 1023, 3, 0, 1023, 0, 0}; + for (SizeT i = 0; i < 12; ++i) { + EXPECT_EQ(result[i], expected[i]) << "component " << i; + } + + // Sized from the LEVEL, never from the source: the driver reads a full width*height*4 shorts + // for the transfer it was handed, so a short source still has to leave a full destination. + { + Vector shortWidened; + const auto* shortResult = static_cast( + PreparePackedIntWidenedUpload(texelSize, source, sizeof(Uint32), shortWidened)); + ASSERT_EQ(shortWidened.size(), 12 * sizeof(Uint16)); + for (SizeT i = 4; i < 12; ++i) { + EXPECT_EQ(shortResult[i], 0u) << "component " << i << " past the source must be zero"; + } + } + + // Nothing to split. + { + Vector empty; + EXPECT_EQ(PreparePackedIntWidenedUpload(texelSize, nullptr, 0, empty), nullptr); + EXPECT_TRUE(empty.empty()); + } +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp index eecdcbed..4105c9d5 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp @@ -60,12 +60,20 @@ namespace MobileGL { // per-channel width, more channels (rg32f -> rgba32f) - and for those the carrier // is bit-exact: the storage holds the identical encoding, only wider. // - // r11f_g11f_b10f is the one entry that is not. It has no same-width core carrier, - // so it takes rgba16f, and the two encodings differ. What matters is that the - // carrier is still LOSSLESS: an 11-bit float is e5m6 and a 10-bit float is e5m5, - // while a half is s1e5m10 - the SAME 5-bit exponent with a strictly longer - // mantissa - so every value the packed format can represent has an exact half. - // Nothing an application stores is rounded away. + // Two entries are not. r11f_g11f_b10f has no same-width core carrier, so it takes + // rgba16f, and the two encodings differ. What matters is that the carrier is still + // LOSSLESS: an 11-bit float is e5m6 and a 10-bit float is e5m5, while a half is + // s1e5m10 - the SAME 5-bit exponent with a strictly longer mantissa - so every + // value the packed format can represent has an exact half. Nothing an application + // stores is rounded away. + // + // rgb10_a2ui is the other. Its four channels are 10, 10, 10 and 2 bits of UNSIGNED + // INTEGER, and rgba16ui gives each of them sixteen - every value of every channel + // fits, with the same component type and the same channel COUNT, so nothing is + // masked and nothing is re-encoded on the shader side at all. Only the transfer + // differs: the frontend's shadow for it is one packed 32-bit word per texel + // (GL_UNSIGNED_INT_2_10_10_10_REV), so the upload has to split that word into four + // shorts the way r11f_g11f_b10f's has to be decoded into four floats. // // What DOES change is the reverse direction: the carrier can hold values the // packed format could not - negatives (11f and 10f are unsigned), and mantissa @@ -79,12 +87,12 @@ namespace MobileGL { // this format alone, and multiple-uniforms, where one such declaration killed a // program holding eight images). // - // The remaining eight - rgb10_a2, rgb10_a2ui, rgba16, rg16, r16, rgba16_snorm, - // rg16_snorm and r16_snorm - stay absent, and for a stronger reason than - // quantisation: core ESSL has no 16-bit normalized format at all and no 10-bit - // one, so every candidate carrier for them either loses range or changes the - // component TYPE the texture presents. They keep the honest "no GLSL ES spelling" - // diagnostic rather than a silent approximation. + // The remaining seven - rgb10_a2, rgba16, rg16, r16, rgba16_snorm, rg16_snorm and + // r16_snorm - stay absent, and for a stronger reason than quantisation: core ESSL + // has no 16-bit normalized format at all and no 10-bit one, so every candidate + // carrier for them either loses range or changes the component TYPE the texture + // presents. They keep the honest "no GLSL ES spelling" diagnostic rather than a + // silent approximation. struct ImageFormatWidening { spv::ImageFormat Carrier = spv::ImageFormat::Unknown; uint32_t Channels = 0; @@ -119,6 +127,9 @@ namespace MobileGL { case spv::ImageFormat::R16ui: return {spv::ImageFormat::Rgba16ui, 1}; case spv::ImageFormat::Rg8ui: return {spv::ImageFormat::Rgba8ui, 2}; case spv::ImageFormat::R8ui: return {spv::ImageFormat::Rgba8ui, 1}; + // FOUR channels, so there is no surplus channel to mask and no access is + // rewritten - 10, 10, 10 and 2 bits of unsigned integer all fit in sixteen. + case spv::ImageFormat::Rgb10a2ui: return {spv::ImageFormat::Rgba16ui, 4}; default: return {}; } @@ -513,6 +524,10 @@ namespace MobileGL { for (Instruction* write : writes) { const WidenedImage* widened = widenedOf(write); if (widened == nullptr) continue; + // A carrier with as many channels as the original (rgb10_a2ui in rgba16ui) has + // no surplus channel to pin, and the shuffle would select (0, 1, 2, 3) from the + // texel - an identity the emitter would still print. Left out entirely. + if (widened->Channels >= 4) continue; uint32_t zeroOneId = 0; uint32_t vec4TypeId = 0; if (!resolveMaskMaterial(widened->SampledTypeId, zeroOneId, vec4TypeId)) { @@ -540,6 +555,7 @@ namespace MobileGL { for (Instruction* read : reads) { const WidenedImage* widened = widenedOf(read); if (widened == nullptr) continue; + if (widened->Channels >= 4) continue; // see the store loop uint32_t zeroOneId = 0; uint32_t vec4TypeId = 0; if (!resolveMaskMaterial(widened->SampledTypeId, zeroOneId, vec4TypeId)) { diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h index e684dc0f..76d22e36 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h @@ -65,12 +65,18 @@ namespace MobileGL { // the SPIRV-Cross throw takes the whole stage, every image uniform declared beside it // included. // - // The other EIGHT (rgb10_a2, rgb10_a2ui, rgba16, rg16, r16, rgba16_snorm, rg16_snorm, - // r16_snorm) are deliberately NOT widened here: core ESSL has no 16-bit normalized - // format at all and no 10-bit one, so every carrier for them either loses range or - // changes the component TYPE the texture a `sampler2D` would read presents. They keep - // the honest "no GLSL ES spelling" diagnostic instead of silently changing an - // application's numeric domain. + // rgb10_a2ui takes rgba16ui for a simpler reason still: its channels are 10, 10, 10 and + // 2 bits of UNSIGNED INTEGER, and rgba16ui gives each of them sixteen. Same component + // type, same channel COUNT, every value representable - so no access is rewritten at + // all, and only the TRANSFER differs (its shadow is one packed 32-bit word per texel, + // which the upload splits into four shorts). + // + // The other SEVEN (rgb10_a2, rgba16, rg16, r16, rgba16_snorm, rg16_snorm, r16_snorm) + // are deliberately NOT widened here: core ESSL has no 16-bit normalized format at all + // and no 10-bit one, so every carrier for them either loses range or changes the + // component TYPE the texture a `sampler2D` would read presents. They keep the honest + // "no GLSL ES spelling" diagnostic instead of silently changing an application's + // numeric domain. // // MUST MOVE WITH THE OTHER TWO LAYERS. The widening is not a shader-local rewrite: the // ES texture behind the image has to be allocated in the carrier format too, and