[Mesa-dev] [RFC 0/2] VK_EXT_shader_stencil_export

[Kenneth Graunke]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180214/8759f0af/attachment.sig>