i965: Fix point size with tessellation/geometry shaders in GLES.

Our previous code worked for desktop GL, and ES without geometry or
tessellation shaders.  But those features require fancier point size
handling.  Fortunately, we can use one rule for all APIs.

Fixes a number of dEQP tests with EXT_tessellation_shader enabled:
dEQP-GLES31.functional.tessellation_geometry_interaction.point_size.*

Signed-off-by: Kenneth Graunke <[email protected]>
Acked-by: Ilia Mirkin <[email protected]>
Reviewed-by: Jason Ekstrand <[email protected]>

4 files changed, 59 insertions, 9 deletions

diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index 0a4c21f8f53..be7f6ce28b1 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -473,6 +473,55 @@ is_drawing_lines(const struct brw_context *brw)
    return false;
 }
 
+static inline bool
+use_state_point_size(const struct brw_context *brw)
+{
+   const struct gl_context *ctx = &brw->ctx;
+
+   /* Section 14.4 (Points) of the OpenGL 4.5 specification says:
+    *
+    *    "If program point size mode is enabled, the derived point size is
+    *     taken from the (potentially clipped) shader built-in gl_PointSize
+    *     written by:
+    *
+    *        * the geometry shader, if active;
+    *        * the tessellation evaluation shader, if active and no
+    *          geometry shader is active;
+    *        * the vertex shader, otherwise
+    *
+    *    and clamped to the implementation-dependent point size range.  If
+    *    the value written to gl_PointSize is less than or equal to zero,
+    *    or if no value was written to gl_PointSize, results are undefined.
+    *    If program point size mode is disabled, the derived point size is
+    *    specified with the command
+    *
+    *       void PointSize(float size);
+    *
+    *    size specifies the requested size of a point.  The default value
+    *    is 1.0."
+    *
+    * The rules for GLES come from the ES 3.2, OES_geometry_point_size, and
+    * OES_tessellation_point_size specifications.  To summarize: if the last
+    * stage before rasterization is a GS or TES, then use gl_PointSize from
+    * the shader if written.  Otherwise, use 1.0.  If the last stage is a
+    * vertex shader, use gl_PointSize, or it is undefined.
+    *
+    * We can combine these rules into a single condition for both APIs.
+    * Using the state point size when the last shader stage doesn't write
+    * gl_PointSize satisfies GL's requirements, as it's undefined.  Because
+    * ES doesn't have a PointSize() command, the state point size will
+    * remain 1.0, satisfying the ES default value in the GS/TES case, and
+    * the VS case (1.0 works for "undefined").  Mesa sets the program point
+    * mode flag to always-enabled in ES, so we can safely check that, and
+    * it'll be ignored for ES.
+    *
+    * _NEW_PROGRAM | _NEW_POINT
+    * BRW_NEW_VUE_MAP_GEOM_OUT
+    */
+   return (!ctx->VertexProgram.PointSizeEnabled && !ctx->Point._Attenuated) ||
+          (brw->vue_map_geom_out.slots_valid & VARYING_BIT_PSIZ) == 0;
+}
+
 
 #ifdef __cplusplus
 }
diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index 32dd8c49150..f70305c174b 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -373,9 +373,8 @@ upload_sf_state(struct brw_context *brw)
    if (multisampled_fbo && ctx->Multisample.Enabled)
       dw3 |= GEN6_SF_MSRAST_ON_PATTERN;
 
-   /* _NEW_PROGRAM | _NEW_POINT */
-   if (!(ctx->VertexProgram.PointSizeEnabled ||
-	 ctx->Point._Attenuated))
+   /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */
+   if (use_state_point_size(brw))
       dw4 |= GEN6_SF_USE_STATE_POINT_WIDTH;
 
    /* _NEW_POINT - Clamp to ARB_point_parameters user limits */
diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index d3a658c024f..8d49e249df1 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -213,8 +213,8 @@ upload_sf_state(struct brw_context *brw)
 
    dw3 = GEN6_SF_LINE_AA_MODE_TRUE;
 
-   /* _NEW_PROGRAM | _NEW_POINT */
-   if (!(ctx->VertexProgram.PointSizeEnabled || ctx->Point._Attenuated))
+   /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */
+   if (use_state_point_size(brw))
       dw3 |= GEN6_SF_USE_STATE_POINT_WIDTH;
 
    /* _NEW_POINT - Clamp to ARB_point_parameters user limits */
@@ -256,7 +256,8 @@ const struct brw_tracked_state gen7_sf_state = {
                _NEW_SCISSOR,
       .brw   = BRW_NEW_BLORP |
                BRW_NEW_CONTEXT |
-               BRW_NEW_PRIMITIVE,
+               BRW_NEW_PRIMITIVE |
+               BRW_NEW_VUE_MAP_GEOM_OUT,
    },
    .emit = upload_sf_state,
 };
diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c b/src/mesa/drivers/dri/i965/gen8_sf_state.c
index d854b6dd47d..0c4f1df6a8d 100644
--- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
@@ -172,8 +172,8 @@ upload_sf(struct brw_context *brw)
    /* Clamp to the hardware limits and convert to fixed point */
    dw3 |= U_FIXED(CLAMP(point_size, 0.125f, 255.875f), 3);
 
-   /* _NEW_PROGRAM | _NEW_POINT */
-   if (!(ctx->VertexProgram.PointSizeEnabled || ctx->Point._Attenuated))
+   /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */
+   if (use_state_point_size(brw))
       dw3 |= GEN6_SF_USE_STATE_POINT_WIDTH;
 
    /* _NEW_POINT | _NEW_MULTISAMPLE */
@@ -209,7 +209,8 @@ const struct brw_tracked_state gen8_sf_state = {
                _NEW_MULTISAMPLE |
                _NEW_POINT,
       .brw   = BRW_NEW_BLORP |
-               BRW_NEW_CONTEXT,
+               BRW_NEW_CONTEXT |
+               BRW_NEW_VUE_MAP_GEOM_OUT,
    },
    .emit = upload_sf,
 };