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

[Jose Fonseca]

On 10/12/15 15:53, Roland Scheidegger wrote:
> 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,  <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".
>
> What do you wonder?

Ah, if these things are really what they sounds like.

>
> 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).

Right.  The key point here is to check that those tests don't regress 
with softpipe.

Jose