[Mesa-dev] [PATCH v2] i965: Fix point size with tessellation/geometry shaders in GLES.
Kenneth Graunke
kenneth at whitecape.org
Fri Jun 3 08:08:09 UTC 2016
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.*
Cc: Ilia Mirkin <imirkin at alum.mit.edu>
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
src/mesa/drivers/dri/i965/brw_state.h | 49 +++++++++++++++++++++++++++++++
src/mesa/drivers/dri/i965/gen6_sf_state.c | 5 ++--
src/mesa/drivers/dri/i965/gen7_sf_state.c | 7 +++--
src/mesa/drivers/dri/i965/gen8_sf_state.c | 7 +++--
4 files changed, 59 insertions(+), 9 deletions(-)
Hey Ilia,
Thanks for helping me figure out these rules on IRC yesterday.
I think this version should work...
--Ken
diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index 0a4c21f..be7f6ce 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 0538ab7..94731e0 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 d3a658c..8d49e24 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 d854b6d..0c4f1df 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,
};
--
2.8.3
More information about the mesa-dev
mailing list