On 10/12/15 08:09, Dave Airlie wrote:
On 10 December 2015 at 14:31,  <srol...@vmware.com> wrote:
From: Roland Scheidegger <srol...@vmware.com>

Discovered this when working on other clip code, apparently didn't work
correctly - the combination of linear interpolated values and using
gl_ClipVertex produced wrong values (failing all such combinations
in piglits glsl-1.30 interpolation tests).
Use the pre-clip-pos values when determining the interpolation factor to
fix this.
Unfortunately I have no idea what I'm doing here really, but it fixes all
these failures in piglit (all interpolation-noperspective-XXX-vertex, 10
tests in total). Albeit piglit coverage of clipping isn't great, so hopefully
someone can confirm this actually makes sense, and wouldn't cause failures
elsewhere...

This makes sense to me, in that interpolating should definitely happen
on pre-clipped coordinates.

The code was added in


http://cgit.freedesktop.org/mesa/mesa/commit/?id=4625a9b1adf7a30c56e2bbeb41573fbba4465851

then revised in


http://cgit.freedesktop.org/mesa/mesa/commit/?id=ab74fee5e1a3fc3323b7238278637b232c2d0d95

and


http://cgit.freedesktop.org/mesa/mesa/commit/?id=5da967aff5adb3e27954488206fb885ea1ede0fd

The 2nd change of Brian seems (if I read correctly) to do precisely the opposite.

I wonder if "clip" and "pre_clip_pos".


The relevant tests seem to be:

GALLIUM_DRIVER=softpipe ./bin/shader_runner generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-noperspective-gl_BackColor-flat-fixed.shader_test -auto

  GALLIUM_DRIVER=softpipe ./bin/fbo-blit-stretch -auto


Jose


Though like you my knowledge of this code is debug only.

So,
Reviewed-by: Dave Airlie <airl...@redhat.com>
---
  src/gallium/auxiliary/draw/draw_pipe_clip.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c 
b/src/gallium/auxiliary/draw/draw_pipe_clip.c
index f2b56b0..7f22eef 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
@@ -192,11 +192,11 @@ static void interp(const struct clip_stage *clip,
        t_nopersp = t;
        /* find either in.x != out.x or in.y != out.y */
        for (k = 0; k < 2; k++) {
-         if (in->clip[k] != out->clip[k]) {
+         if (in->pre_clip_pos[k] != out->pre_clip_pos[k]) {
              /* do divide by W, then compute linear interpolation factor */
-            float in_coord = in->clip[k] / in->clip[3];
-            float out_coord = out->clip[k] / out->clip[3];
-            float dst_coord = dst->clip[k] / dst->clip[3];
+            float in_coord = in->pre_clip_pos[k] / in->pre_clip_pos[3];
+            float out_coord = out->pre_clip_pos[k] / out->pre_clip_pos[3];
+            float dst_coord = dst->pre_clip_pos[k] / dst->pre_clip_pos[3];
              t_nopersp = (dst_coord - out_coord) / (in_coord - out_coord);
              break;
           }
--
2.1.4

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to