SetInlineConstants support for OpenGL #753
Conversation
hzqst
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…Failed to link program 0 for pipeline state 'Render State Cache Test': error: buffer block with binding `0' has mismatching definitions See DiligentGraphics#753
…Failed to link program 0 for pipeline state 'Render State Cache Test': error: buffer block with binding `0' has mismatching definitions See DiligentGraphics#753
| if (!Options.separate_shader_objects) | ||
| { | ||
| // We do not emit binding qualifiers when separate_shader_objects disable. | ||
| // Let OpenGL driver do the binding qualifiers allocation. | ||
| Options.version = 410; | ||
| Options.enable_420pack_extension = false; | ||
| } |
There was a problem hiding this comment.
Why is this needed for inline constants?
There was a problem hiding this comment.
There was a problem hiding this comment.
How to reproduce this issue?
I commented this out, but all tests still pass with both separable and non-separable programs.
There was a problem hiding this comment.
NVM, stale build files. Yes, it reproduces without this fix
There was a problem hiding this comment.
But why does this only fail with non-separable programs?
There was a problem hiding this comment.
I believe it was because there was no kind of VS-PS combination in test shaders that:
VS with references to cbufferA+cbufferB,
PS with references to only cbufferA.
until we introduced InlineConstantsTests.
There was a problem hiding this comment.
Can you create a unit test that reproduces this issue? The fix should be merged to main as it is not related to inline constants - can you submit a PR?
There was a problem hiding this comment.
The new unittest should be in ArchiveTest or ?
There was a problem hiding this comment.
ArchiveTest, or RenderStateCacheTest.
Maybe RenderStateCacheTest is better
Graphics/GraphicsEngineOpenGL/src/PipelineResourceSignatureGLImpl.cpp
Outdated
Show resolved
Hide resolved
Graphics/GraphicsEngineOpenGL/include/PipelineResourceSignatureGLImpl.hpp
Outdated
Show resolved
Hide resolved
Graphics/GraphicsEngineOpenGL/include/PipelineResourceSignatureGLImpl.hpp
Outdated
Show resolved
Hide resolved
Graphics/GraphicsEngineOpenGL/src/PipelineResourceSignatureGLImpl.cpp
Outdated
Show resolved
Hide resolved
Graphics/GraphicsEngineOpenGL/src/PipelineResourceSignatureGLImpl.cpp
Outdated
Show resolved
Hide resolved
Graphics/GraphicsEngineOpenGL/include/ShaderResourceCacheGL.hpp
Outdated
Show resolved
Hide resolved
|
Funny that it took a month to implement inline constants in Vulkan, and a day to implement them in GL |
|
@TheMostDiligent We have similar issue with Cross-Signature Commit in vulkan impl too, the GL impl has been fixed in f747b23 All reviews above have been fixed, except for the |
…Failed to link program 0 for pipeline state 'Render State Cache Test': error: buffer block with binding `0' has mismatching definitions See DiligentGraphics#753
TheMostDiligent
force-pushed
the
inline_constants_gl
branch
2 times, most recently
from
f6d3f6a to
2f82b00
Compare
TheMostDiligent
force-pushed
the
inline_constants
branch
from
61e2b46 to
380f498
Compare
TheMostDiligent
force-pushed
the
inline_constants_gl
branch
from
2f82b00 to
cf85a48
Compare
TheMostDiligent
force-pushed
the
inline_constants_gl
branch
from
cf85a48 to
820adc3
Compare
TheMostDiligent
merged commit 820adc3
into
DiligentGraphics:inline_constants
2 participants
Build plan: SetInlineConstantsGL
InlineConstantBufferAttribsGLstructureCreateLayout(useGetArraySize()) and create shared bufferShaderResourceCacheGLfor inline constant stagingShaderVariableManagerGL::SetConstantsDeviceContextGLImpl(graphics + compute)0has mismatching definitions fromRenderStateCachetest. see comment later.