[Fix] (DirectGLES): piglit fixes batch 1 - keep enabled-but-unsourceable vertex attribs disabled on the backend VAO (Adreno memcpy-from-NULL SIGSEGV on gl-3.1-vao-broken-attrib), content-sync READ-framebuffer texture attachments before blits, clamp out-of-bounds access-chain indices via GraphicsRobustAccessPass before ESSL transpile (Adreno poisons whole-shader output on constant OOB), and fold ConstOffset into the coordinate for 1D texelFetch (SPIRV-Cross emulates 1D as 2D but leaves the scalar offset, which ESSL rejects)

2026-09-09 20:58:31 +09:00 · 2026-07-26 19:17:25 -04:00
parent 761114d022
commit 203d4bce5e
8 changed files with 282 additions and 2 deletions
@@ -194,6 +194,7 @@ set(SOURCE_FILES
    MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripUboMemberRelaxedPrecisionPass.cpp
    MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/StripNoPerspectivePass.cpp
    MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/EmulateNoPerspectivePass.cpp
+    MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/FoldConstOffsetFor1DFetchPass.cpp

    MobileGL/MG_Util/BackendLoaders/OpenGL/Loader.cpp
    MobileGL/MG_Util/BackendLoaders/Vulkan/Loader.cpp
@@ -449,6 +449,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
                    }
                }
            }
