Open drywolf opened 2 weeks ago
I now added a stripped down version of the VUID-01795
error repro-code to the repository:
The original vsg-View-PushConst-bug.cpp code has some additional code for experimentation + it also contains a BugScenario
that seems to reveal a completely separate / different crash-bug in BindGraphicsPipeline::record()
how this ought to be configured/controlled from the C++ code needs to be discussed ?!
I imagine the most correct & least invasive way to fix this, would be to have some kind of meta-data/trait attached to the PushConstantRanges pushConstantRanges; stored in the vsg::PipelineLayout.
That way, this PushConstantRange meta-data/trait could then be handled in CommandBuffer::setCurrentPipelineLayout() in the same way as is already done for the _currentPushConstantStageFlags
.
And finally this meta-data/trait could be fetched during MatrixStack::record() and be used to decide if vkCmdPushConstants()
should be called for the projection/view-model matrices or not.
PS: Maybe the code that is currently in MatrixStack::record()
could even be pulled out of State.h
and this handling of "PushConstant-Traits" be made into a wider-reaching functionality (to make the handling of the projection/model-view matrices less deeply coupled to the core VSG code)
But this is more a question for future architecture improvements rather than fixing the above issue.
PSS: @robertosfield
In this code I noticed that only the first element of pushConstantRanges
is used.
I would have expected this to be checking/combining the stage-flags of all the pushConstantRanges in the layout, not just the first one ?!
Does this code work correctly when using multiple pushConstangRanges, or would this maybe also need fixing/a more complete implementation?
I'm not in position to look into this, but hopefully will be able to reproduce it and ponder on how best to resolve it in the next few weeks.
Describe the bug
TLDR: Minimal repro code https://github.com/drywolf/vsg-View-vkCmdPushConstants-bug (see vsg-View-PushConst-bug-minimal.cpp)
Some context:
sampler2D
and are writing to the bound vsg::Image attachmentsThe Bug:
Recently we have found that under some circumstances in our app, we are getting VUID-vkCmdPushConstants-offset-01795 Vulkan validation-errors. It took some debugging into the VulkanSceneGraph code itself to figure out what was causing it. We are rendering Fullscreen-Passes using pretty much the exact same VSG mechanisms & C++ classes that are otherwise used for the 3D scene rendering. Because of that, when VSG encounters our Fullscreen-Passes, it also calls the following code:
void RecordTraversal::apply(const Command& command)
(source)vsg::StateStack::record()
(source)MatrixStack::record()
(source)vkCmdPushConstants(commandBuffer, pipeline, stageFlags, offset, sizeof(newmatrix), newmatrix.data());
As long as our Fullscreen-Shaders did not use any Push-Constants, the check for
if (pipeline == 0 || stageFlags == 0)
prevented thevkCmdPushConstants()
call for the projection & model-view matrices from happening. But recently we added a Fullscreen-Pass that was using Push-Constants to pass a singlevsg::vec4
to the fullscreen-shader. In this case we started seeing theVUID-vkCmdPushConstants-offset-01795
validation error.PS: the
vsg::Commands
overload ofvoid RecordTraversal::apply()
is calling into exactly the same function-callstack as shown above. (it can lead to the exact same problem)To Reproduce
Steps to reproduce the behavior:
BugScenario
code invsg-View-PushConst-bug.cpp
vsg-View-PushConst-bug.exe
executableVUID-01795
in consoleExpected behavior
vkCmdPushConstants()
inMatrixStack::record()
should only be called when actively requested/configuredmat4
push-constants that are used specifically only for the VSG pbr/phong/flat shaders, should only be set when one of these shader-sets is currently active