[Mesa-dev] [PATCH] i915: Fix gl_Fragcoord interpolation
Ian Romanick
idr at freedesktop.org
Wed Jun 21 22:13:22 UTC 2017
On 06/21/2017 10:38 AM, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> gl_FragCoord contains the window coordinates so it seems to me that
> we should not use perspective correct interpolation for it. At least
> now I get similar output as i965/swrast/llvmpipe produce.
>
> This fixes dEQP-GLES2.functional.shaders.builtin_variable.fragcoord_w.
> dEQP-GLES2.functional.shaders.builtin_variable.fragcoord_xyz was already
> passing, though I'm not quite sure how it managed to do that.
I suspect all the vertices had the same wrong w value, so the
interpolation just worked out.
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
One tiny comment below, but also
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Cc: mesa-stable at lists.freedesktop.org
> ---
> src/mesa/drivers/dri/i915/i915_context.h | 13 +++++++------
> src/mesa/drivers/dri/i915/i915_fragprog.c | 4 ++++
> src/mesa/drivers/dri/i915/i915_state.c | 7 ++++---
> src/mesa/drivers/dri/i915/i915_vtbl.c | 7 +------
> src/mesa/drivers/dri/i915/intel_reg.h | 3 ++-
> 5 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i915/i915_context.h b/src/mesa/drivers/dri/i915/i915_context.h
> index f95b531a413e..4e68d1193ca6 100644
> --- a/src/mesa/drivers/dri/i915/i915_context.h
> +++ b/src/mesa/drivers/dri/i915/i915_context.h
> @@ -79,12 +79,13 @@
> #define I915_CTXREG_STATE4 0
> #define I915_CTXREG_LI 1
> #define I915_CTXREG_LIS2 2
> -#define I915_CTXREG_LIS4 3
> -#define I915_CTXREG_LIS5 4
> -#define I915_CTXREG_LIS6 5
> -#define I915_CTXREG_BF_STENCIL_OPS 6
> -#define I915_CTXREG_BF_STENCIL_MASKS 7
> -#define I915_CTX_SETUP_SIZE 8
> +#define I915_CTXREG_LIS3 3
> +#define I915_CTXREG_LIS4 4
> +#define I915_CTXREG_LIS5 5
> +#define I915_CTXREG_LIS6 6
> +#define I915_CTXREG_BF_STENCIL_OPS 7
> +#define I915_CTXREG_BF_STENCIL_MASKS 8
> +#define I915_CTX_SETUP_SIZE 9
>
> #define I915_BLENDREG_IAB 0
> #define I915_BLENDREG_BLENDCOLOR0 1
> diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c b/src/mesa/drivers/dri/i915/i915_fragprog.c
> index 3657b2d82565..2e0431951217 100644
> --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
> +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
> @@ -1238,6 +1238,7 @@ i915ValidateFragmentProgram(struct i915_context *i915)
> const GLbitfield64 inputsRead = p->FragProg.info.inputs_read;
> GLuint s4 = i915->state.Ctx[I915_CTXREG_LIS4] & ~S4_VFMT_MASK;
> GLuint s2 = S2_TEXCOORD_NONE;
> + GLuint s3 = 0;
> int i, offset = 0;
>
> /* Important:
> @@ -1301,6 +1302,7 @@ i915ValidateFragmentProgram(struct i915_context *i915)
> */
> s2 &= ~S2_TEXCOORD_FMT(i, S2_TEXCOORD_FMT0_MASK);
> s2 |= S2_TEXCOORD_FMT(i, SZ_TO_HW(wpos_size));
> + s3 |= S3_TEXCOORD_PERSPECTIVE_DISABLE(i);
>
> intel->wpos_offset = offset;
> EMIT_PAD(wpos_size);
> @@ -1308,6 +1310,7 @@ i915ValidateFragmentProgram(struct i915_context *i915)
> }
>
> if (s2 != i915->state.Ctx[I915_CTXREG_LIS2] ||
> + s3 != i915->state.Ctx[I915_CTXREG_LIS3] ||
> s4 != i915->state.Ctx[I915_CTXREG_LIS4]) {
> I915_STATECHANGE(i915, I915_UPLOAD_CTX);
>
> @@ -1326,6 +1329,7 @@ i915ValidateFragmentProgram(struct i915_context *i915)
> intel->vertex_size >>= 2;
>
> i915->state.Ctx[I915_CTXREG_LIS2] = s2;
> + i915->state.Ctx[I915_CTXREG_LIS3] = s3;
> i915->state.Ctx[I915_CTXREG_LIS4] = s4;
>
> assert(intel->vtbl.check_vertex_size(intel, intel->vertex_size));
> diff --git a/src/mesa/drivers/dri/i915/i915_state.c b/src/mesa/drivers/dri/i915/i915_state.c
> index 715db1fffa3d..4c4d95c420a1 100644
> --- a/src/mesa/drivers/dri/i915/i915_state.c
> +++ b/src/mesa/drivers/dri/i915/i915_state.c
> @@ -925,11 +925,12 @@ i915_init_packets(struct i915_context *i915)
> * piece changes.
> */
> i915->state.Ctx[I915_CTXREG_LI] = (_3DSTATE_LOAD_STATE_IMMEDIATE_1 |
> - I1_LOAD_S(2) |
> - I1_LOAD_S(4) |
> - I1_LOAD_S(5) | I1_LOAD_S(6) | (3));
> + I1_LOAD_S(2) | I1_LOAD_S(3) |
> + I1_LOAD_S(4) | I1_LOAD_S(5) |
> + I1_LOAD_S(6) | (4));
> i915->state.Ctx[I915_CTXREG_LIS2] = 0;
> i915->state.Ctx[I915_CTXREG_LIS4] = 0;
> + i915->state.Ctx[I915_CTXREG_LIS3] = 0;
> i915->state.Ctx[I915_CTXREG_LIS5] = 0;
>
> if (i915->intel.ctx.Visual.rgbBits == 16)
> diff --git a/src/mesa/drivers/dri/i915/i915_vtbl.c b/src/mesa/drivers/dri/i915/i915_vtbl.c
> index c41cd37bcc23..6a0a121856d7 100644
> --- a/src/mesa/drivers/dri/i915/i915_vtbl.c
> +++ b/src/mesa/drivers/dri/i915/i915_vtbl.c
> @@ -176,7 +176,7 @@ i915_emit_invarient_state(struct intel_context *intel)
> {
> BATCH_LOCALS;
>
> - BEGIN_BATCH(17);
> + BEGIN_BATCH(15);
>
> OUT_BATCH(_3DSTATE_AA_CMD |
> AA_LINE_ECAAR_WIDTH_ENABLE |
> @@ -200,11 +200,6 @@ i915_emit_invarient_state(struct intel_context *intel)
> CSB_TCB(3, 3) |
> CSB_TCB(4, 4) | CSB_TCB(5, 5) | CSB_TCB(6, 6) | CSB_TCB(7, 7));
>
> - /* Need to initialize this to zero.
> - */
> - OUT_BATCH(_3DSTATE_LOAD_STATE_IMMEDIATE_1 | I1_LOAD_S(3) | (0));
> - OUT_BATCH(0);
> -
> OUT_BATCH(_3DSTATE_SCISSOR_RECT_0_CMD);
> OUT_BATCH(0);
> OUT_BATCH(0);
> diff --git a/src/mesa/drivers/dri/i915/intel_reg.h b/src/mesa/drivers/dri/i915/intel_reg.h
> index dabbd612ee04..0e482de84281 100644
> --- a/src/mesa/drivers/dri/i915/intel_reg.h
> +++ b/src/mesa/drivers/dri/i915/intel_reg.h
> @@ -93,7 +93,8 @@
> #define S2_TEX_COUNT_SHIFT_830 12
> #define S2_VERTEX_1_WIDTH_SHIFT_830 0
> #define S2_VERTEX_0_WIDTH_SHIFT_830 6
> -/* S3 not interesting */
> +
> +#define S3_TEXCOORD_PERSPECTIVE_DISABLE(unit) (1<<((unit)*4))
Might be worth adding the "wrap shortest" bits too in case someone ever
cares. I know what they do, but I'm not sure how it's useful. /shrug/
>
> #define S4_POINT_WIDTH_SHIFT 23
> #define S4_POINT_WIDTH_MASK (0x1ff<<23)
>
More information about the mesa-dev
mailing list