+
+            // Textures attached only to the READ framebuffer (blit / ReadPixels sources) need
+            // their content synced too, or the backend reads stale texel data.
+            const auto& readFBO =
+                MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Read).GetBoundObject();
+            if (readFBO && readFBO != currentFBO) {
+                for (const auto& attachment : readFBO->GetAllAttachmentObjects()) {
+                    if (!attachment.IsTexture()) continue;
+                    auto& textureObject = attachment.GetTexture();
+                    if (textureObject) {
+                        SyncTextureObjectToBackend(textureObject);
+                    }
+                }
+            }
        }

        static Bool SupportsLayeredImageBinding(TextureTarget target) {
@@ -1262,15 +1262,31 @@ namespace MobileGL::MG_Backend::DirectGLES {
            const auto& allAttributes = stateVAOObject->GetAllAttributes();
            for (Uint attribIndex = 0; attribIndex < allAttributes.size(); ++attribIndex) {
                const auto& attrib = allAttributes[attribIndex];
+                const Uint32 attribBit = 1u << attribIndex;
+
+                // An enabled attrib with neither a buffer object nor a client pointer has no
+                // source; GL tolerates the state (only draws consuming it are undefined), but
+                // Adreno's ES driver memcpys the "client array" from address 0 at draw time
+                // (SIGSEGV). Keep such attribs disabled on the backend VAO and re-enable them
+                // the moment they gain a source - the mask-vs-current compare below triggers
+                // the enable even when only the Buffer/Format versions changed.
+                const Bool unsourceable = attrib.Enabled && !attrib.Buffer && attrib.Offset == 0;
+                const Bool wasForceDisabled = (m_forceDisabledAttribsMask & attribBit) != 0;
+
                Bool needsSyncSwitch = allAttributeVersions[attribIndex].SwitchVersion !=
                                       m_syncedAttributeVersions[attribIndex].SwitchVersion;
-                if (needsSyncSwitch) {
-                    if (attrib.Enabled) {
+                if (needsSyncSwitch || unsourceable != wasForceDisabled) {
+                    if (attrib.Enabled && !unsourceable) {
                        g_GLESFuncs.glEnableVertexAttribArray(attribIndex);
                    } else {
                        g_GLESFuncs.glDisableVertexAttribArray(attribIndex);
                    }
                }
+                if (unsourceable) {
+                    m_forceDisabledAttribsMask |= attribBit;
+                } else {
+                    m_forceDisabledAttribsMask &= ~attribBit;
+                }

                Bool needsSyncFormat = allAttributeVersions[attribIndex].FormatVersion !=
                                       m_syncedAttributeVersions[attribIndex].FormatVersion;
@@ -1278,7 +1294,17 @@ namespace MobileGL::MG_Backend::DirectGLES {
                                       m_syncedAttributeVersions[attribIndex].BufferVersion;
                if (!needsSyncFormat && !needsSyncBuffer) continue;

+                if (unsourceable) continue;
+
+                // Client-side array with a non-null pointer: the pointer is uploaded and applied
+                // per draw by SyncClientSideAttributesForDrawArrays.
+                if (!attrib.Buffer) continue;
+
                if (!BindAttributeBuffer(attrib)) {
+                    if (attrib.Enabled) {
+                        g_GLESFuncs.glDisableVertexAttribArray(attribIndex);
+                        m_forceDisabledAttribsMask |= attribBit;
+                    }
                    continue;
                }

@@ -3287,6 +3313,32 @@ namespace MobileGL::MG_Backend::DirectGLES {
                    effectiveSpirv = &loweredSpirv;
                }

+                // GL 3.3 only promises undefined *values* for out-of-bounds array indexing, but
+                // Adreno's ESSL compiler constant-folds a provably out-of-bounds local-array
+                // index into poison that corrupts the whole shader's output. Clamp every
+                // access-chain index to its declared bounds before transpiling.
+                Vector<unsigned int> clampedSpirv;
+                if (MG_Util::ShaderTranspiler::ShaderCompiler::ClampAccessChainIndicesForEssl(*effectiveSpirv,
+                                                                                              clampedSpirv) &&
+                    !clampedSpirv.empty()) {
+                    effectiveSpirv = &clampedSpirv;
+                } else {
+                    MGLOG_W("ClampAccessChainIndicesForEssl failed, continuing with unclamped SPIR-V.");
+                }
+
+                // SPIRV-Cross emulates 1D samplers as 2D for ES: it widens texelFetch coordinates
+                // to ivec2 but keeps the ConstOffset operand scalar, which is not a valid ESSL
+                // texelFetchOffset overload (Adreno rejects it). Fold the constant offset into the
+                // coordinate instead (texelFetchOffset(t,P,l,o) == texelFetch(t,P+o,l)).
+                Vector<unsigned int> foldedOffsetSpirv;
+                if (MG_Util::ShaderTranspiler::ShaderCompiler::FoldConstOffsetFor1DFetchForEssl(
+                        *effectiveSpirv, foldedOffsetSpirv) &&
+                    !foldedOffsetSpirv.empty()) {
+                    effectiveSpirv = &foldedOffsetSpirv;
+                } else {
+                    MGLOG_W("FoldConstOffsetFor1DFetchForEssl failed, continuing with unfolded SPIR-V.");
+                }
+
                // ESSL stage-matches uniform blocks by member precision, but SPIRV-Cross prints
                // a RelaxedPrecision member as explicit "mediump" in the vertex stage and as
                // UNQUALIFIED (mediump-by-default) in the fragment stage; after
@@ -258,6 +258,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
        private:
            Uint m_backendVAOId = 0;
            Array<Uint, MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS> m_clientAttributeBufferIds;
+            // Attribs the frontend has Enabled but that have no source at all (no buffer object
+            // and NULL client pointer). GL keeps such attribs latently enabled, but Adreno's ES
+            // driver treats them as client arrays and memcpys from address 0 at draw time
+            // (SIGSEGV), so they are kept disabled on the backend VAO until they gain a source.
+            Uint32 m_forceDisabledAttribsMask = 0;
            Bool m_isInitialized = false;
            Uint16 m_syncedIndexBufferVersion = 0;
            Array<MG_State::GLState::VertexAttributeVersion, MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS>
@@ -22,6 +22,7 @@
 #include "SpirvPasses/StripUboMemberRelaxedPrecisionPass.h"
 #include "SpirvPasses/StripNoPerspectivePass.h"
 #include "SpirvPasses/EmulateNoPerspectivePass.h"
+#include "SpirvPasses/FoldConstOffsetFor1DFetchPass.h"
 #include "spirv-tools/libspirv.h"
 #include "spirv-tools/optimizer.hpp"

@@ -323,6 +324,30 @@ namespace MobileGL {
                return optimizer.Run(inputBinary.data(), inputBinary.size(), &outputBinary, options);
            }

+            bool ShaderCompiler::ClampAccessChainIndicesForEssl(const Vector<Uint32>& inputBinary,
+                                                                Vector<uint32_t>& outputBinary) {
+                using namespace spvtools;
+                OptimizerOptions options;
+                options.set_run_validator(false);
+
+                Optimizer optimizer(SPV_ENV_VULKAN_1_1);
+                optimizer.RegisterPass(CreateGraphicsRobustAccessPass());
+
+                return optimizer.Run(inputBinary.data(), inputBinary.size(), &outputBinary, options);
+            }
+
+            bool ShaderCompiler::FoldConstOffsetFor1DFetchForEssl(const Vector<Uint32>& inputBinary,
+                                                                  Vector<uint32_t>& outputBinary) {
+                using namespace spvtools;
+                OptimizerOptions options;
+                options.set_run_validator(false);
+
+                Optimizer optimizer(SPV_ENV_VULKAN_1_1);
+                optimizer.RegisterPass(FoldConstOffsetFor1DFetchPass::CreateFoldConstOffsetFor1DFetchPass());
+
+                return optimizer.Run(inputBinary.data(), inputBinary.size(), &outputBinary, options);
+            }
+
            bool ShaderCompiler::StripUboMemberRelaxedPrecisionForEssl(const Vector<Uint32>& inputBinary,
                                                                       Vector<uint32_t>& outputBinary) {
                using namespace spvtools;
@@ -27,6 +27,22 @@ namespace MobileGL {
                // Only for backends without native draw-parameter support (DirectGLES).
                static bool LowerDrawParametersForEssl(const Vector<Uint32>& inputBinary,
                                                       Vector<uint32_t>& outputBinary);
+                // Clamps every access-chain index to its declared bounds (spirv-tools
+                // GraphicsRobustAccessPass). GL 3.3 only promises undefined *values* for
+                // out-of-bounds indexing, but Adreno's ESSL compiler constant-folds a provably
+                // out-of-bounds local-array index into poison that corrupts the whole shader's
+                // output; clamping restores the "some value from the array" contract. Only for
+                // the DirectGLES transpile path.
+                static bool ClampAccessChainIndicesForEssl(const Vector<Uint32>& inputBinary,
+                                                           Vector<uint32_t>& outputBinary);
+                // Folds the ConstOffset image operand of Dim1D OpImageFetch into the integer
+                // coordinate (texelFetchOffset(t,P,l,o) == texelFetch(t,P+o,l)). SPIRV-Cross
+                // emulates 1D samplers as 2D for ES: it widens the coordinate to ivec2 but keeps
+                // the scalar offset, and ESSL has no texelFetchOffset(sampler2D, ivec2, int,
+                // scalar) overload, so Adreno rejects the shader. Only for the DirectGLES
+                // transpile path.
+                static bool FoldConstOffsetFor1DFetchForEssl(const Vector<Uint32>& inputBinary,
+                                                             Vector<uint32_t>& outputBinary);
                // Drops RelaxedPrecision member decorations from uniform-block structs so
                // SPIRV-Cross prints the same (highp) member precision in every stage; ES
                // drivers reject cross-stage uniform blocks whose member precisions differ.
@@ -0,0 +1,131 @@
+// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/FoldConstOffsetFor1DFetchPass.cpp
+// Copyright (c) 2025-2026 MobileGL-Dev
+// Licensed under the GNU Lesser General Public License v3.0:
+//   https://www.gnu.org/licenses/gpl-3.0.txt
+//   https://www.gnu.org/licenses/lgpl-3.0.txt
+// SPDX-License-Identifier: LGPL-3.0-only
+// End of Source File Header
+
+#include "FoldConstOffsetFor1DFetchPass.h"
+
+#include "spirv.hpp"
+#include "source/opt/def_use_manager.h"
+#include "source/opt/instruction.h"
+#include "source/opt/ir_builder.h"
+#include "source/opt/ir_context.h"
+#include "source/opt/module.h"
+#include "source/util/make_unique.h"
+
+#include <vector>
+
+namespace MobileGL {
+    namespace MG_Util {
+        namespace ShaderTranspiler {
+            namespace {
+                using spvtools::opt::Instruction;
+                using spvtools::opt::InstructionBuilder;
+                using spvtools::opt::IRContext;
+                using spvtools::opt::Operand;
+
+                // Number of ImageOperands ids that precede the ConstOffset id: one per
+                // lower-order bit set in the mask, except Grad which carries two ids.
+                uint32_t CountIdsBeforeConstOffset(uint32_t mask) {
+                    uint32_t count = 0;
+                    if (mask & static_cast<uint32_t>(spv::ImageOperandsMask::Bias)) count += 1;
+                    if (mask & static_cast<uint32_t>(spv::ImageOperandsMask::Lod)) count += 1;
+                    if (mask & static_cast<uint32_t>(spv::ImageOperandsMask::Grad)) count += 2;
+                    return count;
+                }
+            } // namespace
+
+            spvtools::opt::Pass::Status FoldConstOffsetFor1DFetchPass::Process() {
+                auto* irContext = context();
+                auto* defUseMgr = irContext->get_def_use_mgr();
+                Bool modified = false;
+
+                constexpr uint32_t kConstOffsetBit =
+                    static_cast<uint32_t>(spv::ImageOperandsMask::ConstOffset);
+
+                for (auto& function : *get_module()) {
+                    for (auto& block : function) {
+                        for (auto& inst : block) {
+                            if (inst.opcode() != spv::Op::OpImageFetch) continue;
+                            // In-operands: image, coordinate, [ImageOperands mask, ids...].
+                            if (inst.NumInOperands() < 3) continue;
+                            const uint32_t operandsMask = inst.GetSingleWordInOperand(2);
+                            if ((operandsMask & kConstOffsetBit) == 0) continue;
+
+                            Instruction* imageInst = defUseMgr->GetDef(inst.GetSingleWordInOperand(0));
+                            if (imageInst == nullptr) continue;
+                            Instruction* imageType = defUseMgr->GetDef(imageInst->type_id());
+                            if (imageType == nullptr || imageType->opcode() != spv::Op::OpTypeImage ||
+                                static_cast<spv::Dim>(imageType->GetSingleWordInOperand(1)) != spv::Dim::Dim1D) {
+                                continue;
+                            }
+
+                            const uint32_t offsetOperandIndex = 3 + CountIdsBeforeConstOffset(operandsMask);
+                            const uint32_t offsetId = inst.GetSingleWordInOperand(offsetOperandIndex);
+
+                            const uint32_t coordId = inst.GetSingleWordInOperand(1);
+                            Instruction* coordType = defUseMgr->GetDef(defUseMgr->GetDef(coordId)->type_id());
+
+                            InstructionBuilder builder(
+                                irContext, &inst,
+                                IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
+
+                            uint32_t newCoordId = 0;
+                            if (coordType->opcode() == spv::Op::OpTypeVector) {
+                                // Arrayed 1D fetch: component 0 is the texel coordinate,
+                                // component 1 the layer - only component 0 takes the offset.
+                                const uint32_t componentTypeId = coordType->GetSingleWordInOperand(0);
+                                Instruction* extracted = builder.AddCompositeExtract(componentTypeId, coordId, {0});
+                                Instruction* sum =
+                                    builder.AddIAdd(componentTypeId, extracted->result_id(), offsetId);
+                                Instruction* inserted = builder.AddInstruction(spvtools::MakeUnique<Instruction>(
+                                    irContext, spv::Op::OpCompositeInsert, coordType->result_id(),
+                                    irContext->TakeNextId(),
+                                    std::initializer_list<Operand>{
+                                        {SPV_OPERAND_TYPE_ID, {sum->result_id()}},
+                                        {SPV_OPERAND_TYPE_ID, {coordId}},
+                                        {SPV_OPERAND_TYPE_LITERAL_INTEGER, {0}}}));
+                                newCoordId = inserted->result_id();
+                            } else {
+                                Instruction* sum = builder.AddIAdd(coordType->result_id(), coordId, offsetId);
+                                newCoordId = sum->result_id();
+                            }
+
+                            const uint32_t newMask = operandsMask & ~kConstOffsetBit;
+                            // 3 fixed operands + the offset id: anything beyond that is another
+                            // image-operand id that must keep the mask word alive.
+                            const Bool otherOperandIdsRemain = inst.NumInOperands() > 4;
+
+                            irContext->ForgetUses(&inst);
+                            std::vector<Operand> newOperands;
+                            newOperands.push_back(inst.GetInOperand(0));
+                            newOperands.push_back({SPV_OPERAND_TYPE_ID, {newCoordId}});
+                            if (newMask != 0 || otherOperandIdsRemain) {
+                                Operand maskOperand = inst.GetInOperand(2);
+                                maskOperand.words[0] = newMask;
+                                newOperands.push_back(maskOperand);
+                                for (uint32_t i = 3; i < inst.NumInOperands(); ++i) {
+                                    if (i == offsetOperandIndex) continue;
+                                    newOperands.push_back(inst.GetInOperand(i));
+                                }
+                            }
+                            inst.SetInOperands(std::move(newOperands));
+                            irContext->AnalyzeUses(&inst);
+
+                            modified = true;
+                        }
+                    }
+                }
+
+                return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange;
+            }
+
+            spvtools::Optimizer::PassToken FoldConstOffsetFor1DFetchPass::CreateFoldConstOffsetFor1DFetchPass() {
+                return spvtools::Optimizer::PassToken(MakeUnique<FoldConstOffsetFor1DFetchPass>());
+            }
+        } // namespace ShaderTranspiler
+    } // namespace MG_Util
+} // namespace MobileGL
@@ -0,0 +1,36 @@
+// MobileGL - MobileGL/MG_Util/ShaderTranspiler/SpirvPasses/FoldConstOffsetFor1DFetchPass.h
+// Copyright (c) 2025-2026 MobileGL-Dev
+// Licensed under the GNU Lesser General Public License v3.0:
+//   https://www.gnu.org/licenses/gpl-3.0.txt
+//   https://www.gnu.org/licenses/lgpl-3.0.txt
+// SPDX-License-Identifier: LGPL-3.0-only
+// End of Source File Header
+
+#pragma once
+#include "source/opt/pass.h"
+#include "spirv-tools/optimizer.hpp"
+
+#include <Includes.h>
+
+namespace MobileGL {
+    namespace MG_Util {
+        namespace ShaderTranspiler {
+            // SPIRV-Cross emulates 1D textures as 2D for ES targets: it widens the texelFetch
+            // coordinate to ivec2 but keeps the ConstOffset image operand scalar, and ESSL has
+            // no texelFetchOffset(sampler2D, ivec2, int, scalar-offset) overload, so drivers
+            // (Adreno) reject the transpiled shader. This pass folds the constant offset into
+            // the integer coordinate before the fetch - texelFetchOffset(t, P, l, o) ==
+            // texelFetch(t, P + o, l) per the GLSL spec - and drops the ConstOffset operand,
+            // so SPIRV-Cross emits a plain texelFetch. For arrayed 1D fetches only coordinate
+            // component 0 is offset (component 1 is the layer). Only meant for the DirectGLES
+            // transpile path.
+            class FoldConstOffsetFor1DFetchPass : public spvtools::opt::Pass {
+            public:
+                const char* name() const override { return "fold-const-offset-for-1d-fetch"; }
+                Status Process() override;
+
+                static spvtools::Optimizer::PassToken CreateFoldConstOffsetFor1DFetchPass();
+            };
+        } // namespace ShaderTranspiler
+    } // namespace MG_Util
+} // namespace MobileGL