diff --git a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp index d9a87961..ea8a1201 100644 --- a/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp +++ b/MobileGL/MG_Impl/GLImpl/Program/GL_Program.cpp @@ -735,6 +735,12 @@ namespace MobileGL::MG_Impl::GLImpl { auto size = programObject->GetUniformSizesInBytes(location); char* pUBO = (char*)programObject->MapUBO(); auto* ttype = programObject->GetUniformTType(location); + if (pUBO == nullptr || offset == MG_State::GLState::ProgramObject::kInvalidUniformOffset || + offset + size > programObject->GetUBOSize()) { + MGLOG_E("%s: uniform at program %u location %d has no backing storage; returning nothing", __func__, + program, location); + return; + } if (!ttype->isMatrix() || ttype->getMatrixCols() != 3) Memcpy(params, pUBO + offset, size); @@ -784,6 +790,12 @@ namespace MobileGL::MG_Impl::GLImpl { auto size = programObject->GetUniformSizesInBytes(location); char* pUBO = static_cast(programObject->MapUBO()); auto* ttype = programObject->GetUniformTType(location); + if (pUBO == nullptr || offset == MG_State::GLState::ProgramObject::kInvalidUniformOffset || + offset + size > programObject->GetUBOSize()) { + MGLOG_E("%s: uniform at program %u location %d has no backing storage; returning nothing", __func__, + program, location); + return; + } if constexpr (std::is_same_v) { if (ttype->isMatrix() && ttype->getMatrixCols() == 3) { @@ -900,14 +912,31 @@ namespace MobileGL::MG_Impl::GLImpl { if (!programObject.IsUniformOpaqueAtLocation(location)) { MGLOG_D("%s: program = %d, location = %d, maxLocation = %d", __func__, programObject.GetExternalIndex(), location, programObject.GetMaxUniformLocation()); - auto size = programObject.GetUniformSizesInBytes(location); - auto offset = programObject.GetUniformOffset(location); - MOBILEGL_ASSERT(size >= ItemCount * sizeof(T), - "Uniform size mismatch, expected at least %zu bytes, got %zu bytes.", ItemCount * sizeof(T), - size); + const SizeT size = programObject.GetUniformSizesInBytes(location); + const Uint offset = programObject.GetUniformOffset(location); + char* pUBO = static_cast(programObject.MapUBO()); + const SizeT uboSize = programObject.GetUBOSize(); + SizeT writeSize = ItemCount * sizeof(T); + if (size < writeSize) { + // Metadata bug: degrade to a clamped copy instead of killing the process. + MGLOG_E("%s: uniform size mismatch at program %u location %u: expected at least %zu bytes, got %zu " + "bytes; clamping", + __func__, programObject.GetExternalIndex(), location, ItemCount * sizeof(T), size); + writeSize = size; + } + if (pUBO == nullptr || offset == MG_State::GLState::ProgramObject::kInvalidUniformOffset || + offset + byteOffsetInsideUniform + writeSize > uboSize) { + // Should not happen: linking gives every settable uniform backing + // storage. Log and drop the write instead of faulting. + MGLOG_E("%s: uniform at program %u location %u has no backing storage (ubo=%p offset=%u size=%zu " + "uboSize=%zu); dropping write", + __func__, programObject.GetExternalIndex(), location, static_cast(pUBO), offset, + writeSize, uboSize); + return; + } MGLOG_D("%s: program = %d, location = %d, byteOffset = %d", __func__, programObject.GetExternalIndex(), location, offset + byteOffsetInsideUniform); - Memcpy((char*)programObject.MapUBO() + offset + byteOffsetInsideUniform, value, ItemCount * sizeof(T)); + Memcpy(pUBO + offset + byteOffsetInsideUniform, value, writeSize); programObject.MarkUBOContentDirty(); } else { auto* ttype = programObject.GetUniformTType(location); @@ -940,6 +969,11 @@ namespace MobileGL::MG_Impl::GLImpl { } for (GLint offset = 0; offset < count; offset++) { + if (offset > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + offset)) { + // GL 3.3 §2.11.4: values for elements beyond the end of the uniform + // array are ignored. Never step onto a neighboring uniform's location. + break; + } if (!programObject->IsValidUniformLocation(location + offset)) { RecordInvalidUniformLocationError(__func__, location + offset, "the current program object"); return; @@ -964,6 +998,10 @@ namespace MobileGL::MG_Impl::GLImpl { } for (GLint offset = 0; offset < count; offset++) { + if (offset > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + offset)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + offset)) { RecordInvalidUniformLocationError(__func__, location + offset, "program " + std::to_string(program)); @@ -1092,6 +1130,10 @@ namespace MobileGL::MG_Impl::GLImpl { // For matrix uniforms, we handle each matrix individually for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "the current program object"); return; @@ -1124,6 +1166,10 @@ namespace MobileGL::MG_Impl::GLImpl { // For matrix uniforms, we handle each matrix individually // Handle padding in mat3 correctly!! for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "the current program object"); return; @@ -1159,6 +1205,10 @@ namespace MobileGL::MG_Impl::GLImpl { // For matrix uniforms, we handle each matrix individually for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "the current program object"); return; @@ -1219,6 +1269,10 @@ namespace MobileGL::MG_Impl::GLImpl { } for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "program " + std::to_string(program)); return; @@ -1249,6 +1303,10 @@ namespace MobileGL::MG_Impl::GLImpl { } for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "program " + std::to_string(program)); return; @@ -1283,6 +1341,10 @@ namespace MobileGL::MG_Impl::GLImpl { } for (GLint i = 0; i < count; i++) { + if (i > 0 && !programObject->UniformLocationsAliasSameUniform(location, location + i)) { + // Values for elements beyond the end of the uniform array are ignored. + break; + } if (!programObject->IsValidUniformLocation(location + i)) { RecordInvalidUniformLocationError(__func__, location + i, "program " + std::to_string(program)); return; diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp index 8abe3b5d..5acd0bd6 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.cpp @@ -8,6 +8,7 @@ #include "ProgramObject.h" #include +#include #include #include #include @@ -87,6 +88,19 @@ namespace { } } + // How many consecutive uniform locations a uniform occupies. Array uniforms (opaque + // or not) span one location per element so glUniform*v(count > 1) and + // glGetUniformLocation("arr[k]") can address elements individually; everything else + // spans a single location. TObjectReflection.size only carries the element count for + // non-block arrays, so prefer the TType, which is authoritative for both. + static MobileGL::Int GetUniformLocationSpan(const glslang::TObjectReflection& uniform) { + const glslang::TType* type = uniform.getType(); + if (type != nullptr && type->isSizedArray()) { + return std::max(1, type->getOuterArraySize()); + } + return std::max(1, uniform.size); + } + static bool ComputeShaderDeclaresLocalSize(const MobileGL::String& source) { bool inLineComment = false; bool inBlockComment = false; @@ -361,8 +375,7 @@ namespace MobileGL::MG_State::GLState { for (int i = 0; i < m_activeUniformCount; i++) { auto& uniform = m_program->getUniform(i); auto location = uniform.layoutLocation(); - const Int locationSpan = - (uniform.getType() && uniform.getType()->isOpaque()) ? std::max(1, uniform.size) : 1; + const Int locationSpan = GetUniformLocationSpan(uniform); requiredUniformLocations += locationSpan; if (location != glslang::TQualifier::layoutLocationEnd) { m_maxUniformLocation = std::max(m_maxUniformLocation, location + locationSpan - 1); @@ -401,8 +414,7 @@ namespace MobileGL::MG_State::GLState { m_externalIndex, uniform.name.c_str()); continue; // will allocate unallocated uniforms later } - const Int locationSpan = - (uniform.getType() && uniform.getType()->isOpaque()) ? std::max(1, uniform.size) : 1; + const Int locationSpan = GetUniformLocationSpan(uniform); for (Int element = 0; element < locationSpan; ++element) { m_uniformIndexInTProgram[location + element] = i; } @@ -419,8 +431,8 @@ namespace MobileGL::MG_State::GLState { }); for (auto index : unallocatedUniformIndex) { auto& uniform = m_program->getUniform(index); - const Int locationSpan = - (uniform.getType() && uniform.getType()->isOpaque()) ? std::max(1, uniform.size) : 1; + const Int locationSpan = GetUniformLocationSpan(uniform); + Bool placed = false; for (; locNeedle <= m_maxUniformLocation; locNeedle++) { bool hasRoom = locNeedle + locationSpan - 1 <= m_maxUniformLocation; for (Int element = 0; hasRoom && element < locationSpan; ++element) { @@ -437,8 +449,25 @@ namespace MobileGL::MG_State::GLState { "(index %d)", m_externalIndex, uniform.name.c_str(), locNeedle, locNeedle + locationSpan - 1, index); locNeedle += locationSpan; + placed = true; break; } + if (!placed) { + // Explicit-location uniforms can fragment the space so no contiguous + // span is left; grow the table instead of leaving the uniform without + // a location (which would make it unsettable via glUniform*). + const SizeT base = m_uniformIndexInTProgram.size(); + m_uniformIndexInTProgram.resize(base + locationSpan, glslang::TQualifier::layoutLocationEnd); + m_uniformSamplerOrImageUnitIndex.resize(base + locationSpan, -1); + m_maxUniformLocation = static_cast(base + locationSpan - 1); + for (Int element = 0; element < locationSpan; ++element) { + m_uniformIndexInTProgram[base + element] = index; + } + m_uniformLocations[uniform.name] = static_cast(base); + MGLOG_D("ProgramObject %u: Reflection - grew location table to place uniform '%s' at %zu..%zu", + m_externalIndex, uniform.name.c_str(), base, base + locationSpan - 1); + locNeedle = base + locationSpan; + } } for (int i = 0; i < m_activeUniformCount; i++) { @@ -457,7 +486,7 @@ namespace MobileGL::MG_State::GLState { const auto explicitBinding = m_explicitOpaqueUniformBindings.find(uniform.name); const int initialUnit = explicitBinding != m_explicitOpaqueUniformBindings.end() ? static_cast(explicitBinding->second) : 0; - const Int locationSpan = std::max(1, uniform.size); + const Int locationSpan = GetUniformLocationSpan(uniform); for (Int element = 0; element < locationSpan && location + element < m_uniformSamplerOrImageUnitIndex.size(); ++element) { m_uniformSamplerOrImageUnitIndex[location + element] = @@ -622,8 +651,11 @@ namespace MobileGL::MG_State::GLState { m_uniformSizesInBytes.clear(); m_uniformOffsets.clear(); m_globalUboScratch.clear(); - m_uniformOffsets.resize(m_maxUniformLocation + 1); - m_uniformSizesInBytes.resize(m_maxUniformLocation + 1); + // kInvalidUniformOffset marks locations that end up without global-UBO backing + // (e.g. the optimizer eliminated every use of the uniform); the fallback pass + // below gives those locations tail storage so glUniform* always has a target. + m_uniformOffsets.resize(m_maxUniformLocation + 1, kInvalidUniformOffset); + m_uniformSizesInBytes.resize(m_maxUniformLocation + 1, 0); for (SizeT i = 0; i < m_generatedSpirv.size(); i++) { auto& spv = m_generatedSpirv[i]; @@ -653,31 +685,89 @@ namespace MobileGL::MG_State::GLState { m_globalUboScratch.resize(size); } for (const auto& [name, offset] : meta.plainUniformOffsetsInUBO) { - if (m_uniformLocations.find(name) != m_uniformLocations.end()) { - m_uniformOffsets[m_uniformLocations[name]] = offset; - MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' offset=%u assigned to location %u", - m_externalIndex, name.c_str(), offset, m_uniformLocations[name]); - } else { - MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' offset=%u but not found in " - "m_uniformLocations", - m_externalIndex, name.c_str(), offset); - } - } - for (const auto& [name, size] : meta.plainUniformMemberSizesInBytes) { - if (m_uniformLocations.find(name) != m_uniformLocations.end()) { - m_uniformSizesInBytes[m_uniformLocations[name]] = size; - MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' size=%u assigned to location %u", - m_externalIndex, name.c_str(), size, m_uniformLocations[name]); - } else { - MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' size=%u but not found in " - "m_uniformLocations", - m_externalIndex, name.c_str(), size); - } + const auto locationIt = m_uniformLocations.find(name); + if (locationIt == m_uniformLocations.end()) { + MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' offset=%u but not found in " + "m_uniformLocations", + m_externalIndex, name.c_str(), offset); + continue; + } + const Uint baseLocation = locationIt->second; + if (!IsValidUniformLocation(static_cast(baseLocation))) { + continue; + } + + const Int uniformIndex = m_uniformIndexInTProgram[baseLocation]; + const GLint arraySize = GetActiveUniformArraySize(uniformIndex); + SizeT memberSize = 0; + const auto sizeIt = meta.plainUniformMemberSizesInBytes.find(name); + if (sizeIt != meta.plainUniformMemberSizesInBytes.end()) { + memberSize = sizeIt->second; + } + Uint arrayStride = 0; + const auto strideIt = meta.plainUniformArrayStridesInUBO.find(name); + if (strideIt != meta.plainUniformArrayStridesInUBO.end()) { + arrayStride = strideIt->second; + } + + // Array uniforms span one location per element (see DoReflection); + // give each element its real byte offset inside the UBO. + const GLint elementCount = (arraySize > 1 && arrayStride == 0) ? 1 : std::max(arraySize, 1); + for (GLint element = 0; element < elementCount; ++element) { + const Uint location = baseLocation + static_cast(element); + if (location > m_maxUniformLocation || m_uniformIndexInTProgram[location] != uniformIndex) { + break; + } + m_uniformOffsets[location] = offset + static_cast(element) * arrayStride; + const SizeT consumed = static_cast(element) * arrayStride; + m_uniformSizesInBytes[location] = memberSize > consumed ? memberSize - consumed : 0; + } + MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' offset=%u stride=%u size=%zu assigned " + "to locations %u..%u", + m_externalIndex, name.c_str(), offset, arrayStride, memberSize, baseLocation, + baseLocation + static_cast(elementCount) - 1); } MGLOG_D("ProgramObject %u: GenerateBinary - finished parsing module %zu metadata", m_externalIndex, i); } } + + // Fallback pass: a linked program's active non-opaque uniforms must accept + // glUniform*/glGetUniform* even when the optimized SPIR-V no longer contains + // them (AggressiveDCE can remove a dead loop together with the only loads of a + // uniform -- or the entire global UBO, leaving the scratch unallocated). Hand + // such locations CPU-side storage at the (16-byte aligned) tail of the shadow + // buffer; backends bind at least the SPIR-V-declared UBO range, and the GPU + // never reads these bytes, so this only keeps the GL-visible state coherent. + for (Uint location = 0; location <= m_maxUniformLocation; ++location) { + if (m_uniformOffsets[location] != kInvalidUniformOffset) continue; + if (!IsValidUniformLocation(static_cast(location))) continue; + const auto& uniform = m_program->getUniform(m_uniformIndexInTProgram[location]); + const glslang::TType* type = uniform.getType(); + if (type != nullptr && type->isOpaque()) continue; + if (uniform.index >= 0 && uniform.index < m_program->getNumUniformBlocks() && + std::strstr(m_program->getUniformBlock(uniform.index).name.c_str(), + MG_Util::ShaderTranspiler::GLOBAL_UBO_NAME) == nullptr) { + // Member of a named uniform block: not settable through glUniform*, so it + // needs no global-UBO shadow storage. + continue; + } + + // std140-style slot: the matrix upload paths write column vectors at + // 16-byte strides, so a matrix slot must cover cols * 16 bytes. + SizeT slotSize = MG_Util::GetGLTypeSize(uniform.glDefineType); + if (type != nullptr && type->isMatrix()) { + slotSize = static_cast(type->getMatrixCols()) * 16u; + } + slotSize = (slotSize + 15u) & ~static_cast(15u); + const SizeT slotOffset = (m_globalUboScratch.size() + 15u) & ~static_cast(15u); + m_globalUboScratch.resize(slotOffset + slotSize, 0); + m_uniformOffsets[location] = static_cast(slotOffset); + m_uniformSizesInBytes[location] = slotSize; + MGLOG_D("ProgramObject %u: GenerateBinary - uniform '%s' location %u has no UBO backing in the " + "generated SPIR-V (optimized out?); allocated %zu fallback bytes at scratch offset %zu", + m_externalIndex, uniform.name.c_str(), location, slotSize, slotOffset); + } } void ProgramObject::WaitUntilGenerationCompleted() const { diff --git a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h index 3687e05d..715f56a4 100644 --- a/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h +++ b/MobileGL/MG_State/GLState/ProgramState/ProgramObject.h @@ -43,8 +43,40 @@ namespace MobileGL::MG_State::GLState { Uint GetMaxUniformLocation() const { return m_maxUniformLocation; } Int GetUniformLocation(const String& name) const { const auto it = m_uniformLocations.find(name); - if (it == m_uniformLocations.end()) return -1; - return (Int)it->second; + if (it != m_uniformLocations.end()) return (Int)it->second; + + // "arr[k]" resolves to the location of element k: glslang reflection stores + // arrays under their base name (no "[0]" suffix), and DoReflection reserves + // one location per array element, so element k lives at base + k. + if (name.length() < 4 || name.back() != ']') return -1; + const SizeT bracket = name.rfind('['); + // Require at least one digit between the brackets. + if (bracket == String::npos || bracket + 1 >= name.length() - 1) return -1; + Uint element = 0; + for (SizeT i = bracket + 1; i < name.length() - 1; ++i) { + if (name[i] < '0' || name[i] > '9') return -1; + element = element * 10 + static_cast(name[i] - '0'); + if (element > 0x0FFFFFFFu) return -1; + } + const auto baseIt = m_uniformLocations.find(name.substr(0, bracket)); + if (baseIt == m_uniformLocations.end()) return -1; + const Int base = (Int)baseIt->second; + if (!IsValidUniformLocation(base)) return -1; + const Int index = m_uniformIndexInTProgram[base]; + // "[k]" only addresses arrays ("scalar[0]" is not a uniform name), and only + // in-range elements. + const glslang::TType* type = m_program->getUniform(index).getType(); + if (type == nullptr || !type->isArray()) return -1; + if (static_cast(element) >= GetActiveUniformArraySize(index)) return -1; + const Int location = base + (Int)element; + if (!UniformLocationsAliasSameUniform(base, location)) return -1; + return location; + } + + // True when both locations are element slots of the same uniform variable. + Bool UniformLocationsAliasSameUniform(Int a, Int b) const { + if (!IsValidUniformLocation(a) || !IsValidUniformLocation(b)) return false; + return m_uniformIndexInTProgram[a] == m_uniformIndexInTProgram[b]; } Int GetActiveUniformIndex(const String& name) const { @@ -170,6 +202,9 @@ namespace MobileGL::MG_State::GLState { auto& uniform = m_program->getUniform(static_cast(index)); return uniform.name; } + // Sentinel for a uniform location without global-UBO backing storage (should not + // survive linking: GenerateBinary falls back to tail-allocated scratch storage). + static constexpr Uint kInvalidUniformOffset = ~0u; Uint GetUniformOffset(Uint location) const { return m_uniformOffsets[location]; } Uint GetUniformSizesInBytes(Uint location) const { return MG_Util::GetGLTypeSize(GetUniformType(location)); } diff --git a/MobileGL/MG_Test/Program/ProgramTest.cpp b/MobileGL/MG_Test/Program/ProgramTest.cpp index acad1095..015445a1 100644 --- a/MobileGL/MG_Test/Program/ProgramTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramTest.cpp @@ -1845,3 +1845,211 @@ TEST_F(ProgramTest, GetActiveUniformsivErrors) { EXPECT_EQ(GetError(), GL_NO_ERROR); EXPECT_EQ(params[0], -999); } + +namespace { + GLuint LinkVsFsProgram(const char* vsSource, const char* fsSource) { + char infoLog[4096] = ""; + GLuint vs = CreateShader(GL_VERTEX_SHADER); + ShaderSource(vs, 1, &vsSource, nullptr); + CompileShader(vs); + GLint vsStatus = GL_FALSE; + GetShaderiv(vs, GL_COMPILE_STATUS, &vsStatus); + GetShaderInfoLog(vs, sizeof(infoLog), nullptr, infoLog); + EXPECT_EQ(vsStatus, GL_TRUE) << infoLog; + + GLuint fs = CreateShader(GL_FRAGMENT_SHADER); + ShaderSource(fs, 1, &fsSource, nullptr); + CompileShader(fs); + GLint fsStatus = GL_FALSE; + GetShaderiv(fs, GL_COMPILE_STATUS, &fsStatus); + GetShaderInfoLog(fs, sizeof(infoLog), nullptr, infoLog); + EXPECT_EQ(fsStatus, GL_TRUE) << infoLog; + + GLuint program = CreateProgram(); + AttachShader(program, vs); + AttachShader(program, fs); + LinkProgram(program); + GLint linkStatus = GL_FALSE; + GetProgramiv(program, GL_LINK_STATUS, &linkStatus); + GetProgramInfoLog(program, sizeof(infoLog), nullptr, infoLog); + EXPECT_EQ(linkStatus, GL_TRUE) << infoLog; + return program; + } + + const char* kPassthroughCoordsVs = R"(#version 330 +in vec4 a_position; +in vec4 a_coords; +out vec4 coords_in; +void main() { + gl_Position = a_position; + coords_in = a_coords; +})"; +} // namespace + +// Repro for KHR-GL33.shaders.loops.do_while_dynamic_iterations.empty_body_* (and the +// only_continue / unconditional_break variants): the loop is dead code, so the SPIR-V +// optimizer eliminates it together with the only loads of `one` / `ui_one` -- and with +// them the entire global UBO. The uniforms stay active in link reflection, so +// glUniform1i on them must still have backing storage instead of memcpy-ing to null. +TEST_F(ProgramTest, DoWhileDeadLoopUniformsKeepBackingStorage) { + const char* loopBodies[] = {"", "continue;", "break;"}; + for (const char* body : loopBodies) { + const String fsSource = String(R"(#version 330 +uniform int ui_one; +uniform mediump int one; +in vec4 coords_in; +out vec4 o_color; +void main() { + vec4 res = coords_in; + mediump int i = 0; + do {)") + body + R"(} while (i++ < one*ui_one); + o_color = res; +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource.c_str()); + + const GLint locOne = GetUniformLocation(program, "one"); + const GLint locUiOne = GetUniformLocation(program, "ui_one"); + ASSERT_GE(locOne, 0) << "body: '" << body << "'"; + ASSERT_GE(locUiOne, 0) << "body: '" << body << "'"; + + UseProgram(program); + Uniform1i(locOne, 1); // crashed with a null MapUBO() before the fallback storage + Uniform1i(locUiOne, 2); + EXPECT_EQ(GetError(), GL_NO_ERROR) << "body: '" << body << "'"; + + GLint readback = -1; + GetUniformiv(program, locOne, &readback); + EXPECT_EQ(readback, 1) << "body: '" << body << "'"; + readback = -1; + GetUniformiv(program, locUiOne, &readback); + EXPECT_EQ(readback, 2) << "body: '" << body << "'"; + EXPECT_EQ(GetError(), GL_NO_ERROR) << "body: '" << body << "'"; + } +} + +// Repro for KHR-GL33.shaders.struct.uniform.*nested_struct_array_*: leaf uniforms of +// nested struct arrays need (a) one location per array element and (b) real byte +// offsets inside the global UBO. Before the fix every leaf had a single location and +// offset 0, so glUniform2fv(loc, 2, ...) tripped the size assert on the neighboring +// float uniform (and corrupted it in release builds). +TEST_F(ProgramTest, NestedStructArrayUniformElementWrites) { + // Struct shape from CTS glcShaderStructTests nested_struct_array (uniform case). + const char* fsSource = R"(#version 330 +struct T { + mediump float a; + mediump vec2 b[2]; +}; +struct S { + mediump float a; + T b[3]; + int c; +}; +uniform S s[2]; +in vec4 coords_in; +out vec4 o_color; +void main() { + mediump float r = (s[0].b[1].b[0].x + s[1].b[2].b[1].y) * s[0].b[0].a; + mediump float g = s[1].b[0].b[0].y * s[0].b[2].a * s[1].b[2].a; + mediump float b = (s[0].b[2].b[1].y + s[0].b[1].b[0].y + s[1].a) * s[0].b[1].a; + mediump float a = float(s[0].c) + s[1].b[2].a - s[1].b[1].a; + o_color = vec4(r, g, b, a); +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + UseProgram(program); + + const GLint locVecArray = GetUniformLocation(program, "s[0].b[1].b"); + ASSERT_GE(locVecArray, 0); + // Element locations are consecutive and reachable via the "[k]" suffix. + EXPECT_EQ(GetUniformLocation(program, "s[0].b[1].b[0]"), locVecArray); + EXPECT_EQ(GetUniformLocation(program, "s[0].b[1].b[1]"), locVecArray + 1); + EXPECT_EQ(GetUniformLocation(program, "s[0].b[1].b[2]"), -1); + + // Distinct scalar leaves must land at distinct UBO offsets (they all aliased + // offset 0 before the fix). + const char* scalarLeaves[] = {"s[0].b[0].a", "s[0].b[1].a", "s[0].b[2].a", "s[1].a", "s[1].b[1].a", + "s[1].b[2].a"}; + const GLfloat scalarValues[] = {0.5f, 0.25f, 0.125f, 7.0f, 3.0f, 4.0f}; + for (SizeT i = 0; i < std::size(scalarLeaves); ++i) { + const GLint loc = GetUniformLocation(program, scalarLeaves[i]); + ASSERT_GE(loc, 0) << scalarLeaves[i]; + Uniform1f(loc, scalarValues[i]); + } + + // CTS-style whole-array write: glUniform2fv with count = 2 on a vec2[2] leaf. + // Before the fix this asserted/corrupted the next uniform ("s[0].b[2].a"). + const GLfloat vecData[4] = {1.0f, 2.0f, 3.0f, 4.0f}; + Uniform2fv(locVecArray, 2, vecData); + EXPECT_EQ(GetError(), GL_NO_ERROR); + + GLfloat vecReadback[2] = {}; + GetUniformfv(program, locVecArray, vecReadback); + EXPECT_EQ(vecReadback[0], 1.0f); + EXPECT_EQ(vecReadback[1], 2.0f); + GetUniformfv(program, locVecArray + 1, vecReadback); + EXPECT_EQ(vecReadback[0], 3.0f); + EXPECT_EQ(vecReadback[1], 4.0f); + + // All scalar leaves survived the array write intact. + for (SizeT i = 0; i < std::size(scalarLeaves); ++i) { + GLfloat readback = -1.0f; + GetUniformfv(program, GetUniformLocation(program, scalarLeaves[i]), &readback); + EXPECT_EQ(readback, scalarValues[i]) << scalarLeaves[i]; + } + + // std140: vec2 array elements inside the struct are 16 bytes apart, and the + // per-element offsets differ. + auto programObject = MG_State::pGLContext->GetProgramObject(program); + ASSERT_NE(programObject, nullptr); + const Uint offsetElement0 = programObject->GetUniformOffset(static_cast(locVecArray)); + const Uint offsetElement1 = programObject->GetUniformOffset(static_cast(locVecArray + 1)); + EXPECT_EQ(offsetElement1, offsetElement0 + 16u); + EXPECT_EQ(GetError(), GL_NO_ERROR); +} + +// Plain top-level uniform arrays share the same per-element location machinery. +TEST_F(ProgramTest, PlainArrayUniformElementLocationsAndWrites) { + const char* fsSource = R"(#version 330 +uniform float arr[4]; +uniform float guard; +in vec4 coords_in; +out vec4 o_color; +void main() { + o_color = vec4(arr[0] + arr[1], arr[2] + arr[3], guard, 1.0); +})"; + GLuint program = LinkVsFsProgram(kPassthroughCoordsVs, fsSource); + UseProgram(program); + + const GLint locArr = GetUniformLocation(program, "arr"); + ASSERT_GE(locArr, 0); + EXPECT_EQ(GetUniformLocation(program, "arr[0]"), locArr); + EXPECT_EQ(GetUniformLocation(program, "arr[2]"), locArr + 2); + EXPECT_EQ(GetUniformLocation(program, "arr[4]"), -1); + + const GLint locGuard = GetUniformLocation(program, "guard"); + ASSERT_GE(locGuard, 0); + EXPECT_EQ(GetUniformLocation(program, "guard[0]"), -1); // not an array + + Uniform1f(locGuard, 9.0f); + + const GLfloat values[4] = {1.0f, 2.0f, 3.0f, 4.0f}; + Uniform1fv(locArr, 4, values); + for (int i = 0; i < 4; ++i) { + GLfloat readback = -1.0f; + GetUniformfv(program, locArr + i, &readback); + EXPECT_EQ(readback, values[i]) << "arr[" << i << "]"; + } + + // Overlong writes stop at the end of the array (GL 3.3 §2.11.4) instead of + // spilling into the next uniform. + const GLfloat tail[3] = {30.0f, 40.0f, 50.0f}; + Uniform1fv(GetUniformLocation(program, "arr[2]"), 3, tail); + EXPECT_EQ(GetError(), GL_NO_ERROR); + GLfloat readback = -1.0f; + GetUniformfv(program, locArr + 2, &readback); + EXPECT_EQ(readback, 30.0f); + GetUniformfv(program, locArr + 3, &readback); + EXPECT_EQ(readback, 40.0f); + GetUniformfv(program, locGuard, &readback); + EXPECT_EQ(readback, 9.0f); // untouched by the overlong write + EXPECT_EQ(GetError(), GL_NO_ERROR); +} diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp index 7c31ecb0..32e3d831 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.cpp @@ -48,6 +48,69 @@ namespace MobileGL { return SPVC_BASETYPE_UNKNOWN; } + // Record one flattened leaf uniform of the global UBO into the metadata maps. + static void RecordGlobalUboLeaf(const SpvReflectBlockVariable& member, const String& name, + Uint32 offsetInUBO, SpvcMetadata& metadata) { + metadata.plainUniformOffsetsInUBO[name] = offsetInUBO; + metadata.plainUniformMemberSizesInBytes[name] = member.size; + metadata.plainUniformArrayStridesInUBO[name] = + member.array.dims_count > 0 ? member.array.stride : 0; + + Uint32 vectorSize = member.numeric.vector.component_count; + if (vectorSize == 0) vectorSize = 1; + Uint32 matCol = member.numeric.matrix.column_count; + if (matCol == 0) matCol = 1; + metadata.plainUniformMemberTypes[name] = { + .basetype = MapReflectToSpvcBasetype(member), + .vectorSize = vectorSize, + .matCol = matCol, + }; + } + + // Flatten a (possibly nested struct / struct array) member of the global UBO + // into leaf entries named the way glslang reflection names plain uniforms: + // "s[0].b[1].b" for `uniform S s[2]` with `struct T { vec2 b[2]; }` members. + // glUniform* writes are routed per leaf location, so the state layer needs a + // byte offset for every leaf, not just for the top-level block members. + // `baseOffset` accumulates parent offsets; member.offset is relative to the + // enclosing struct (top-level members: relative to the block start). + static void FlattenGlobalUboMember(const SpvReflectBlockVariable& member, const String& prefix, + Uint32 baseOffset, SpvcMetadata& metadata) { + const String name = prefix + (member.name != nullptr ? member.name : ""); + const Uint32 selfOffset = baseOffset + member.offset; + + if (member.member_count == 0 || member.members == nullptr) { + RecordGlobalUboLeaf(member, name, selfOffset, metadata); + return; + } + + if (member.array.dims_count == 0) { + // Plain nested struct. + for (Uint32 j = 0; j < member.member_count; ++j) { + FlattenGlobalUboMember(member.members[j], name + ".", selfOffset, metadata); + } + return; + } + + if (member.array.dims_count > 1) { + // Arrays of arrays of structs cannot be declared in the GL 3.3-era GLSL + // MobileGL ingests; record the base so at least element 0 resolves. + MGLOG_W("FlattenGlobalUboMember: multi-dimensional struct array '%s' is not supported, " + "flattening element 0 only", + name.c_str()); + } + + const Uint32 elementCount = member.array.dims[0] > 0 ? member.array.dims[0] : 1; + const Uint32 elementStride = member.array.stride; + for (Uint32 element = 0; element < elementCount; ++element) { + const String elementPrefix = name + "[" + std::to_string(element) + "]."; + const Uint32 elementOffset = selfOffset + element * elementStride; + for (Uint32 j = 0; j < member.member_count; ++j) { + FlattenGlobalUboMember(member.members[j], elementPrefix, elementOffset, metadata); + } + } + } + SpvcSession::SpvcSession(const Vector& spirv, Flags usage) : usage(usage) { if (usage & SessionUsageBit::Transpile) { @@ -299,20 +362,10 @@ namespace MobileGL { metadata.globalUboSize = block.size; for (uint32_t j = 0; j < block.member_count; ++j) { - auto& member = block.members[j]; - metadata.plainUniformOffsetsInUBO[member.name] = member.offset; - metadata.plainUniformMemberSizesInBytes[member.name] = member.size; - - Uint32 vectorSize = member.numeric.vector.component_count; - if (vectorSize == 0) vectorSize = 1; - Uint32 matCol = member.numeric.matrix.column_count; - if (matCol == 0) matCol = 1; - - metadata.plainUniformMemberTypes[member.name] = { - .basetype = MapReflectToSpvcBasetype(member), - .vectorSize = vectorSize, - .matCol = matCol, - }; + // Recurse into nested structs / struct arrays so every leaf + // uniform ("s[0].b[1].b") gets its real byte offset; top-level + // scalars/vectors/matrices flatten to themselves. + FlattenGlobalUboMember(block.members[j], "", 0, metadata); } return SPVC_SUCCESS; } diff --git a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h index bd7de090..f5296b2e 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h +++ b/MobileGL/MG_Util/ShaderTranspiler/SpvcSession.h @@ -65,6 +65,11 @@ namespace MobileGL { UnorderedMap plainUniformOffsetsInUBO; UnorderedMap plainUniformMemberSizesInBytes; UnorderedMap plainUniformMemberTypes; + // Byte stride between consecutive array elements of an arrayed plain + // uniform (0 for non-arrays). Keyed like the offset map: names are the + // flattened leaf names glslang reflection uses ("s[0].b[1].b"), without + // a trailing "[0]". + UnorderedMap plainUniformArrayStridesInUBO; SizeT globalUboSize = 0; };