Hi Kenneth,
Am 03.06.2016 um 10:08 schrieb Kenneth Graunke: > 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 <imir...@alum.mit.edu> > Signed-off-by: Kenneth Graunke <kenn...@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; That last condition looks decidedly weird. Are those brackets correct? --Michael > +} > + > > #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, > }; > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev