Skip to content

Commit

Permalink
Decouple OpenGL version from the logic for querying Uniform block buf…
Browse files Browse the repository at this point in the history 
…fer name

Summary:
The Uniform block buffer and member names used for querying the reflection data were tied to OpenGL version which is not necessarily always true. This led to platform specific bugs for example where certain materials using Uniform blocks rendered incorrectly on Windows on OpenGL 3.2.
This diff ensures we go through all possible buffer names and use the one that matches.

Reviewed By: syeh1, AmesingFlank

Differential Revision: D53013021

fbshipit-source-id: 24351b0437b98a5701d0168bfa197c4bbb9dc644
  • Loading branch information
@Abhi-hpp @facebook-github-bot
Abhi-hpp authored and facebook-github-bot committed Jan 26, 2024
1 parent 0810f11 commit a14e09c
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 50 deletions.
89 changes: 46 additions & 43 deletions IGLU/simple_renderer/ShaderUniforms.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,10 @@ igl::NameHandle ShaderUniforms::getQualifiedMemberName(const igl::NameHandle& bl
return igl::genNameHandle(blockInstanceName.toString() + "." + memberName.toString());
}

igl::NameHandle ShaderUniforms::getBufferName(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName) {
std::vector<std::pair<igl::NameHandle, igl::NameHandle>>
ShaderUniforms::getPossibleBufferAndMemberNames(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName) {
/**
Given an SparkSL/GLSL3 interface block:
```
Expand All @@ -184,34 +185,23 @@ igl::NameHandle ShaderUniforms::getBufferName(const igl::NameHandle& blockTypeNa
}
```
In OpenGL3, the name of the buffer block is `BlockTypeName`.
In OpenGL3, the name of the buffer block is `BlockTypeName` and the member name is 'memberName'.
In legacy OpenGL, we treat each member of the struct an individual uniform, so the name is
`blockInstanceName.f`
In legacy OpenGL, we treat each member of the struct an individual uniform, so both the buffer
name and member name are `blockInstanceName.f`
In Metal, the name of the block is `blockInstanceName`
In Metal, the name of the block is `blockInstanceName` and the member name is 'memberName'.
*/
if (device_.getBackendType() == igl::BackendType::Metal) {
return blockInstanceName;
return {{blockInstanceName, memberName}};
} else {
if (device_.getBackendType() == igl::BackendType::OpenGL) {
if (device_.getShaderVersion().majorVersion < 3) {
return getQualifiedMemberName(blockTypeName, blockInstanceName, memberName);
}
}
return blockTypeName;
}
}

igl::NameHandle ShaderUniforms::getBufferMemberName(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName) {
if (device_.getBackendType() == igl::BackendType::OpenGL) {
if (device_.getShaderVersion().majorVersion < 3) {
return getQualifiedMemberName(blockTypeName, blockInstanceName, memberName);
auto qualifiedName =
ShaderUniforms::getQualifiedMemberName(blockTypeName, blockInstanceName, memberName);
return {{blockTypeName, memberName}, {qualifiedName, qualifiedName}};
}
return {{blockTypeName, memberName}};
}
return memberName;
}

void ShaderUniforms::setUniformBytes(const UniformDesc& uniformDesc,
Expand Down Expand Up @@ -258,22 +248,29 @@ void ShaderUniforms::setUniformBytes(const igl::NameHandle& blockTypeName,
size_t elementSize,
size_t count,
size_t arrayIndex) {
auto bufferName = getBufferName(blockTypeName, blockInstanceName, memberName);
auto range = _bufferDescs.equal_range(bufferName);
IGL_ASSERT_MSG(range.first != range.second, "Buffer not found: %s", bufferName.c_str());
auto possibleBufferNames =
getPossibleBufferAndMemberNames(blockTypeName, blockInstanceName, memberName);

for (auto bufferDescIt = range.first; bufferDescIt != range.second; ++bufferDescIt) {
auto& bufferDesc = bufferDescIt->second;
auto bufferMemberName = getBufferMemberName(blockTypeName, blockInstanceName, memberName);
auto memberIndexIt = bufferDesc->memberIndices.find(bufferMemberName);
if (memberIndexIt == bufferDesc->memberIndices.end()) {
IGL_LOG_ERROR_ONCE(
"Member %s not found in buffer %s", bufferMemberName.c_str(), bufferName.c_str());
for (auto& [bufferName, bufferMemberName] : possibleBufferNames) {
auto range = _bufferDescs.equal_range(bufferName);
if (range.first == range.second) {
continue;
}
auto& uniformDesc = bufferDesc->uniforms[memberIndexIt->second];
setUniformBytes(uniformDesc, data, elementSize, count, arrayIndex);

for (auto bufferDescIt = range.first; bufferDescIt != range.second; ++bufferDescIt) {
auto& bufferDesc = bufferDescIt->second;
auto memberIndexIt = bufferDesc->memberIndices.find(bufferMemberName);
if (memberIndexIt == bufferDesc->memberIndices.end()) {
IGL_LOG_ERROR_ONCE(
"Member %s not found in buffer %s", bufferMemberName.c_str(), bufferName.c_str());
continue;
}
auto& uniformDesc = bufferDesc->uniforms[memberIndexIt->second];
setUniformBytes(uniformDesc, data, elementSize, count, arrayIndex);
}
return;
}
IGL_LOG_ERROR_ONCE("Buffer block not found: %s", blockTypeName.c_str());
}

void ShaderUniforms::setUniformBytes(const igl::NameHandle& name,
Expand Down Expand Up @@ -932,13 +929,19 @@ igl::Result ShaderUniforms::setSuballocationIndex(const igl::NameHandle& name, i
bool ShaderUniforms::containsUniform(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName) {
auto bufferName = getBufferName(blockTypeName, blockInstanceName, memberName);
auto bufferDescIt = _bufferDescs.find(bufferName);
if (bufferDescIt == _bufferDescs.end()) {
return false;
}
auto& bufferDesc = bufferDescIt->second;
auto bufferMemberName = getBufferMemberName(blockTypeName, blockInstanceName, memberName);
return bufferDesc->memberIndices.find(bufferMemberName) != bufferDesc->memberIndices.end();
auto possibleBufferNames =
getPossibleBufferAndMemberNames(blockTypeName, blockInstanceName, memberName);

for (auto& [bufferName, bufferMemberName] : possibleBufferNames) {
auto bufferDescIt = _bufferDescs.find(bufferName);
if (bufferDescIt == _bufferDescs.end()) {
continue;
}
auto& bufferDesc = bufferDescIt->second;
if (bufferDesc->memberIndices.find(bufferMemberName) != bufferDesc->memberIndices.end()) {
return true;
}
}
return false;
}
} // namespace iglu::material
11 changes: 4 additions & 7 deletions IGLU/simple_renderer/ShaderUniforms.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,13 +279,10 @@ class ShaderUniforms final {
std::unordered_map<std::string, TextureSlot> _allTexturesByName;
std::unordered_map<std::string, SamplerSlot> _allSamplersByName;

igl::NameHandle getBufferName(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName);

igl::NameHandle getBufferMemberName(const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName);
std::vector<std::pair<igl::NameHandle, igl::NameHandle>> getPossibleBufferAndMemberNames(
const igl::NameHandle& blockTypeName,
const igl::NameHandle& blockInstanceName,
const igl::NameHandle& memberName);

void setUniformBytes(const UniformDesc& uniformDesc,
const void* data,
Expand Down

0 comments on commit a14e09c

Please sign in to comment.