Am 10.12.2015 um 15:44 schrieb Jose Fonseca:
> 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".
What do you wonder?
I'm thinking there could be a mismatch what softpipe and llvmpipe do
here (rather, the llvm vs. non-llvm path). In particular, draw_llvm will
always write to both clip pos and pre-clip-pos, if there wasn't a a
clip_user it will just write the same (look at invocation of store_clip).
>
>
> 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
>
They still seem to pass even with softpipe (and this patch also fixes the
same bug as it did with llvmpipe, i.e.
generated_tests/spec/glsl-1.30/execution/interpolation/interpolation-noperspective-gl_BackColor-flat-vertex.shader_test
and similar.
So maybe non-llvm and llvm path are doing the same these days.
And I suppose using the pos data out of the pos_attr output vs. the one
from clip vs. pre-clip-pos isn't quite doing the same.
I'll do a full piglit run with softpipe too...
Roland
> 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
