swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-07 19:58:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (DirectGLES): run the interface-block location strip last, where its deliberately Vulkan-invalid module reaches no validator

Browse Source
This commit is contained in:
swung0x48
2026-08-27 19:44:18 -04:00
parent 23565fcacd
commit 0d0ef13619
3 changed files with 57 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/Managers.cpp
+39 -29
View File
@@ -6488,35 +6488,6 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
}
// The second half of the inter-stage interface-block repair, and the one that
// actually closes the 420pack group: this driver drops the payload of a block that
// carries an explicit layout(location=) whenever a tessellation or geometry stage
// is in the pipeline, so the qualifier comes off and ES matches the block by name
// and member sequence instead. The names those two sides agree on are the ones the
// rename above just fixed, which is why this runs AFTER it and not before.
//
// The caller arms the two directions; both are false unless the driver POST
// measured the defect AND this program has a stage that can hit it. Adopted only
// when this stage really had a located block, for the reason the array-input split
// documents: the optimizer hands back a re-serialised copy either way.
Vector<unsigned int> strippedIoBlockLocationSpirv;
if (stripInputBlockLocations || stripOutputBlockLocations) {
Bool strippedAny = false;
if (MG_Util::ShaderTranspiler::ShaderCompiler::StripIoBlockLocationsForEssl(
*effectiveSpirv, stripInputBlockLocations, stripOutputBlockLocations,
strippedAny, strippedIoBlockLocationSpirv, enableSpirvValidation) &&
!strippedIoBlockLocationSpirv.empty() && strippedAny) {
effectiveSpirv = &strippedIoBlockLocationSpirv;
MGLOG_D("Program %u stage %s: interface-block location qualifiers dropped "
"(%s), because this driver loses a located block's payload across a "
"tessellation or geometry boundary.",
m_backendProgramId, MG_Util::ConvertGLEnumToString(glShaderType).c_str(),
stripInputBlockLocations
? (stripOutputBlockLocations ? "consumed and produced" : "consumed")
: "produced");
}
}
// 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
@@ -6722,6 +6693,45 @@ namespace MobileGL::MG_Backend::DirectGLES {
effectiveSpirv = &atomicCounterSpirv;
}
// The second half of the inter-stage interface-block repair, and the one that
// actually closes the 420pack group: this driver drops the payload of a block that
// carries an explicit layout(location=) whenever a tessellation or geometry stage
// is in the pipeline, so the qualifier comes off and ES matches the block by name
// and member sequence instead. The names those two sides agree on are the ones the
// rename above just fixed, which is why this runs AFTER it and not before.
//
// The caller arms the two directions; both are false unless the driver POST
// measured the defect AND this program has a stage that can hit it. Adopted only
// when this stage really had a located block, for the reason the array-input split
// documents: the optimizer hands back a re-serialised copy either way.
//
// LAST IN THE CHAIN, and that position is load-bearing. Vulkan SPIR-V REQUIRES a
// Location on every user-defined Input/Output variable
// ([VUID-StandaloneSpirv-Location-04915]), so the module this produces is
// deliberately no longer valid Vulkan SPIR-V - it is an ESSL-emission intermediate
// that goes straight into SPIRV-Cross and reaches no driver as SPIR-V. Running it
// here means no later pass validates what it produced; the pass itself skips
// validation for the same reason (see StripIoBlockLocationsForEssl). Anywhere
// earlier and every remaining pass would latch a validation failure on a module
// that is doing exactly what it was asked to.
Vector<unsigned int> strippedIoBlockLocationSpirv;
if (stripInputBlockLocations || stripOutputBlockLocations) {
Bool strippedAny = false;
if (MG_Util::ShaderTranspiler::ShaderCompiler::StripIoBlockLocationsForEssl(
*effectiveSpirv, stripInputBlockLocations, stripOutputBlockLocations,
strippedAny, strippedIoBlockLocationSpirv, enableSpirvValidation) &&
!strippedIoBlockLocationSpirv.empty() && strippedAny) {
effectiveSpirv = &strippedIoBlockLocationSpirv;
MGLOG_D("Program %u stage %s: interface-block location qualifiers dropped "
"(%s), because this driver loses a located block's payload across a "
"tessellation or geometry boundary.",
m_backendProgramId, MG_Util::ConvertGLEnumToString(glShaderType).c_str(),
stripInputBlockLocations
? (stripOutputBlockLocations ? "consumed and produced" : "consumed")
: "produced");
}
}
MG_Util::ShaderTranspiler::SpvcSession spvcSession(*effectiveSpirv,
MG_Util::ShaderTranspiler::SessionUsageBit::Transpile);
MobileGL/MG_Test/ShaderTranspiler/StripIoBlockLocationsTest.cpp
+9 -16
View File
@@ -17,8 +17,6 @@
#include <MG_Util/ShaderTranspiler/SpvcSession.h>
#include <MG_Util/ShaderTranspiler/Types.h>
#include <spirv-tools/libspirv.hpp>
using namespace MobileGL;
using MobileGL::MG_Util::ShaderTranspiler::SessionUsageBit;
using MobileGL::MG_Util::ShaderTranspiler::ShaderCompiler;
@@ -100,19 +98,17 @@ void main()
)";
} // namespace
// NOTE ON spirv-val, because its absence here is deliberate and every sibling pass test
// asserts the opposite. Vulkan SPIR-V REQUIRES a Location decoration on every user-defined
// Input/Output variable ([VUID-StandaloneSpirv-Location-04915]), so a module whose interface
// blocks have had theirs removed is INVALID Vulkan SPIR-V by construction - that is what the
// pass was asked to produce. It never reaches a driver as SPIR-V: DirectGLES runs this last
// in its chain and hands the result straight to SPIRV-Cross, which needs no location to print
// a block. What the cases below assert instead is the thing that actually matters - that
// SPIRV-Cross still emits a complete, matchable interface from it.
class StripIoBlockLocationsTest : public ::testing::Test {
protected:
void SetUp() override {
MobileGL::Initialize();
m_validationFailuresAtStart = ShaderCompiler::SpirvValidationFailureCount();
}
void TearDown() override {
EXPECT_EQ(ShaderCompiler::SpirvValidationFailureCount(), m_validationFailuresAtStart)
<< "the stripped module did not survive spirv-val";
}
Uint64 m_validationFailuresAtStart = 0;
void SetUp() override { MobileGL::Initialize(); }
};
TEST_F(StripIoBlockLocationsTest, DropsTheQualifierFromBothBlocksAndLeavesVaryingsAlone) {
@@ -131,9 +127,6 @@ TEST_F(StripIoBlockLocationsTest, DropsTheQualifierFromBothBlocksAndLeavesVaryin
ASSERT_FALSE(output.empty());
EXPECT_TRUE(strippedAny);
spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1);
ASSERT_TRUE(tools.Validate(output));
const String after = Transpile(output);
// The blocks come out bare...
EXPECT_NE(after.find("in TCSOutputBlock"), String::npos) << after;
MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp
+9 -1
View File
@@ -1241,8 +1241,16 @@ namespace MobileGL {
optimizer.RegisterPass(StripIoBlockLocationsPass::CreateStripIoBlockLocationsPass(
stripInputBlocks, stripOutputBlocks, &strippedAny));
// NOT VALIDATED, and that is the point of the pass rather than an oversight.
// Vulkan SPIR-V requires a Location on every user-defined Input/Output variable
// ([VUID-StandaloneSpirv-Location-04915]), so a module whose interface blocks
// have deliberately lost theirs fails spirv-val by construction. It never
// reaches a driver as SPIR-V: the caller runs this last in the DirectGLES chain
// and hands the result straight to SPIRV-Cross, which needs no location to
// print a block. Validating here would latch a failure on every affected
// program and teach the counter to cry wolf.
return RunOptimizerChecked("StripIoBlockLocationsForEssl", optimizer, inputBinary,
outputBinary, true, enableSpirvValidation);
outputBinary, false, enableSpirvValidation);
}
bool ShaderCompiler::PackDoubleVertexInputsForVulkan(const Vector<Uint32>& inputBinary,