[Mesa-dev] [PATCH] draw: fix clipping with linear interpolated values and gl_ClipVertex

[Jose Fonseca]

On 10/12/15 08:09, Dave Airlie wrote:
> On 10 December 2015 at 14:31,  <sroland at vmware.com> wrote:
>> From: Roland Scheidegger <sroland at 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 <airlied at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev