diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index a78232e1..ef690599 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -5875,6 +5875,12 @@ namespace MobileGL::MG_Backend::DirectGLES { spvcSession.DropDefaultFragmentOutputColorIndex(); } + // `readonly writeonly` together says the buffer variable can only be asked its + // .length(), which the frontend has already enforced - so the pair is inert, and + // printing it is not. Mesa's ES compiler refuses a block spelled that way and the + // stage never reaches the program. + spvcSession.RelaxReadWriteExclusiveStorageBuffers(); + const char* result = nullptr; spvcSession.Compile(&result); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayLengthScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayLengthScenario.cpp index 9a1b6404..934891f0 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayLengthScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayLengthScenario.cpp @@ -64,6 +64,25 @@ void main() { g_length[2] = g_input23[0].data.length(); g_length[3] = g_input23[1].data.length(); } +)"; + + // GL 4.6 core 4.10 lets a buffer variable be declared readonly AND writeonly at once: + // it can then be neither read nor written, and `.length()` is the only thing left that + // may be asked of it. The pair is inert - and printing it into ESSL is not, because + // SPIRV-Cross hoists the qualifiers every member shares onto the BLOCK and Mesa's ES + // compiler refuses that spelling ("Interface block sets both readonly and writeonly"). + // Lifted from KHR-GL43.shader_storage_buffer_object.basic-readonly-writeonly. + constexpr const char* kReadonlyWriteonlyComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Input { + readonly writeonly int g_in[]; +}; +layout(std430, binding = 4) buffer Output { + int g_length[]; +}; +void main() { + g_length[0] = g_in.length(); +} )"; constexpr int kElementBytes = 16; // ivec4, std430 @@ -212,4 +231,33 @@ void main() { glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, input0); glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 3, input3); } + + // A buffer variable qualified readonly AND writeonly can only be asked its length, and that + // question still has to be answered. A stage the driver refused answers 0 - and refuses + // silently, because the program links without it and the dispatch is then a no-op. + TEST_F(SsboArrayLengthScenario, AReadonlyWriteonlyArrayStillReportsItsLength) { + if (!Ready() || IsSkipped()) return; + + const GLuint program = CompileComputeProgram(kReadonlyWriteonlyComputeSource); + ASSERT_NE(program, 0u) << m_buildLog; + + const GLuint input = MakeStorageBuffer(6); // 6 ivec4 = 24 ints + const GLuint output = MakeStorageBuffer(1); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, input); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 4, output); + ASSERT_EQ(FirstGLError(), 0u); + + glUseProgram(program); + glDispatchCompute(1, 1, 1); + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + int length = -1; + glBindBuffer(GL_SHADER_STORAGE_BUFFER, output); + glGetBufferSubData(GL_SHADER_STORAGE_BUFFER, 0, sizeof(length), &length); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_EQ(length, 24) << "a readonly+writeonly runtime array reported length " << length + << "; 0 means the stage never reached the program"; + + glUseProgram(m_program); + glDeleteProgram(program); + } } // namespace MGITest diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp index 453c0c16..095149c9 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp @@ -459,6 +459,43 @@ namespace MobileGL { SPVC_CHK_RETURN } + spvc_result SpvcSession::RelaxReadWriteExclusiveStorageBuffers() { + if (!(usage & SessionUsageBit::Transpile)) return SPVC_ERROR_INVALID_ARGUMENT; + + SPVC_CHK_INIT + const spvc_reflected_resource* list = nullptr; + size_t count = 0; + SPVC_CHK_RESULT(spvc_resources_get_resource_list_for_type( + resources, SPVC_RESOURCE_TYPE_STORAGE_BUFFER, &list, &count)); + for (size_t i = 0; i < count; ++i) { + const spvc_reflected_resource& resource = list[i]; + // The variable itself, for a block the application qualified as a whole. + if (spvc_compiler_has_decoration(compiler, resource.id, SpvDecorationNonReadable) && + spvc_compiler_has_decoration(compiler, resource.id, SpvDecorationNonWritable)) { + spvc_compiler_unset_decoration(compiler, resource.id, SpvDecorationNonReadable); + spvc_compiler_unset_decoration(compiler, resource.id, SpvDecorationNonWritable); + } + // ...and each member, which is where the qualifiers usually sit and where + // SPIRV-Cross reads them from before hoisting the ones every member shares. + const spvc_type blockType = spvc_compiler_get_type_handle(compiler, resource.base_type_id); + if (blockType == nullptr) continue; + const unsigned memberCount = spvc_type_get_num_member_types(blockType); + for (unsigned member = 0; member < memberCount; ++member) { + if (!spvc_compiler_has_member_decoration(compiler, resource.base_type_id, member, + SpvDecorationNonReadable) || + !spvc_compiler_has_member_decoration(compiler, resource.base_type_id, member, + SpvDecorationNonWritable)) { + continue; + } + spvc_compiler_unset_member_decoration(compiler, resource.base_type_id, member, + SpvDecorationNonReadable); + spvc_compiler_unset_member_decoration(compiler, resource.base_type_id, member, + SpvDecorationNonWritable); + } + } + SPVC_CHK_RETURN + } + spvc_result SpvcSession::Compile(const char** result) { if (!(usage & SessionUsageBit::Transpile)) return SPVC_ERROR_INVALID_ARGUMENT; SPVC_CHK_INIT diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h index 357d63c6..c1d7114a 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h @@ -139,6 +139,27 @@ namespace MobileGL { // must keep reaching the driver (the frontend's own glBindFragDataLocationIndexed // path already emits only non-zero indices for the same reason). spvc_result DropDefaultFragmentOutputColorIndex(); + // Drops `readonly` and `writeonly` from every shader storage block - and every + // block member - that carries BOTH of them. + // + // GL 4.6 core 4.10 lets a buffer variable be declared readonly AND writeonly at + // once: it then cannot be read or written at all, and the only thing left that + // it can be used for is `.length()`. The pair is therefore inert by + // construction - the frontend has already rejected any access to it - so + // dropping it cannot change what the shader does. + // + // Emitting it does change whether the shader EXISTS. SPIRV-Cross hoists the + // qualifiers every member shares onto the block, and Mesa's ES compiler rejects + // that spelling outright ("Interface block sets both readonly and writeonly", + // verified on Mesa 26.1.4 llvmpipe with no MobileGL in the process, against the + // exact source this transpiler emitted). The stage then never compiles, the + // program links without it, and every dispatch or draw is a silent no-op - + // which is how KHR-GL43.shader_storage_buffer_object.basic-readonly-writeonly + // read back 0 instead of the array length. + // + // A block carrying only ONE of the two is left exactly as it is: those really do + // constrain the accesses the shader makes, and the driver is entitled to know. + spvc_result RelaxReadWriteExclusiveStorageBuffers(); spvc_result Compile(const char** result); const SpvcMetadata& GetMetadata() const; const char* GetLastErrorString() const;