[Mesa-dev] [PATCH] anv: Default PointSize to 1.0 if not written by the shader

[Kenneth Graunke]

On Friday, January 13, 2017 9:41:58 AM PST Jason Ekstrand wrote:
> The Vulkan rules for point size are a bit whacky.  If you only have a
> vertex shader and you use points, then you must write PointSize in your
> vertex shader.  If you have a geometry or tessellation shader, then it's
> dependent on the shaderTessellationAndGeometryPointSize device feature.
> From the Vulkan 1.0.38 specification:
> 
>    "shaderTessellationAndGeometryPointSize indicates whether the
>    PointSize built-in decoration is available in the tessellation
>    control, tessellation evaluation, and geometry shader stages. If this
>    feature is not enabled, members decorated with the PointSize built-in
>    decoration must not be read from or written to and all points written
>    from a tessellation or geometry shader will have a size of 1.0. This
>    also indicates whether shader modules can declare the
>    TessellationPointSize capability for tessellation control and
>    evaluation shaders, or if the shader modules can declare the
>    GeometryPointSize capability for geometry shaders. An implementation
>    supporting this feature must also support one or both of the
>    tessellationShader or geometryShader features."
> 
> In other words, if the feature is disbled (the client can disable
> features!) then they don't write PointSize and we provide a 1.0 default
> but if the feature is enabled, they do write PointSize and we use the
> one they wrote in the shader.  There are at least two valid ways we can
> implement this:
> 
>  1) Track whether or not shaderTessellationAndGeometryPointSize is
>     enabled and set the 3DSTATE_SF bits based on that and what stages
>     are enabled, ignoring the shader source.
> 
>  2) Just look at the last geometry stage VUE map and see if they wrote
>     PointSize and set the 3DSTATE_SF accordingly.
> 
> The second solution is the easiest and the most robust against invalid
> usage of the Vulkan API, so we choose to go with that one.
> 
> This fixes all of the dEQP-VK.tessellation.primitive_discard.*point_mode
> tests.  The tests are also broken because they unconditionally enable
> shaderTessellationAndGeometryPointSize if it's supported by the
> implementation and then don't write PointSize in the evaluation shader.
> However, since this is the "robust against invalid API usage" solution,
> the tests happily pass. :-)
> ---
>  src/intel/vulkan/genX_pipeline.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
> index 7fa68c0..a537a40 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -420,8 +420,16 @@ emit_rs_state(struct anv_pipeline *pipeline,
>     sf.TriangleStripListProvokingVertexSelect = 0;
>     sf.LineStripListProvokingVertexSelect = 0;
>     sf.TriangleFanProvokingVertexSelect = 1;
> -   sf.PointWidthSource = Vertex;
> -   sf.PointWidth = 1.0;
> +
> +   const struct brw_vue_prog_data *last_vue_prog_data =
> +      anv_pipeline_get_last_vue_prog_data(pipeline);
> +
> +   if (last_vue_prog_data->vue_map.slots_valid & VARYING_BIT_PSIZ) {
> +      sf.PointWidthSource = Vertex;
> +   } else {
> +      sf.PointWidthSource = State;
> +      sf.PointWidth = 1.0;
> +   }
>  
>  #if GEN_GEN >= 8
>     struct GENX(3DSTATE_RASTER) raster = {
> 

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- 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/20170113/86aefc15/attachment-0001.sig>