On Sunday, February 11, 2018 6:26:39 PM PST Gustavo Lima Chaves wrote: > I've been seeking to add this extension support on my free time and > have now come to a point where some input could really help.
Hi Gustavo! Welcome to the Mesa community :) Thanks for taking a shot at this! > At vtn_get_builtin_location(), is that existing one used for the > extension what we want? Should we also check the shader stage we're in > there too, once it's meant for fragment shaders? I think your code to extend vtn_get_builtin_location looks fine. We'll see SpvDecorationBuiltIn, call that function, and your new code will handle SpvBuiltInFragStencilRefEXT, assigning it the location FRAG_RESULT_STENCIL, which is what we want. IIRC, SPIR-V implementations aren't required to handle errors (unlike GLSL) - they leave that to the validation layers. So, while you probably don't need to check the stage, it might be nice to assert(b->shader->info.stage == MESA_SHADER_FRAGMENT); just to be on the safe side. > The new ExecutionModeStencilRefReplacingEXT execution mode seems to > have no support in the code base yet. I'm still trying to figure out > what it should do in the meantime (the original > ARB_shader_stencil_export had no mentions of modes, I'm afraid). Right. You'll need to add support for it, as part of implementing this SPIR-V extension. What exactly it should do is a good question :) With GL_ARB_shader_stencil_export, #extension GL_ARB_shader_stencil_export: enable to a shader causes the compiler to declare a built-in output variable called gl_FragStencilRefARB. If the shader statically writes to gl_FragStencilRefARB, then the stencil value is taken from the shader. Otherwise, it comes from the OpenGL API specified value. It looks like Vulkan works a little differently. The VK spec says: FragStencilRefEXT Decorating a variable with the FragStencilRefEXT built-in decoration will make that variable contain the stencil reference value for all samples covered by the fragment. This value will be used as the stencil reference value used in stencil testing. To write to FragStencilRefEXT, a shader must declare the StencilRefReplacingEXT execution mode. If a shader declares the StencilRefReplacingEXT execution mode and there is an execution path through the shader that does not set FragStencilRefEXT, then the fragment’s stencil reference value is undefined for executions of the shader that take that path. [...] I believe that specifying the StencilRefReplacingEXT execution mode indicates "this shader will write stencil - use the shader's value rather than the API-specified one." This lets the compiler know ahead of time, without having to search through the code to see if it's statically written. It sounds like shaders are perhaps allowed to declare the FragStencilRefEXT built-in variable without the execution mode - they're just not allowed to write to it. That seems strange and useless... That got me thinking - what happens if you read it? Issue 1 of the ARB_shader_stencil_export spec makes it clear that you can't: 1) Should gl_FragStencilRefARB be initialized to the current stencil reference value on input to the fragment shader? RESOLVED: No. gl_FragStencilRefARB is write-only. If the current stencil reference value is required in a shader, the application should place it in a uniform. (If I recall correctly, outputs in GLSL are definitely readable, so I think what they mean is that the initial value is undefined. A shader could write a value, then read it back later...it just doesn't magically start with a meaningful value.) But the Vulkan wording, "will make that variable contain the stencil reference value", sounds a bit scary - like it might guarantee that. I doubt they intended that, though. My guess is that you'll want to extend vtn_handle_execution_mode() to make SpvExecutionModeStencilRefReplacingEXT set a "outputs stencil" flag in vtn_builder. Then, make apply_var_decoration() do case SpvBuiltinFragStencilRefEXT: if (!b->outputs_stencil) nir_var->data.read_only = true; break; to prevent writes to the variable when the execution mode isn't set. I'm not the most familiar with SPIR-V, so I copied Jason - he may be able to give a more authoritative answer. > When it comes to testing, I started simple with > https://github.com/glima/VK-GL-CTS/tree/stencil_export . The idea > there is just to assert that a custom value is really written to a > FragStencilRefEXT-decorated output variable. I'm still not positive > that I should go the createTestsForAllStages() path there--maybe > something like addTessCtrlTest() for fragment tests would be more > reasonable. I also know there are issues in the tests, but I hope > anyone more familiar with vtn_get_builtin_location() will be able to > help sort it out. Thanks for adding tests! Since you can only write FragStencilRefEXT from the fragment stage, I'm not sure what you would test in the other stages. In GLSL, you could write a negative test making sure it isn't possible to write gl_FragStencilRefARB from other stages...but with SPIR-V's lax error checking...there's probably nothing to do there. > BTW, when said tests come to a better/final shape, do people really > take pull requests for VK-GL-CTS from github? All commits in there > seem to have Google's gerrit annotations, so I'm confused about that. > > Thanks a lot. Khronos uses an internal Gerrit for accepting patches from member companies who've already signed contributor agreements. Being an Intel employee, you'll want to use that. I believe they only use Github for contributions from volunteers who aren't affiliated with a company. I'll send you a separate email with instructions on submitting CTS patches. --Ken
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list email@example.com https://lists.freedesktop.org/mailman/listinfo/mesa-dev