i965: Fix point size with tessellation/geometry shaders in GLES.
authorKenneth Graunke <kenneth@whitecape.org>
Thu, 2 Jun 2016 00:32:55 +0000 (17:32 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Wed, 22 Jun 2016 19:22:50 +0000 (12:22 -0700)
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 <kenneth@whitecape.org>
Acked-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/mesa/drivers/dri/i965/brw_state.h
src/mesa/drivers/dri/i965/gen6_sf_state.c
src/mesa/drivers/dri/i965/gen7_sf_state.c
src/mesa/drivers/dri/i965/gen8_sf_state.c

index 0a4c21f8f53b2e557617857c3c5522041158ced7..be7f6ce28b13d23eb1234b75a657b64153c7495a 100644 (file)
@@ -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
 }
index 32dd8c49150f09a069b07ee119de77516a9004e8..f70305c174b7a60db7b62c87b2c22863e8cdb945 100644 (file)
@@ -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 */
index d3a658c024f2383333bb2b2637ea823d532c7af1..8d49e249df1950ad757a779cc2ee9483271b0589 100644 (file)
@@ -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,
 };
index d854b6dd47d57e01dbe3db148517ae7bbf04ba7e..0c4f1df6a8d271ec0b08fc6aa61aac0ef27240b0 100644 (file)
@@ -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,
 };