Re: [Mesa-dev] [PATCH] anv: Default PointSize to 1.0 if not written by the shader
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
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: Default PointSize to 1.0 if not written by the shader
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 = {
--
2.5.0.400.gff86faf
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
