diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 3dfc63aa..cea0df7d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5181,12 +5181,23 @@ namespace MobileGL::MG_Backend::DirectGLES { } // Whether the ESSL chain will re-declare an image of this format in a core carrier - // and mask its accesses (WidenImageFormatsForEssl). Armed only where there is no - // GL_NV_image_formats to spell the format natively - a driver that has the extension - // keeps the declaration and gets the `#extension` directive instead. + // and mask its accesses (WidenImageFormatsForEssl). The same rule + // TextureImpl::GetImageBindableStorageWidening applies to the storage and the bind - + // the three layers move together or the shader addresses a texel size the storage + // does not have. + // + // Without GL_NV_image_formats there is no legal spelling for any non-core format, so + // everything carriable widens. WITH the extension only the formats SPIRV-Cross + // refuses to print do: it throws for its is_desktop_only_format set instead of + // emitting a token, and the throw loses the stage however willing the driver was. Bool ImageFormatWillBeWidened(Uint glInternalFormat) { - return !g_GLESCapabilities.SupportsExtendedImageFormats && glInternalFormat != 0 && - MG_Util::ShaderTranspiler::ShaderCompiler::WidenedCoreEsslImageFormat(glInternalFormat) != 0; + if (glInternalFormat == 0) return false; + if (MG_Util::ShaderTranspiler::ShaderCompiler::WidenedCoreEsslImageFormat(glInternalFormat) == 0) { + return false; + } + return !g_GLESCapabilities.SupportsExtendedImageFormats || + !MG_Util::ShaderTranspiler::ShaderCompiler::SpirvCrossCanPrintEsslImageFormat( + glInternalFormat); } } // namespace @@ -5229,7 +5240,9 @@ namespace MobileGL::MG_Backend::DirectGLES { // 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))) { + if (ImageFormatWillBeWidened(GLInternalFormatOfLayoutFormat(declaredFormat))) { + inputs.declaresWidenableImageFormat = true; + } else { inputs.needsExtendedImageFormats = true; if (!g_GLESCapabilities.SupportsExtendedImageFormats) { // From the OWNED TypeFacts, not from a live TType: the reflection @@ -5277,6 +5290,9 @@ namespace MobileGL::MG_Backend::DirectGLES { name, MG_Util::ShaderTranspiler::ShaderCompiler::EsslImageFormatSpelling(boundFormat)); continue; } + // The bake writes this format INTO the module, so the widening that runs + // straight after has to be armed for it even though nothing DECLARED it. + inputs.declaresWidenableImageFormat = true; } else { inputs.needsExtendedImageFormats = true; } @@ -5413,11 +5429,8 @@ namespace MobileGL::MG_Backend::DirectGLES { // Declares* probes each cost a BuildModule per stage, and on a driver where both // gates are armed (Mali: no GL_OES_viewport_array AND integer multisample // squeezed to 1) the doubled parse made compile-heavy workloads ~10% slower. - // Probing the pre-lowering module is sound for all three gates: demoting - // gl_ViewportIndex neither adds nor removes multisampled image types, and no pass - // between here and the widening changes which image FORMATS the module declares - - // Lower1DArrayImagesPass rebuilds an image type but copies its format operand across, - // and the one pass that can introduce a format (the bake) reports for itself below. + // Probing the pre-lowering module is sound for both gates: demoting + // gl_ViewportIndex neither adds nor removes multisampled image types. // Recomputed here rather than calling GL_Getter's GetAdvertisedMaxSamples(): // this is backend code and must not reach into the GL frontend. 4 is that // translation unit's kFrontendMaxSamples, which is the source of truth - @@ -5429,13 +5442,15 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESCapabilities.MaxColorTextureSamples < advertisedMaxSamples || g_GLESCapabilities.MaxIntegerSamples < advertisedMaxSamples || g_GLESCapabilities.MaxDepthTextureSamples < advertisedMaxSamples; - // The image-format widening is armed on EVERY real device (no driver tested advertises - // GL_NV_image_formats), so its probe has to ride the shared parse rather than add one: - // it is asked of every stage of every program, and a BuildModule per stage per gate is - // exactly what cost compile-heavy CTS cases ~10% before this struct existed. - const Bool imageFormatWideningArmed = !g_GLESCapabilities.SupportsExtendedImageFormats; + // The image-format widening is armed on EVERY driver, so its probe has to ride the + // shared parse rather than add one: it is asked of every stage of every program, and + // a BuildModule per stage per gate is exactly what cost compile-heavy CTS cases ~10% + // before this struct existed. What differs per driver is only HOW MUCH it widens - + // everything carriable where there is no GL_NV_image_formats to spell the narrow + // format, and only the formats SPIRV-Cross refuses to print where there is. + const Bool widenOnlyUnprintableImageFormats = g_GLESCapabilities.SupportsExtendedImageFormats; MG_Util::ShaderTranspiler::ShaderCompiler::SpirvGateFeatures spirvGates; - if (viewportLoweringArmed || sampleClampArmed || imageFormatWideningArmed) { + if (viewportLoweringArmed || sampleClampArmed) { spirvGates = MG_Util::ShaderTranspiler::ShaderCompiler::ProbeSpirvGateFeatures( *effectiveSpirv); } @@ -5615,7 +5630,6 @@ namespace MobileGL::MG_Backend::DirectGLES { // whose images all declare formats, and the cheap probe keeps a program that has // an unbound format-less image from paying an optimizer round trip per stage. Vector imageFormatSpirv; - Bool bakedAWidenableImageFormat = false; if (!imageFormatBake.glFormatByUniformName.empty() && MG_Util::ShaderTranspiler::ShaderCompiler::DeclaresFormatlessStorageImage(*effectiveSpirv) && MG_Util::ShaderTranspiler::ShaderCompiler::BakeImageFormatsForEssl( @@ -5623,19 +5637,6 @@ namespace MobileGL::MG_Backend::DirectGLES { enableSpirvValidation) && !imageFormatSpirv.empty()) { effectiveSpirv = &imageFormatSpirv; - // The bake can put a format into the module that was not there when the gate - // probe above parsed it, so the probe's verdict is stale for exactly this case. - // Answered from the bake's own map rather than by re-parsing: an entry the map - // does not carry cannot have been baked. Conservative in the harmless direction - - // the pass declines individual uniforms the map names, and a widening run that - // finds nothing to widen only costs a re-serialisation of a module that already - // went through the optimizer one line above. - for (const auto& entry : imageFormatBake.glFormatByUniformName) { - if (MG_Util::ShaderTranspiler::ShaderCompiler::WidenedCoreEsslImageFormat(entry.second) != 0) { - bakedAWidenableImageFormat = true; - break; - } - } } // GL has forty image formats and GLSL ES core has thirteen; the other twenty-seven @@ -5650,23 +5651,34 @@ namespace MobileGL::MG_Backend::DirectGLES { // have no exact carrier and keep the honest diagnostic // CollectImageFormatBakeInputs emits. // + // A driver that HAS GL_NV_image_formats still needs part of this. SPIRV-Cross throws + // for its is_desktop_only_format set when it targets ESSL rather than printing a + // token, and the throw loses the stage however willing the driver was - Mesa + // advertises the extension and `layout(r8ui) uimage2D` lost its whole program there + // until the widening ran for it too. So the driver bit decides HOW MUCH is widened, + // never WHETHER. + // // AFTER the bake above, deliberately: a format-less image whose unit holds a non-core // format is baked with that format and widened here, so both routes end in the same // place and there is no second widening rule for baked declarations. // - // KEY MATERIAL: g_GLESCapabilities.SupportsExtendedImageFormats, which ARMS this - - // see EsslTranslationKeyInputs::supportsExtendedImageFormats. The pass takes no other - // input: what it rewrites is a pure function of the module's own declared formats, - // and the module is already the largest thing in the L2 key. + // KEY MATERIAL: g_GLESCapabilities.SupportsExtendedImageFormats, which selects the + // mode - see EsslTranslationKeyInputs::supportsExtendedImageFormats. The pass takes + // no other input: what it rewrites is a pure function of the module's own declared + // formats and that mode, and the module is already the largest thing in the L2 key. + // The ARMING flag is deliberately NOT key material: it only decides whether the pass + // runs, and the module below is adopted only when the pass actually changed the bytes + // - so a program-wide flag that over-arms a stage costs an optimizer round trip and + // changes no output. // // DirectVulkan is deliberately not given this: it takes the declared format natively // and resolves the descriptor's view format from the same bind state. Vector widenedImageFormatSpirv; - if (imageFormatWideningArmed && - (spirvGates.DeclaresWidenableImageFormat || bakedAWidenableImageFormat) && + if (imageFormatBake.declaresWidenableImageFormat && MG_Util::ShaderTranspiler::ShaderCompiler::WidenImageFormatsForEssl( - *effectiveSpirv, widenedImageFormatSpirv, enableSpirvValidation) && - !widenedImageFormatSpirv.empty()) { + *effectiveSpirv, widenedImageFormatSpirv, widenOnlyUnprintableImageFormats, + enableSpirvValidation) && + !widenedImageFormatSpirv.empty() && widenedImageFormatSpirv != *effectiveSpirv) { effectiveSpirv = &widenedImageFormatSpirv; } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index a289a242..a0b54dd2 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1398,6 +1398,14 @@ namespace MobileGL::MG_Backend::DirectGLES { // Some format in play - declared or baked - is outside the GLSL ES core image // format set, so the emitted ESSL needs the GL_NV_image_formats directive. Bool needsExtendedImageFormats = false; + // Some DECLARED format in play is one WidenImageFormatsForEssl will re-declare in a + // core carrier. Answered from the uniform reflection rather than from a module parse + // on purpose: the widening is armed on every driver, so a per-stage BuildModule to + // find out would land on every stage of every program - which is the cost + // SpirvGateFeatures exists to avoid. Program-wide, so it can over-arm a stage that + // declares no image; the pass then finds nothing, reports no change, and the caller + // keeps the module it already had. + Bool declaresWidenableImageFormat = false; }; ImageFormatBakeInputs CollectImageFormatBakeInputs( const MG_State::GLState::ProgramObject& stateProgramObject); diff --git a/MobileGL/MG_Backend/DirectGLES/Utils.cpp b/MobileGL/MG_Backend/DirectGLES/Utils.cpp index 1f5a53ef..be295362 100644 --- a/MobileGL/MG_Backend/DirectGLES/Utils.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Utils.cpp @@ -236,18 +236,26 @@ namespace MobileGL::MG_Backend::DirectGLES { } ImageBindableStorageWidening GetImageBindableStorageWidening(TextureInternalFormat internalFormat) { - // Same arming as WidenImageFormatsForEssl. A driver with GL_NV_image_formats spells - // the narrow format natively, and widening the storage behind a shader that still - // says `rg32f` would make the two disagree about the texel size. - if (g_GLESCapabilities.SupportsExtendedImageFormats) { - return {}; - } const GLenum requested = MG_Util::ConvertTextureInternalFormatToGLEnum(internalFormat); const auto carrier = static_cast( MG_Util::ShaderTranspiler::ShaderCompiler::WidenedCoreEsslImageFormat(requested)); if (carrier == 0) { return {}; } + // EXACTLY the arming WidenImageFormatsForEssl uses, and it has to be: the shader, the + // storage and the bind must all widen or none of them may, or the shader addresses a + // texel size the storage does not have (which every driver tested accepts silently, + // reading and writing out of bounds). + // + // A driver WITH GL_NV_image_formats can spell the narrow format - but only for the + // formats SPIRV-Cross will actually print. It throws for its is_desktop_only_format + // set instead of emitting a token, and the throw loses the stage whatever the driver + // would have accepted: on Mesa, which advertises the extension, `layout(r8ui) + // uimage2D` still lost its whole program until the widening ran for it too. + if (g_GLESCapabilities.SupportsExtendedImageFormats && + MG_Util::ShaderTranspiler::ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(requested)) { + return {}; + } ImageBindableStorageWidening widening; widening.InternalFormat = carrier; widening.SourceChannels = diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index eb91e470..4a680bd0 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -3762,7 +3762,7 @@ void main() { imageStore(uni_image, ivec2(0), uvec4(15u)); } ASSERT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(baked)); Vector widened; - ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(baked, widened, true)); + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(baked, widened, false, true)); ASSERT_FALSE(widened.empty()); const String essl = DecompileToEssl(widened); ASSERT_FALSE(essl.empty()); diff --git a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp index 29e143cc..8f94f164 100644 --- a/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp +++ b/MobileGL/MG_Test/ShaderTranspiler/WidenImageFormatsTest.cpp @@ -319,7 +319,8 @@ TEST(WidenImageFormats, TwoChannelFloatImageBecomesRgba32fWithBothAccessesMasked ASSERT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); Vector widened; - ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*enableSpirvValidation=*/true)); + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true)); ASSERT_FALSE(widened.empty()); EXPECT_TRUE(Validates(widened)); // ...and there is nothing left for a second run to do. @@ -362,7 +363,8 @@ TEST(WidenImageFormats, SingleChannelUnsignedImageBecomesRgba8uiWithBothAccesses ASSERT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); Vector widened; - ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*enableSpirvValidation=*/true)); + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true)); ASSERT_FALSE(widened.empty()); EXPECT_TRUE(Validates(widened)); @@ -406,7 +408,7 @@ TEST(WidenImageFormats, WidenedModulesEmitEsslNamingTheCoreCarrier) { << before.text; Vector widened; - ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, true)); + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, false, true)); const EsslAttempt after = EmitEssl(widened); ASSERT_TRUE(after.succeeded) << after.error; EXPECT_NE(after.text.find("rgba8ui"), String::npos) << after.text; @@ -425,7 +427,7 @@ TEST(WidenImageFormats, WidenedModulesEmitEsslNamingTheCoreCarrier) { EXPECT_NE(before.text.find("rg32f"), String::npos) << before.text; Vector widened; - ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, true)); + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, false, true)); const EsslAttempt after = EmitEssl(widened); ASSERT_TRUE(after.succeeded) << after.error; EXPECT_NE(after.text.find("rgba32f"), String::npos) << after.text; @@ -435,6 +437,55 @@ TEST(WidenImageFormats, WidenedModulesEmitEsslNamingTheCoreCarrier) { } } +// The narrow mode, for a driver that HAS GL_NV_image_formats - Mesa, which every software lane +// runs on. There the driver can spell rg32f, so widening it would spend two to four times the +// texture memory to change nothing; but SPIRV-Cross STILL throws for r8ui rather than printing it, +// and the throw loses the stage whatever the driver would have accepted. So the extension narrows +// the emulation to its is_desktop_only_format set rather than switching it off. +TEST(WidenImageFormats, TheExtensionNarrowsTheWideningToWhatSpirvCrossWillNotPrint) { + ASSERT_TRUE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(0x8230 /*GL_RG32F*/)); + ASSERT_FALSE(ShaderCompiler::SpirvCrossCanPrintEsslImageFormat(0x8232 /*GL_R8UI*/)); + + { // rg32f: printable, so the narrow mode leaves it exactly as declared. + const Vector spirv = CompileFragment(kRg32fLoadStore); + ASSERT_FALSE(spirv.empty()); + EXPECT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(spirv, false)); + EXPECT_FALSE(ShaderCompiler::DeclaresWidenableImageFormat(spirv, true)); + + Vector widened; + ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, + /*onlyFormatsSpirvCrossRefusesToPrint=*/true, true); + if (!widened.empty()) { + const auto types = CollectStorageImageTypes(widened); + ASSERT_EQ(types.size(), 1u); + EXPECT_EQ(types.front().format, static_cast(spv::ImageFormat::Rg32f)); + EXPECT_EQ(CollectVectorShuffles(widened).size(), CollectVectorShuffles(spirv).size()); + } + } + { // r8ui: unprintable, so the narrow mode still carries it - and must mask it exactly as + // the wide mode does, because the storage and the bind widen with it either way. + const Vector spirv = CompileFragment(kR8uiLoadStore); + ASSERT_FALSE(spirv.empty()); + EXPECT_TRUE(ShaderCompiler::DeclaresWidenableImageFormat(spirv, true)); + + Vector widened; + ASSERT_TRUE(ShaderCompiler::WidenImageFormatsForEssl( + spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/true, true)); + ASSERT_FALSE(widened.empty()); + EXPECT_TRUE(Validates(widened)); + const auto types = CollectStorageImageTypes(widened); + ASSERT_EQ(types.size(), 1u); + EXPECT_EQ(types.front().format, static_cast(spv::ImageFormat::Rgba8ui)); + + 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); + EXPECT_TRUE(HasComponents(*storeMask, {0u, 5u, 6u, 7u})); + } +} + TEST(WidenImageFormats, CoreFormatModuleIsHandedBackUntouched) { const Vector spirv = CompileFragment(kCoreFormatLoadStore); ASSERT_FALSE(spirv.empty()); @@ -442,7 +493,8 @@ TEST(WidenImageFormats, CoreFormatModuleIsHandedBackUntouched) { EXPECT_FALSE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); Vector widened; - ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*enableSpirvValidation=*/true); + ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true); if (!widened.empty()) { EXPECT_EQ(CollectVectorShuffles(widened).size(), CollectVectorShuffles(spirv).size()) << "a core-format module must gain no masks"; @@ -466,7 +518,8 @@ TEST(WidenImageFormats, FormatWithoutAnExactCarrierIsLeftAlone) { EXPECT_FALSE(ShaderCompiler::DeclaresWidenableImageFormat(spirv)); Vector widened; - ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*enableSpirvValidation=*/true); + ShaderCompiler::WidenImageFormatsForEssl(spirv, widened, /*onlyFormatsSpirvCrossRefusesToPrint=*/false, + /*enableSpirvValidation=*/true); if (!widened.empty()) { const auto afterTypes = CollectStorageImageTypes(widened); ASSERT_EQ(afterTypes.size(), 1u); diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp index 550bbc62..bd15b5aa 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp @@ -728,8 +728,6 @@ namespace MobileGL { LowerViewportIndexPass::DeclaresViewportIndexBuiltin(context.get()); features.DeclaresMultisampledImage = ClampMultisampleFetchPass::DeclaresMultisampledImage(context.get()); - features.DeclaresWidenableImageFormat = - WidenImageFormatsPass::DeclaresWidenableImageFormat(context.get()); return features; } @@ -776,10 +774,12 @@ namespace MobileGL { bool ShaderCompiler::WidenImageFormatsForEssl(const Vector& inputBinary, Vector& outputBinary, + const bool onlyFormatsSpirvCrossRefusesToPrint, const bool enableSpirvValidation) { using namespace spvtools; Optimizer optimizer(SPV_ENV_VULKAN_1_1); - optimizer.RegisterPass(WidenImageFormatsPass::CreateWidenImageFormatsPass()); + optimizer.RegisterPass( + WidenImageFormatsPass::CreateWidenImageFormatsPass(onlyFormatsSpirvCrossRefusesToPrint)); // Two image types that differed only in a format the widening collapses - // `layout(rg32f)` and `layout(rgba32f)` in one module - are one type afterwards, // and duplicate non-aggregate type declarations are invalid SPIR-V. This joins @@ -791,8 +791,10 @@ namespace MobileGL { true, enableSpirvValidation); } - bool ShaderCompiler::DeclaresWidenableImageFormat(const Vector& binary) { - return WidenImageFormatsPass::DeclaresWidenableImageFormat(binary); + bool ShaderCompiler::DeclaresWidenableImageFormat(const Vector& binary, + const bool onlyFormatsSpirvCrossRefusesToPrint) { + return WidenImageFormatsPass::DeclaresWidenableImageFormat(binary, + onlyFormatsSpirvCrossRefusesToPrint); } Uint ShaderCompiler::WidenedCoreEsslImageFormat(Uint glInternalFormat) { diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h index 99497620..5f844d6f 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.h @@ -71,10 +71,14 @@ namespace MobileGL { // GL_OES_viewport_array AND integer multisample squeezed to 1) the separate // probes made compile-heavy workloads measurably slower - ReservedNames-class // CTS cases paid ~10%. Callers with more than one armed gate use this instead. + // The image-format widening deliberately does NOT ride this probe, even though it + // is a module question of exactly the same shape. It is armed on every driver, so + // a gate answered from the module would put a BuildModule on every stage of every + // program - and the frontend's uniform reflection can answer it for free + // (PrgramImpl::ImageFormatBakeInputs::declaresWidenableImageFormat). struct SpirvGateFeatures { Bool WritesViewportIndexOutput = false; Bool DeclaresMultisampledImage = false; - Bool DeclaresWidenableImageFormat = false; }; static SpirvGateFeatures ProbeSpirvGateFeatures(const Vector& binary); // Replaces an ARRAY vertex input with one input per element at consecutive @@ -241,15 +245,21 @@ namespace MobileGL { // declared format natively. See WidenImageFormatsPass for the table, for the nine // formats it deliberately does NOT widen, and for why the texture storage and the // glBindImageTexture argument have to move with it. + // `onlyFormatsSpirvCrossRefusesToPrint` narrows it to the formats that have no + // ESSL route even WITH GL_NV_image_formats, because SPIRV-Cross throws for them + // rather than printing a token - which is the whole set a driver that advertises + // the extension still needs. See WidenImageFormatsPass. static bool WidenImageFormatsForEssl(const Vector& inputBinary, Vector& outputBinary, + bool onlyFormatsSpirvCrossRefusesToPrint = false, bool enableSpirvValidation = false); // Whether the module declares a storage image WidenImageFormatsForEssl would - // widen, so the ~every shader that declares none pays no optimizer run. Costs its - // own module parse: the transpile path asks the same question through - // ProbeSpirvGateFeatures instead, because this gate is armed on every real driver - // and would otherwise put a BuildModule on every stage of every program. - static bool DeclaresWidenableImageFormat(const Vector& binary); + // widen, under the same mode the run would use. Costs its own module parse, so + // the transpile path does NOT gate on this - it answers the question from the + // frontend's uniform reflection instead, for the reason on SpirvGateFeatures. + // Here for tests and for callers that already hold nothing but the binary. + static bool DeclaresWidenableImageFormat(const Vector& binary, + bool onlyFormatsSpirvCrossRefusesToPrint = false); // The core-ESSL GL internal format that carries `glInternalFormat` exactly, or 0 // when it needs no widening or cannot be widened exactly. The single source of // truth for all three layers of the emulation: this one answers the shader, and diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp index 9a47b32a..2b074fd5 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.cpp @@ -8,6 +8,10 @@ #include "WidenImageFormatsPass.h" +// For IsSpirvCrossEsslPrintableFormat: the two passes share one question about the emitter, and +// the answer belongs where the rest of the image-format tables already are. +#include "BakeImageFormatsPass.h" + #include "spirv.hpp" #include "source/opt/build_module.h" #include "source/opt/constants.h" @@ -211,12 +215,20 @@ namespace MobileGL { } } - Bool IsWidenableStorageImageType(const Instruction* type) { + Bool IsWidenableStorageImageType(const Instruction* type, + bool onlyFormatsSpirvCrossRefusesToPrint) { if (type == nullptr || type->opcode() != spv::Op::OpTypeImage) return false; if (type->GetSingleWordInOperand(kImageSampledOperand) != kSampledStorageImage) return false; const auto format = static_cast(type->GetSingleWordInOperand(kImageFormatOperand)); - return static_cast(WideningOfSpirvImageFormat(format)); + if (!WideningOfSpirvImageFormat(format)) return false; + if (onlyFormatsSpirvCrossRefusesToPrint && + BakeImageFormatsPass::IsSpirvCrossEsslPrintableFormat(static_cast(format))) { + // The driver can spell this one and the emitter will print it; widening it + // would spend two to four times the texture memory to change nothing. + return false; + } + return true; } } // namespace @@ -231,23 +243,25 @@ namespace MobileGL { return ChannelsOfSpirvImageFormat(SpirvImageFormatOfGL(glInternalFormat)); } - bool WidenImageFormatsPass::DeclaresWidenableImageFormat(IRContext* context) { + bool WidenImageFormatsPass::DeclaresWidenableImageFormat( + IRContext* context, const bool onlyFormatsSpirvCrossRefusesToPrint) { if (context == nullptr) { return false; } for (const Instruction& type : context->module()->types_values()) { - if (IsWidenableStorageImageType(&type)) { + if (IsWidenableStorageImageType(&type, onlyFormatsSpirvCrossRefusesToPrint)) { return true; } } return false; } - bool WidenImageFormatsPass::DeclaresWidenableImageFormat(const Vector& binary) { + bool WidenImageFormatsPass::DeclaresWidenableImageFormat( + const Vector& binary, const bool onlyFormatsSpirvCrossRefusesToPrint) { std::unique_ptr context = spvtools::BuildModule( SPV_ENV_VULKAN_1_1, [](spv_message_level_t, const char*, const spv_position_t&, const char*) {}, binary.data(), binary.size()); - return DeclaresWidenableImageFormat(context.get()); + return DeclaresWidenableImageFormat(context.get(), onlyFormatsSpirvCrossRefusesToPrint); } spvtools::opt::Pass::Status WidenImageFormatsPass::Process() { @@ -258,7 +272,7 @@ namespace MobileGL { // byte-identical - which is every shader but a handful. std::vector imageTypes; for (Instruction& type : irContext->types_values()) { - if (IsWidenableStorageImageType(&type)) { + if (IsWidenableStorageImageType(&type, m_onlyFormatsSpirvCrossRefusesToPrint)) { imageTypes.push_back(&type); } } @@ -539,8 +553,10 @@ namespace MobileGL { return Status::SuccessWithChange; } - spvtools::Optimizer::PassToken WidenImageFormatsPass::CreateWidenImageFormatsPass() { - return spvtools::Optimizer::PassToken(spvtools::MakeUnique()); + spvtools::Optimizer::PassToken WidenImageFormatsPass::CreateWidenImageFormatsPass( + const bool onlyFormatsSpirvCrossRefusesToPrint) { + return spvtools::Optimizer::PassToken( + spvtools::MakeUnique(onlyFormatsSpirvCrossRefusesToPrint)); } } // namespace ShaderTranspiler } // namespace MG_Util diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h index b9d13013..f5d7be0a 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/WidenImageFormatsPass.h @@ -74,19 +74,39 @@ namespace MobileGL { // format from the same bind state, so the module must reach it unchanged. class WidenImageFormatsPass final : public spvtools::opt::Pass { public: + // `onlyFormatsSpirvCrossRefusesToPrint` narrows the pass to the formats that have + // no ESSL route even on a driver that DOES advertise GL_NV_image_formats. + // SPIRV-Cross's is_desktop_only_format set - r8ui, rg16f, r16i and fifteen others - + // makes it THROW for an ESSL target rather than print a token, and the throw takes + // the stage with it whatever the driver could have accepted. Mesa is exactly that + // case: it advertises the extension, so nothing else needs widening there, and + // `layout(r8ui) uimage2D` still lost its whole program until this ran for it. + // + // Off, the pass widens every format in the table, which is what a driver without + // the extension needs. The caller sets it from + // g_GLESCapabilities.SupportsExtendedImageFormats, and the SAME rule decides + // whether the ES texture storage and the glBindImageTexture argument widen + // (TextureImpl::GetImageBindableStorageWidening) - all three have to agree or the + // shader addresses a texel size the storage does not have. + explicit WidenImageFormatsPass(bool onlyFormatsSpirvCrossRefusesToPrint = false) + : m_onlyFormatsSpirvCrossRefusesToPrint(onlyFormatsSpirvCrossRefusesToPrint) {} + const char* name() const override { return "mobilegl-widen-image-formats"; } Status Process() override; // Whether the module declares a storage image whose format this pass would widen, // i.e. whether running it could change anything. Answered from a single parse so // the caller can skip the optimizer run entirely - which is every shader but a - // handful. - static bool DeclaresWidenableImageFormat(const Vector& binary); + // handful. `onlyFormatsSpirvCrossRefusesToPrint` must match what the run will use, + // or the gate answers a question the pass is not being asked. + static bool DeclaresWidenableImageFormat(const Vector& binary, + bool onlyFormatsSpirvCrossRefusesToPrint = false); // The same question asked of a module the caller has ALREADY parsed, so a stage // that has to answer several gate questions pays one BuildModule rather than one // per gate - see ShaderCompiler::ProbeSpirvGateFeatures, and the ~10% it cost // compile-heavy CTS cases when two gates each parsed for themselves. - static bool DeclaresWidenableImageFormat(spvtools::opt::IRContext* context); + static bool DeclaresWidenableImageFormat(spvtools::opt::IRContext* context, + bool onlyFormatsSpirvCrossRefusesToPrint = false); // The core-ESSL GL internal format that carries `glInternalFormat` exactly, or 0 // when the format needs no widening (it is core already) or cannot be widened @@ -99,7 +119,11 @@ namespace MobileGL { // forty image formats. The count the widened accesses are masked back to. static Uint ImageFormatChannelCount(Uint glInternalFormat); - static spvtools::Optimizer::PassToken CreateWidenImageFormatsPass(); + static spvtools::Optimizer::PassToken CreateWidenImageFormatsPass( + bool onlyFormatsSpirvCrossRefusesToPrint = false); + + private: + bool m_onlyFormatsSpirvCrossRefusesToPrint = false; }; } // namespace ShaderTranspiler } // namespace MG_Util