On Mon, 12 Mar 2012 16:04:00 +0800, Yuanhan Liu <yuanhan....@linux.intel.com>
wrote:
> When SPRITE_POINT_ENABLE bit is set, the texture coord would be
> replaced, and this is only needed when we called something like
> glTexEnvi(GL_POINT_SPRITE, GL_COORD_REPLACE, GL_TRUE).
>
> And more, we currently handle varying inputs as texture coord,
> we would be careful when setting this bit and set it just when
> needed, or you will find the value of varying input is not right
> and changed.
>
> Thus we do set SPRITE_POINT_ENABLE bit only when all enabled tex
> coord units need do CoordReplace. Or fallback is needed to make
> sure the rendering is right.
>
> As we need guarantee the CoordReplace changes handled well and
> be able to fallback when finding something wrong, I added another
> function to handle it at intelRunPipepline, where the drawing
> happened here and tnl pipeline hasn't started yet.
>
> With handling the bit setup at intel_validate_sprite_point_enable(),
> we don't need the relative code at i915Enable then.
>
> This patch would _really_ fix the webglc point-size.html test case and
> of course, not regress piglit point-sprite and glean-pointSprite testcase.
>
> NOTE: This is a candidate for stable release branches.
>
> v2: fallback just when all enabled tex coord units need do
> CoordReplace(Eric).
>
> Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com>
> ---
> src/mesa/drivers/dri/i915/i915_context.h | 1 +
> src/mesa/drivers/dri/i915/i915_state.c | 13 +-------
> src/mesa/drivers/dri/i915/intel_tris.c | 52
> ++++++++++++++++++++++++++++++
> 3 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i915/i915_context.h
> b/src/mesa/drivers/dri/i915/i915_context.h
> index 8167137..59eeb6e 100644
> --- a/src/mesa/drivers/dri/i915/i915_context.h
> +++ b/src/mesa/drivers/dri/i915/i915_context.h
> @@ -40,6 +40,7 @@
> #define I915_FALLBACK_POINT_SMOOTH 0x80000
> #define I915_FALLBACK_POINT_SPRITE_COORD_ORIGIN 0x100000
> #define I915_FALLBACK_DRAW_OFFSET 0x200000
> +#define I915_FALLBACK_COORD_REPLACE 0x400000
>
> #define I915_UPLOAD_CTX 0x1
> #define I915_UPLOAD_BUFFERS 0x2
> diff --git a/src/mesa/drivers/dri/i915/i915_state.c
> b/src/mesa/drivers/dri/i915/i915_state.c
> index 756001f..3c751e4 100644
> --- a/src/mesa/drivers/dri/i915/i915_state.c
> +++ b/src/mesa/drivers/dri/i915/i915_state.c
> @@ -869,18 +869,7 @@ i915Enable(struct gl_context * ctx, GLenum cap,
> GLboolean state)
> break;
>
> case GL_POINT_SPRITE:
> - /* This state change is handled in i915_reduced_primitive_state because
> - * the hardware bit should only be set when rendering points.
> - */
> - dw = i915->state.Ctx[I915_CTXREG_LIS4];
> - if (state)
> - dw |= S4_SPRITE_POINT_ENABLE;
> - else
> - dw &= ~S4_SPRITE_POINT_ENABLE;
> - if (dw != i915->state.Ctx[I915_CTXREG_LIS4]) {
> - i915->state.Ctx[I915_CTXREG_LIS4] = dw;
> - I915_STATECHANGE(i915, I915_UPLOAD_CTX);
> - }
> + /* Handle it at intel_validate_sprite_point_enable() */
> break;
>
> case GL_POINT_SMOOTH:
> diff --git a/src/mesa/drivers/dri/i915/intel_tris.c
> b/src/mesa/drivers/dri/i915/intel_tris.c
> index a36011a..58f6a59 100644
> --- a/src/mesa/drivers/dri/i915/intel_tris.c
> +++ b/src/mesa/drivers/dri/i915/intel_tris.c
> @@ -1052,6 +1052,48 @@ static const GLenum reduced_prim[GL_POLYGON + 1] = {
> GL_TRIANGLES
> };
>
> +static void
> +intel_validate_sprite_point_enable(struct intel_context *intel)
> +{
> + struct gl_context *ctx = &intel->ctx;
> + struct i915_fragment_program *p =
> + (struct i915_fragment_program *) ctx->FragmentProgram._Current;
> + const GLbitfield64 inputsRead = p->FragProg.Base.InputsRead;
> + struct i915_context *i915 = i915_context(ctx);
> + GLuint s4 = i915->state.Ctx[I915_CTXREG_LIS4] & ~S4_VFMT_MASK;
> + int i;
> + GLuint coord_replace_bits = 0x0;
> + GLuint tex_coord_unit_bits = 0x0;
> +
> + for (i = 0; i < ctx->Const.MaxTextureCoordUnits; i++) {
> + if (ctx->Point.CoordReplace[i] && ctx->Point.PointSprite)
> + coord_replace_bits |= (1 << i);
> + if (inputsRead & FRAG_BIT_TEX(i))
> + tex_coord_unit_bits |= (1 << i);
> + }
This ignores varyings assigned to texture coordinate inputs.
> +
> + s4 &= ~S4_SPRITE_POINT_ENABLE;
> + if (coord_replace_bits) {
> + if (coord_replace_bits != tex_coord_unit_bits) {
> + /*
> + * Here we can't enable the SPRITE_POINT_ENABLE bit due to the
> + * mis-match of tex_coord_unit_bits and coord_replace_bits, or
> + * this will make all the other non-point-sprite coords be
> + * replaced to value (0, 0)-(1, 1).
> + *
> + * Thus, a fallback is needed.
> + */
> + FALLBACK(intel, I915_FALLBACK_COORD_REPLACE, true);
Here, if you ever fall back for coordinate replacement, you'll never
exit software fallbacks.
> /**********************************************************************/
> /* High level hooks for t_vb_render.c */
> @@ -1070,6 +1112,15 @@ intelRunPipeline(struct gl_context * ctx)
> if (ctx->NewState)
> _mesa_update_state_locked(ctx);
>
> + /*
> + * Enable POINT_SPRITE_ENABLE bit when needed here
> + *
> + * Handle it at _draw_ time so that we can guarantee the CoordReplace
> + * changes handled well. And we must do it before the tnl pipeline is
> + * running so that we can fallback when finding something goes wrong.
> + */
> + intel_validate_sprite_point_enable(intel);
Other computed state happens in i915InvalidateState. Why does this one
go here? I'm also pretty sure this is also being called on i830.
pgpqaPU7nsgA0.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
