Commit e8eb5e80 authored by Ville Syrjälä's avatar Ville Syrjälä Committed by Andres Gomez

i915: Fix wpos_tex vs. -1 comparison

wpos_tex used to be a GLuint so assigning -1 to it and
later comparing with -1 worked correctly, but commit
c349031c ("i915: Fix texcoord vs. varying collision in
fragment programs") changed wpos_tex to uint8_t and hence
broke the comparison. To fix this define a more explicit
invalid value for wpos_tex.

gcc warns us:
i915_fragprog.c:1255:57: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    if (inputsRead & VARYING_BITS_TEX_ANY || p->wpos_tex != -1) {
                                                         ^

And clang says:
i915_fragprog.c:1255:57: warning: comparison of constant -1 with expression of type 'uint8_t' (aka 'unsigned char') is always true [-Wtautological-constant-out-of-range-compare]
   if (inputsRead & VARYING_BITS_TEX_ANY || p->wpos_tex != -1) {
                                            ~~~~~~~~~~~ ^  ~~

Cc: Chih-Wei Huang <cwhuang@android-x86.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: Ian Romanick <ian.d.romanick@intel.com>
Cc: mesa-stable@lists.freedesktop.org
Fixes: c349031c ("i915: Fix texcoord vs. varying collision in fragment programs")
Signed-off-by: Ville Syrjälä's avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarEmil Velikov <emil.velikov@collabora.com>
(cherry picked from commit c1eedb43)

Squashed with commit:

i915: Always emit W on gen3

Unlike the older gen2 hardware, gen3 performs perspective
correct interpolation even for the primary/secondary colors.
To do that it naturally needs us to emit W for the vertices.

Currently we emit W only when at least one texture coordinate
set gets emitted. This means the interpolation of color will
change depending on whether texcoords/varyings are used or not.
That's probably not what anyone would expect, so let's just
always emit W to get consistent behaviour. Trying to avoid
emitting W seems like more hassle than it's worth, especially
as bspec seems to suggest that the hardware will perform the
perspective division anyway.

This used to be broken until it was accidentally fixed it in
commit c349031c ("i915: Fix texcoord vs. varying collision
in fragment programs") by introducing a bug that made the driver
always emit W. After fixing that bug in commit c1eedb43
("i915: Fix wpos_tex vs. -1 comparison") we went back to the
old behaviour and caused an apparent regression.

Fixes: c1eedb43 ("i915: Fix wpos_tex vs. -1 comparison")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101451Signed-off-by: Ville Syrjälä's avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarIan Romanick <ian.d.romanick@intel.com>
(cherry picked from commit 0eef03a6)
parent 62cbd6d1
......@@ -117,6 +117,7 @@ enum {
};
#define I915_TEX_UNITS 8
#define I915_WPOS_TEX_INVALID 0xff
#define I915_MAX_CONSTANT 32
#define I915_CONSTANT_SIZE (2+(4*I915_MAX_CONSTANT))
......
......@@ -1063,7 +1063,7 @@ check_wpos(struct i915_fragment_program *p)
GLint i;
unsigned unit = 0;
p->wpos_tex = -1;
p->wpos_tex = I915_WPOS_TEX_INVALID;
if ((inputs & VARYING_BIT_POS) == 0)
return;
......@@ -1253,12 +1253,10 @@ i915ValidateFragmentProgram(struct i915_context *i915)
intel->coloroffset = 0;
intel->specoffset = 0;
if (inputsRead & VARYING_BITS_TEX_ANY || p->wpos_tex != -1) {
EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, S4_VFMT_XYZW, 16);
}
else {
EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_3F_VIEWPORT, S4_VFMT_XYZ, 12);
}
/* Always emit W to get consistent perspective
* correct interpolation of primary/secondary colors.
*/
EMIT_ATTR(_TNL_ATTRIB_POS, EMIT_4F_VIEWPORT, S4_VFMT_XYZW, 16);
/* Handle gl_PointSize builtin var here */
if (ctx->Point._Attenuated || ctx->VertexProgram.PointSizeEnabled)
......
......@@ -482,7 +482,7 @@ i915_init_program(struct i915_context *i915, struct i915_fragment_program *p)
p->decl_t = 0;
p->temp_flag = 0xffff000;
p->utemp_flag = ~0x7;
p->wpos_tex = -1;
p->wpos_tex = I915_WPOS_TEX_INVALID;
p->depth_written = 0;
p->nr_params = 0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment