Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run the reducer in an explicit Vulkan mode, rather than relying on whether uniform bindings are present #1046

Open
afd opened this issue Jul 30, 2020 · 0 comments
Labels
component:reducer The GLSL shader reducer (glsl-reduce)

Comments

@afd
Copy link
Contributor

afd commented Jul 30, 2020

Context:

Internally, GraphicsFuzz does not properly support uniform buffer blocks. When shaders are transformed - during generation or reduction - uniforms are in plain form, such as:

uniform float foo;

The GlslShaderJob class has a method, makeUniformBindings, to turn plain uniforms into uniform buffer blocks, and a method, removeUniformBindings, to go back to plain uniforms.

The reducer can be applied to a shader job that uses either plain uniforms, or that uses uniforms with buffer blocks. It deals with the latter by calling removeUniformBindings, working with the shader as if it used plain uniforms, and then calling makeUniformBindings right before emitting the shader again as text.

The problem:

A problem arises when a shader job has zero uniforms. In that case, the reducer doesn't detect any uniform bindings, so it treats the shader job as being a "plain uniforms" shader job.

This has not mattered so far, but the new --literals-to-uniforms option recently added to the reducer may well be used on a shader that initially has no uniforms, and we might want (and do in practice want) it to lead to uniform buffer blocks being added. With the current design of the tool, the right thing for --literals-to-uniforms to do is to add uniforms without buffer blocks / bindings, and to rely on the reducer invoking makeUniformBindings before emitting the shader. But if there were no uniforms to start with, the reducer won't make that call.

Short term fix:

For now - because Vulkan is our priority - we assume that uniform bindings are required in the case where a shader job has zero uniforms.

Long term fix:

Stop the reducer from inferring whether Vulkan-style bindings are required, and have it take a --vulkan argument - i.e., instruct the reducer as to when we need Vulkan-style uniforms.

Separate from this: we should ideally support uniform buffer blocks properly in the tool and get rid of makeUniformBindings and removeUniformBindings. But still we would need the --vulkan option to make it clear what to do when a shader job has no uniforms and we wish to add some.

@afd afd added the component:reducer The GLSL shader reducer (glsl-reduce) label Jul 30, 2020
afd added a commit that referenced this issue Jul 31, 2020
The LiteralToUniformReductionOpportunity adds new uniforms to a shader
job.  It should not provide these uniforms with bindings -- it is up
to the reducer to add bindings if they are deemed necessary, right
before emitting the shader as a text file.

This change fixes that issue.  However, it turns out that the reducer
was not adding bindings in the case that no uniforms were present in
the first place -- it was treating a uniform-free shader as a
non-Vulkan shader.  As a temporary workaround, a uniform-free shader
is now treated by the reducer as a Vulkan shader so that if uniforms
are then created, they get bindings when written out as text files.

Related issue to fix this more elegantly in the long-run: #1046.
paulthomson pushed a commit that referenced this issue Aug 4, 2020
The LiteralToUniformReductionOpportunity adds new uniforms to a shader
job.  It should not provide these uniforms with bindings -- it is up
to the reducer to add bindings if they are deemed necessary, right
before emitting the shader as a text file.

This change fixes that issue.  However, it turns out that the reducer
was not adding bindings in the case that no uniforms were present in
the first place -- it was treating a uniform-free shader as a
non-Vulkan shader.  As a temporary workaround, a uniform-free shader
is now treated by the reducer as a Vulkan shader so that if uniforms
are then created, they get bindings when written out as text files.

Related issue to fix this more elegantly in the long-run: #1046.
nipunG314 pushed a commit to nipunG314/graphicsfuzz that referenced this issue Aug 30, 2020
The LiteralToUniformReductionOpportunity adds new uniforms to a shader
job.  It should not provide these uniforms with bindings -- it is up
to the reducer to add bindings if they are deemed necessary, right
before emitting the shader as a text file.

This change fixes that issue.  However, it turns out that the reducer
was not adding bindings in the case that no uniforms were present in
the first place -- it was treating a uniform-free shader as a
non-Vulkan shader.  As a temporary workaround, a uniform-free shader
is now treated by the reducer as a Vulkan shader so that if uniforms
are then created, they get bindings when written out as text files.

Related issue to fix this more elegantly in the long-run: google#1046.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:reducer The GLSL shader reducer (glsl-reduce)
Projects
None yet
Development

No branches or pull requests

1 participant