From 9bd125aeec7c1b72386534ac75ab8bc68801e437 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 21:21:54 -0400 Subject: [PATCH 1/4] [Fix, Test] (ShaderTranspiler): carry r11f_g11f_b10f storage images in rgba16f instead of losing the stage --- .../WidenImageFormatsTest.cpp | 108 +++++++++++++++--- .../SpirvPasses/WidenImageFormatsPass.cpp | 45 ++++++-- .../SpirvPasses/WidenImageFormatsPass.h | 23 +++- 3 files changed, 148 insertions(+), 28 deletions(-) diff --git a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp index 8f94f164..e73c346d 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp @@ -14,10 +14,10 @@ // and every draw with the program silently renders nothing while GL_LINK_STATUS still says TRUE. // // What has to hold is the emulation's exactness, in three parts at once: the DECLARED format must -// become the core carrier of the same per-channel width, every imageStore through it must have its -// surplus components replaced by GL's own (0.., 1) so the carrier's extra channels never hold -// anything GL has not defined, and every imageLoad must come back masked the same way. A module -// that declares only core formats - or one of the nine formats with no exact carrier - must come +// become a core carrier that loses nothing, every imageStore through it must have its surplus +// components replaced by GL's own (0.., 1) so the carrier's extra channels never hold anything GL +// has not defined, and every imageLoad must come back masked the same way. A module that declares +// only core formats - or one of the eight formats with no lossless carrier at all - must come // out untouched, because widening those would be an approximation rather than an emulation. Real // GLSL through the same glslang path the backends use, for the same reason // ClampMultisampleFetchTest.cpp does it: what matters is what glslang actually emits. @@ -229,9 +229,23 @@ void main() { } )"; - // rg16 is one of the NINE with no core carrier of the same per-channel width. Widening it - // would change the quantisation an application sees, so it must be left alone and keep the - // honest "no GLSL ES spelling" diagnostic instead. + // r11f_g11f_b10f: THREE float channels in a packed 32-bit word, and the only format the four + // CTS allFormats/allTargets walkers still aborted on after the channel widening landed - it + // has no core carrier of the same per-channel width, so it took rgba16f, whose 5-bit exponent + // and longer mantissa represent every 11f and 10f value exactly. + const char* const kR11fG11fB10fLoadStore = R"(#version 430 core +layout(r11f_g11f_b10f, binding = 0) uniform image2D img; +out vec4 fragColor; +void main() { + vec4 texel = imageLoad(img, ivec2(gl_FragCoord.xy)); + imageStore(img, ivec2(gl_FragCoord.xy), vec4(1.0, 2.0, 3.0, 4.0)); + fragColor = texel; +} +)"; + + // rg16 is one of the EIGHT 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 layout(rg16, binding = 0) uniform image2D img; out vec4 fragColor; @@ -247,7 +261,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, SeventeenNonCoreFormatsHaveAnExactSameWidthCarrier) { +TEST(WidenImageFormats, EighteenNonCoreFormatsHaveALosslessCoreCarrier) { struct Case { Uint requested; Uint carrier; @@ -272,6 +286,10 @@ TEST(WidenImageFormats, SeventeenNonCoreFormatsHaveAnExactSameWidthCarrier) { {0x8234, 0x8D76, 1, "GL_R16UI -> GL_RGBA16UI"}, {0x8238, 0x8D7C, 2, "GL_RG8UI -> GL_RGBA8UI"}, {0x8232, 0x8D7C, 1, "GL_R8UI -> GL_RGBA8UI"}, + // The one entry that is a re-encoding rather than a channel widening: 11f is e5m6 and 10f + // 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"}, }; for (const Case& testCase : cases) { EXPECT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(testCase.requested), testCase.carrier) @@ -287,7 +305,7 @@ TEST(WidenImageFormats, SeventeenNonCoreFormatsHaveAnExactSameWidthCarrier) { } } -TEST(WidenImageFormats, CoreFormatsAndTheNineWithoutAnExactCarrierAreRefused) { +TEST(WidenImageFormats, CoreFormatsAndTheEightWithoutALosslessCarrierAreRefused) { // 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*/, @@ -297,10 +315,12 @@ TEST(WidenImageFormats, CoreFormatsAndTheNineWithoutAnExactCarrierAreRefused) { EXPECT_EQ(ShaderCompiler::WidenedCoreEsslImageFormat(coreFormat), 0u) << "core format 0x" << std::hex << coreFormat; } - // The nine with no core format of the same per-channel width. Carrying these would be an - // approximation - a different quantisation, or a different numeric domain for anything that - // samples the same texture - so they are deliberately left to the honest diagnostic. - for (const Uint hardFormat : {0x8C3Au /*R11F_G11F_B10F*/, 0x8059u /*RGB10_A2*/, + // The eight 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*/, 0x822Au /*R16*/, 0x8F9Bu /*RGBA16_SNORM*/, 0x8F99u /*RG16_SNORM*/, 0x8F98u /*R16_SNORM*/}) { @@ -357,6 +377,68 @@ TEST(WidenImageFormats, TwoChannelFloatImageBecomesRgba32fWithBothAccessesMasked << "the mask must be a separate value, or it would feed itself"; } +// The three-channel case, which no format exercised before r11f_g11f_b10f was carried: only ALPHA +// is surplus, so the mask must take r, g and b from the texel and nothing but the fourth component +// from the (0, 0, 0, 1) constant. A mask that zeroed blue here - the shape a two-channel format +// wants - would silently drop the third channel of every store. +TEST(WidenImageFormats, ThreeChannelPackedFloatImageBecomesRgba16fWithOnlyAlphaPinned) { + const Vector spirv = CompileFragment(kR11fG11fB10fLoadStore); + 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::R11fG11fB10f)); + + 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::Rgba16f)); + + const auto shuffles = CollectVectorShuffles(widened); + + const auto texelIds = CollectImageWriteTexelIds(widened); + ASSERT_EQ(texelIds.size(), 1u); + const VectorShuffle* storeMask = FindShuffleWithResult(shuffles, texelIds.front()); + ASSERT_NE(storeMask, nullptr) << "the imageStore texel is not a masked value"; + EXPECT_TRUE(HasComponents(*storeMask, {0u, 1u, 2u, 7u})) + << "expected (r, g, b, 1) - components 0, 1 and 2 of the texel, then 3 of (0,0,0,1)"; + + const auto readIds = CollectImageReadResultIds(widened); + ASSERT_EQ(readIds.size(), 1u); + const VectorShuffle* loadMask = FindShuffleOver(shuffles, readIds.front()); + ASSERT_NE(loadMask, nullptr) << "the imageLoad result is consumed unmasked"; + EXPECT_TRUE(HasComponents(*loadMask, {0u, 1u, 2u, 7u})); +} + +// ...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. +TEST(WidenImageFormats, PackedFloatImageOnlyReachesEsslThroughTheCarrier) { + const Vector spirv = CompileFragment(kR11fG11fB10fLoadStore); + ASSERT_FALSE(spirv.empty()); + + const EsslAttempt before = EmitEssl(spirv); + EXPECT_FALSE(before.succeeded) + << "SPIRV-Cross printed r11f_g11f_b10f for an ES target; the widening's premise has " + "changed:\n" + << before.text; + + Vector widened; + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true)); + const EsslAttempt after = EmitEssl(widened); + ASSERT_TRUE(after.succeeded) << after.error; + EXPECT_NE(after.text.find("rgba16f"), String::npos) << after.text; + EXPECT_EQ(after.text.find("r11f_g11f_b10f"), String::npos) << after.text; +} + TEST(WidenImageFormats, SingleChannelUnsignedImageBecomesRgba8uiWithBothAccessesMasked) { const Vector spirv = CompileFragment(kR8uiLoadStore); ASSERT_FALSE(spirv.empty()); diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp index 2b074fd5..174896a8 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp @@ -50,16 +50,40 @@ namespace MobileGL { constexpr uint32_t kImageAccessImageOperand = 0; constexpr uint32_t kImageWriteTexelOperand = 2; - // The exact carrier of a non-core image format: the core GLSL ES format with the - // SAME component type and the SAME per-channel width, differing only in channel - // count. `channels` is what the original format really has, which is what every - // access through the carrier is masked back to. + // The carrier of a non-core image format: a core GLSL ES format that represents + // every value the original can hold, WITHOUT LOSS. `channels` is what the original + // format really has, which is what every access through the carrier is masked back + // to. // - // Only formats that widen EXACTLY appear here. r11f_g11f_b10f, rgb10_a2, - // rgb10_a2ui, rgba16, rg16, r16, rgba16_snorm, rg16_snorm and r16_snorm have no - // same-width core carrier - every candidate is either lossy or changes the numeric - // domain a sampler would read - and are deliberately absent, so they keep the - // honest "no GLSL ES spelling" diagnostic rather than a silent approximation. + // Almost every entry is a pure CHANNEL widening - same component type, same + // 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. + // + // 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 + // bits finer than the 6 and 5 the format quantises to - so a value written through + // the image and then SAMPLED comes back on half's grid rather than the packed + // format's. That is a strictly finer grid, never a lossy one, and it is measured + // against the alternative, which is not a more faithful quantisation but no + // program at all: `layout(r11f_g11f_b10f)` has no ESSL spelling, SPIRV-Cross + // throws for it, and the stage - with every other image uniform declared beside it + // - is lost (KHR-GL43.shader_image_load_store.basic-allFormats-*, which fail on + // 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. struct ImageFormatWidening { spv::ImageFormat Carrier = spv::ImageFormat::Unknown; uint32_t Channels = 0; @@ -73,6 +97,9 @@ namespace MobileGL { case spv::ImageFormat::Rg32f: return {spv::ImageFormat::Rgba32f, 2}; case spv::ImageFormat::Rg16f: return {spv::ImageFormat::Rgba16f, 2}; case spv::ImageFormat::R16f: return {spv::ImageFormat::Rgba16f, 1}; + // Not a channel widening but a lossless re-encoding - see above. Three + // channels, so the fourth reads as the 1 GL defines for a format without one. + case spv::ImageFormat::R11fG11fB10f: return {spv::ImageFormat::Rgba16f, 3}; // Unsigned normalized. case spv::ImageFormat::Rg8: return {spv::ImageFormat::Rgba8, 2}; case spv::ImageFormat::R8: return {spv::ImageFormat::Rgba8, 1}; diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h index f5d7be0a..e684dc0f 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h @@ -54,12 +54,23 @@ namespace MobileGL { // alone survives storage this shader never wrote (glTexStorage with no upload, whose // surplus channels are undefined). // - // The other NINE (r11f_g11f_b10f, rgb10_a2, rgb10_a2ui, rgba16, rg16, r16, - // rgba16_snorm, rg16_snorm, r16_snorm) have NO same-width core carrier and are - // deliberately NOT widened here: every carrier for them is either lossy or changes the - // numeric domain of the texture a `sampler2D` would read from it. They keep the honest - // "no GLSL ES spelling" diagnostic instead of silently changing an application's - // quantisation behaviour. + // r11f_g11f_b10f has no same-width core carrier either, and takes rgba16f anyway, + // because that carrier is still LOSSLESS: 11f is e5m6 and 10f is e5m5 against a half's + // s1e5m10 - the SAME 5-bit exponent with a strictly longer mantissa - so every value + // the packed format can hold has an exact half. Only the reverse direction differs + // (the carrier also holds negatives, which 11f and 10f cannot sign, and mantissa bits + // finer than the 6 and 5 they quantise to, so a value written through the image and + // then SAMPLED lands on half's grid rather than the packed format's). That is measured + // against the alternative, which is not a truer quantisation but no program at all: + // 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. // // 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 From 529d26f38fdc09c21407ffb4bce4dce020782fb0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 21:27:14 -0400 Subject: [PATCH 2/4] [Fix] (DirectGLES): arm the image-format widening for r11f_g11f_b10f in the reflection gate too --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 25 +++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f9fa4894..0a81fb6c 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5150,12 +5150,19 @@ namespace MobileGL::MG_Backend::DirectGLES { } } - // The GL internal format a glslang layout format names, for the seventeen non-core - // formats WidenImageFormatsForEssl carries exactly plus nothing else: the only + // The GL internal format a glslang layout format names, for the eighteen non-core + // formats WidenImageFormatsForEssl carries losslessly plus nothing else: the only // question asked of it is "does this DECLARED format widen", and answering 0 for // everything else is the same "no" a non-widenable format gets. Kept as its own // switch rather than routed through the frontend's enum converters because a // TLayoutFormat is a glslang value and the reflection snapshot stores it raw. + // + // IT MUST LIST EXACTLY WHAT WideningOfSpirvImageFormat DOES. This table is what arms + // the pass (ImageFormatWillBeWidened -> declaresWidenableImageFormat), so a format the + // pass would carry but this switch answers 0 for never gets the chance: the module + // reaches SPIRV-Cross with its original qualifier, the throw takes the stage, and the + // only visible symptom is the "no GLSL ES spelling" diagnostic for a format that has + // one. That is exactly what r11f_g11f_b10f did until it was added here. Uint GLInternalFormatOfLayoutFormat(glslang::TLayoutFormat format) { switch (format) { case glslang::ElfRg32f: return 0x8230; // GL_RG32F @@ -5175,6 +5182,10 @@ namespace MobileGL::MG_Backend::DirectGLES { case glslang::ElfR16ui: return 0x8234; // GL_R16UI case glslang::ElfRg8ui: return 0x8238; // GL_RG8UI case glslang::ElfR8ui: return 0x8232; // GL_R8UI + // Not a channel widening but a lossless re-encoding into rgba16f - the one entry + // here whose carrier has a different per-channel layout. See + // WidenImageFormatsPass.h. + case glslang::ElfR11fG11fB10f: return 0x8C3A; // GL_R11F_G11F_B10F default: return 0; } @@ -5235,11 +5246,11 @@ namespace MobileGL::MG_Backend::DirectGLES { // spelling still has to become legal ESSL somehow. const auto declaredFormat = static_cast(type.layoutFormat); if (!IsCoreEsslLayoutFormat(declaredFormat)) { - // Seventeen of the twenty-six non-core formats are re-declared in the core - // format that carries them exactly, with every access masked back to the - // channels GL says they have (WidenImageFormatsForEssl, and the matching - // storage/bind widening in TextureImpl). Those need neither the extension - // nor the diagnostic: there IS a legal spelling for them now. + // Eighteen of the twenty-six non-core formats are re-declared in a core + // format that carries them losslessly, with every access masked back to + // the channels GL says they have (WidenImageFormatsForEssl, and the + // matching storage/bind widening in TextureImpl). Those need neither the + // extension nor the diagnostic: there IS a legal spelling for them now. if (ImageFormatWillBeWidened(GLInternalFormatOfLayoutFormat(declaredFormat))) { inputs.declaresWidenableImageFormat = true; } else { From f3cd4091bf6624faf3116c4e5dc4a574e07f390c Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 21:38:45 -0400 Subject: [PATCH 3/4] [Fix, Test] (DirectGLES): decode the packed r11f_g11f_b10f shadow into the float level its rgba16f carrier is uploaded as --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 95 +++++++++++++++++-- MobileGL/MG_Backend/DirectGLES/Utils.cpp | 13 +++ MobileGL/MG_Backend/DirectGLES/Utils.h | 7 ++ .../Scenarios/NonCoreImageFormatScenario.cpp | 89 +++++++++++++++++ 4 files changed, 196 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 0a81fb6c..06538896 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -29,7 +29,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -2706,21 +2708,98 @@ namespace MobileGL::MG_Backend::DirectGLES { widenedData, IsIntegerWidenableFormat(format)); } + // One channel of a packed r11f_g11f_b10f word as a float. The two 11-bit channels are + // e5m6 and the 10-bit one e5m5 - IEEE-shaped but UNSIGNED, so there is no sign bit to + // read and the exponent bias is the 15 a 5-bit exponent always carries. + static Float DecodePackedUnsignedFloat(Uint32 bits, Uint mantissaBits) { + const Uint32 mantissaScale = 1u << mantissaBits; + const Uint32 mantissa = bits & (mantissaScale - 1u); + const Uint32 exponent = bits >> mantissaBits; + if (exponent == 0u) { + // Subnormal, and zero with it: no implied leading 1, and the exponent is the + // smallest NORMAL one rather than the encoded 0. + return std::ldexp(static_cast(mantissa) / static_cast(mantissaScale), -14); + } + if (exponent == 31u) { + return mantissa == 0u ? std::numeric_limits::infinity() + : std::numeric_limits::quiet_NaN(); + } + return std::ldexp(1.0f + static_cast(mantissa) / static_cast(mantissaScale), + static_cast(exponent) - 15); + } + + // The r11f_g11f_b10f shadow decoded into the GL_RGBA / GL_FLOAT level its GL_RGBA16F + // carrier is uploaded as. Alpha is the 1 GL defines for a format that has no alpha + // channel, which is the same constant the shader-side mask writes, so a texel this + // function produced and a texel an imageStore produced are indistinguishable. + // + // Sized from the LEVEL, not the source, for the reason PrepareChannelWidenedUpload is: + // the driver reads a full width*height*depth*4 floats for the transfer it was handed. + static const void* PreparePackedFloatWidenedUpload(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(Float), 0); + const auto* src = static_cast(data); + auto* dst = reinterpret_cast(widenedData.data()); + for (SizeT i = 0; i < texelCount; ++i, dst += 4) { + Float rgb[3] = {0.0f, 0.0f, 0.0f}; + if (i < copyTexelCount) { + Uint32 packed = 0; + // 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)); + rgb[0] = DecodePackedUnsignedFloat(packed & 0x7FFu, 6u); + rgb[1] = DecodePackedUnsignedFloat((packed >> 11u) & 0x7FFu, 6u); + rgb[2] = DecodePackedUnsignedFloat((packed >> 22u) & 0x3FFu, 5u); + } + dst[0] = rgb[0]; + dst[1] = rgb[1]; + dst[2] = rgb[2]; + dst[3] = 1.0f; + } + 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 one- or two-component client data has to be repacked the same way the - // three-channel colour-renderable widening repacks its own. + // 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). // // Composes with PrepareFallbackUpload rather than replacing it, and the composition is a - // no-op by construction: none of the seventeen widened formats is a three-channel one - // (GetWidenableClientComponentCount reports 0 for every one of them), and the SNORM - // shadow-to-float conversion only fires for a GL_FLOAT transfer type, which the widened - // triple never picks for the two SNORM8 formats. So the shadow reaches this untouched and - // one repack is all that runs. + // no-op by construction: none of the widened formats is one GetWidenableClientComponentCount + // reports a count for, and the SNORM shadow-to-float conversion only fires for a GL_FLOAT + // transfer type, which the widened triple never picks for the two SNORM8 formats. So the + // shadow reaches this untouched and one conversion is all that runs. static const void* PrepareImageWidenedUpload(const TextureImpl::ImageBindableStorageWidening& widening, const IntVec3& texelSize, const void* data, SizeT byteSize, Vector& widenedData) { - if (!widening || widening.SourceChannels == 0 || widening.SourceChannels >= 4) { + if (!widening || widening.SourceChannels == 0 || widening.SourceChannels > 4) { + return data; + } + if (widening.PackedFloatSource) { + return PreparePackedFloatWidenedUpload(texelSize, data, byteSize, widenedData); + } + if (widening.SourceChannels == 4) { return data; } return PrepareChannelWidenedUpload(widening.SourceChannels, texelSize, data, byteSize, widening.Type, diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index cfdadae7..2d3ba4c1 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -277,6 +277,19 @@ 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) { + widening.Format = GL_RGBA; + widening.Type = GL_FLOAT; + widening.PackedFloatSource = true; + } return widening; } } // namespace TextureImpl diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.h b/MobileGL/MG_Backend/DirectGLES/Utils.h index 8d9d0dcc..b062671c 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.h +++ b/MobileGL/MG_Backend/DirectGLES/Utils.h @@ -113,6 +113,13 @@ 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; 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 60f45bb6..53e52209 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/NonCoreImageFormatScenario.cpp @@ -262,6 +262,95 @@ void main() } } + // GL_R11F_G11F_B10F, the format the allFormats and allTargets walkers stop at once the + // channel widening has carried everything before it - and the one carrier that is NOT a + // channel widening. It has no core format of its own per-channel width, so it is carried + // in GL_RGBA16F, whose 5-bit exponent and longer mantissa hold every 11f (e5m6) and 10f + // (e5m5) value exactly. + // + // What makes this case different from every other one here, and why it is worth its own + // test: the frontend's shadow for this format is ONE PACKED 32-BIT WORD per texel, not + // three components of the carrier's type. The upload therefore has to DECODE it, where + // every other widening only pads channels onto data already in the right component type. + // A widening that reused the channel repack reads three floats out of a four-byte texel + // and shears the whole level - which a STORE test cannot see, because the dispatch + // overwrites every texel the upload got wrong. So the seed here is per-texel distinct and + // is checked through an imageLoad BEFORE anything is stored. + // + // Every constant is chosen to be exact in both encodings, so the comparisons can be + // equality rather than tolerance: the 1/8 steps need three mantissa bits of the 11f + // channels' six, and the 1/16 steps at exponent 1 need one of the 10f channel's five. + TEST_F(NonCoreImageFormatScenario, PackedFloatImageDecodesItsUploadAndDropsSurplusStores) { + 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) * 3u, 0.0f); + for (int texel = 0; texel < kTexels; ++texel) { + seed[texel * 3 + 0] = 1.0f + static_cast(texel) / 8.0f; + seed[texel * 3 + 1] = 2.0f + static_cast(texel) / 8.0f; + seed[texel * 3 + 2] = 3.0f + static_cast(texel) / 16.0f; + } + const std::vector wideSeed(static_cast(kTexels) * 4u, -1.0f); + const GLuint narrow = MakeTexture(GL_R11F_G11F_B10F, GL_RGB, GL_FLOAT, seed.data()); + const GLuint wide = MakeTexture(GL_RGBA32F, GL_RGBA, GL_FLOAT, 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 (r11f_g11f_b10f, binding = 0) readonly uniform image2D narrow; +layout (rgba32f, binding = 1) writeonly uniform image2D 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 (r11f_g11f_b10f, binding = 0) writeonly uniform image2D narrow; + +void main() +{ + imageStore(narrow, ivec2(gl_GlobalInvocationID.xy), vec4(5.0, 6.0, 7.0, 8.0)); +} +)"); + if (loadProgram == 0 || storeProgram == 0) return; + + // THE UPLOAD, read back through the image. A sheared decode still produces plausible + // floats, so the check is per texel and the seed never repeats a value. + BindImage(kNarrowUnit, narrow, GL_R11F_G11F_B10F, GL_READ_ONLY); + BindImage(kWideUnit, wide, GL_RGBA32F, GL_WRITE_ONLY); + Dispatch(loadProgram); + + const std::vector loaded = ReadFloats(wide, GL_RGBA, 4); + for (int texel = 0; texel < kTexels; ++texel) { + EXPECT_FLOAT_EQ(loaded[texel * 4 + 0], seed[texel * 3 + 0]) << "texel " << texel << " red"; + EXPECT_FLOAT_EQ(loaded[texel * 4 + 1], seed[texel * 3 + 1]) << "texel " << texel << " green"; + EXPECT_FLOAT_EQ(loaded[texel * 4 + 2], seed[texel * 3 + 2]) << "texel " << texel << " blue"; + EXPECT_FLOAT_EQ(loaded[texel * 4 + 3], 1.0f) + << "texel " << texel << ": imageLoad on a format without alpha must report 1"; + } + + // THE STORE. Three channels survive and the fourth is dropped, which is the mask this + // format needs and no other widened format does - every other carrier here pins two + // or three of the carrier's channels, this one pins only alpha. + BindImage(kNarrowUnit, narrow, GL_R11F_G11F_B10F, GL_WRITE_ONLY); + Dispatch(storeProgram); + + const std::vector stored = ReadFloats(narrow, GL_RGB, 3); + for (int texel = 0; texel < kTexels; ++texel) { + EXPECT_FLOAT_EQ(stored[texel * 3 + 0], 5.0f) << "texel " << texel << " red"; + EXPECT_FLOAT_EQ(stored[texel * 3 + 1], 6.0f) << "texel " << texel << " green"; + EXPECT_FLOAT_EQ(stored[texel * 3 + 2], 7.0f) << "texel " << texel << " blue"; + } + } + // 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. From e18bac8cb2dbabb75c19f53ed969ef7a4b11afda Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 21 Aug 2026 22:02:23 -0400 Subject: [PATCH 4/4] [Fix, Test] (ShaderTranspiler, DirectGLES): never widen a buffer image - its texels are the application buffer, not storage we can reallocate --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 9 +++- .../WidenImageFormatsTest.cpp | 45 +++++++++++++++++++ .../SpirvPasses/WidenImageFormatsPass.cpp | 19 ++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index c3cc734c..245786a0 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1482,8 +1482,15 @@ namespace MobileGL::MG_Backend::DirectGLES { // a bind format that names a class the storage does not have is left alone: GL // already calls that undefined, and inventing a carrier for it would only make the // out-of-class read wider. + // + // A BUFFER texture is excluded on both sides: it has no storage of its own to widen + // (its texels are the application's buffer object), so WidenImageFormatsPass declines + // every buffer image and the bind must decline with it, or the driver would be handed + // a carrier the shader never addressed. See the Dim::Buffer guard there for the + // 32-byte GL_RG32F measurement that pinned it. GLenum bindFormat = imageBinding.Format; - if (TextureImpl::GetImageBindableStorageWidening(imageBinding.Texture->GetFormat())) { + if (imageBinding.Texture->GetTarget() != TextureTarget::TextureBuffer && + TextureImpl::GetImageBindableStorageWidening(imageBinding.Texture->GetFormat())) { const auto boundFormatWidening = TextureImpl::GetImageBindableStorageWidening( MG_Util::ConvertGLEnumToTextureInternalFormat(imageBinding.Format)); if (boundFormatWidening) { diff --git a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp index e73c346d..efc85822 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp @@ -241,6 +241,18 @@ void main() { imageStore(img, ivec2(gl_FragCoord.xy), vec4(1.0, 2.0, 3.0, 4.0)); fragColor = texel; } +)"; + + // rg32f again, but as a BUFFER image. Same format, same carrier on paper - and it must be + // left alone anyway, because a buffer image's texels are the application's buffer object. + const char* const kRg32fBufferLoadStore = R"(#version 430 core +layout(rg32f, binding = 0) uniform imageBuffer img; +out vec4 fragColor; +void main() { + vec4 texel = imageLoad(img, int(gl_FragCoord.x)); + imageStore(img, int(gl_FragCoord.x), vec4(1.0, 2.0, 3.0, 4.0)); + fragColor = texel; +} )"; // rg16 is one of the EIGHT with no core carrier at all - core ESSL has no 16-bit normalized @@ -439,6 +451,39 @@ TEST(WidenImageFormats, PackedFloatImageOnlyReachesEsslThroughTheCarrier) { EXPECT_EQ(after.text.find("r11f_g11f_b10f"), String::npos) << after.text; } +// A BUFFER image is declined whatever its format, and the format alone cannot say so - rg32f is +// carried exactly when it is an image2D. What makes the difference is that widening REALLOCATES +// the texture behind the image in the carrier, and a buffer image has no texture storage to +// reallocate: its texels are the application's buffer object, usually also a vertex, index or +// storage buffer. Widening one leaves the shader striding 16 bytes through 8-byte texels - the +// measured symptom on an Adreno 830 was a 32-byte GL_RG32F buffer reading back +// [1,100] [0,1] [2,100] [0,1] instead of [1,100] [2,100] [3,100] [4,100], with the last two texels +// written past the end of the application's buffer. +TEST(WidenImageFormats, BufferImagesAreDeclinedEvenWhenTheirFormatHasACarrier) { + const Vector spirv = CompileFragment(kRg32fBufferLoadStore); + ASSERT_FALSE(spirv.empty()); + + const auto types = CollectStorageImageTypes(spirv); + ASSERT_EQ(types.size(), 1u); + EXPECT_EQ(types.front().format, static_cast(spv::ImageFormat::Rg32f)) + << "the fixture stopped declaring the format this test is about"; + + // The gate says no, so the optimizer is never even run for it... + EXPECT_FALSE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); + // ...and running it anyway changes nothing, which is what keeps the gate and the pass from + // disagreeing about a module. + Vector widened; + ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true); + EXPECT_TRUE(widened.empty() || widened == spirv) << "a buffer image was rewritten"; + + // The same format in a NON-buffer image still widens, or this test would pass for the wrong + // reason - a widening that had simply stopped working. + const Vector planar = CompileFragment(kRg32fLoadStore); + ASSERT_FALSE(planar.empty()); + EXPECT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(planar)); +} + TEST(WidenImageFormats, SingleChannelUnsignedImageBecomesRgba8uiWithBothAccessesMasked) { const Vector spirv = CompileFragment(kR8uiLoadStore); ASSERT_FALSE(spirv.empty()); diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp index 174896a8..eecdcbed 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp @@ -39,6 +39,7 @@ namespace MobileGL { // OpTypeImage in-operands: 0 sampled type, 1 Dim, 2 Depth, 3 Arrayed, 4 MS, // 5 Sampled, 6 Format. constexpr uint32_t kImageSampledTypeOperand = 0; + constexpr uint32_t kImageDimOperand = 1; constexpr uint32_t kImageSampledOperand = 5; constexpr uint32_t kImageFormatOperand = 6; // A storage image, i.e. one reached through imageLoad/imageStore rather than a @@ -246,6 +247,24 @@ namespace MobileGL { bool onlyFormatsSpirvCrossRefusesToPrint) { if (type == nullptr || type->opcode() != spv::Op::OpTypeImage) return false; if (type->GetSingleWordInOperand(kImageSampledOperand) != kSampledStorageImage) return false; + // A BUFFER image is never widened, whatever its format. Widening works because + // the ES texture behind the image can be REALLOCATED in the carrier, so the + // texel the shader addresses and the texel the storage holds stay the same + // size. A buffer image has no storage of its own to reallocate: its texels are + // the application's buffer object, at the size and layout the application gave + // it, and that buffer is usually also a vertex, index or storage buffer whose + // contents are not ours to relayout. + // + // Widening one anyway makes the shader stride 16 bytes through 8-byte texels. + // Measured on an Adreno 830 with a 32-byte GL_RG32F buffer and a shader storing + // (i+1, 100) at texel i: the readback came back [1,100] [0,1] [2,100] [0,1] - + // texels 0 and 1 landed on top of all four, texels 2 and 3 ran off the end of + // the application's buffer. Declining leaves the honest "no GLSL ES spelling" + // failure instead, which loses the same stage but corrupts nothing. + if (static_cast(type->GetSingleWordInOperand(kImageDimOperand)) == + spv::Dim::Buffer) { + return false; + } const auto format = static_cast(type->GetSingleWordInOperand(kImageFormatOperand)); if (!WideningOfSpirvImageFormat(format)) return false;