Am 10.12.2015 um 17:06 schrieb 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, <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? > > Ah, if these things are really what they sounds like. I guess they aren't, both are really clip space coords, just one of them containing what was in clipVertex instead of position. Albeit I wonder if we couldn't eliminate one of them. If I see that right, we only need two if we have clipVertex. But, in this case, I think we could take that data directly from the "normal" vertex shader clipVertex output. We can't do that for ordinary pos, since at least for the llvm case we put the viewport-transformed data there - which is actually a hack causing problems since all stages which technically come after vs but before viewport transform need special code to deal with that (though that's mostly only clip itself and streamout). But, for clipVertex output, we don't really do anything special with it, so I can't see why we couldn't pull that data from there. I think that would clean things up. (Technically, things would actually be a LOT cleaner if viewport transform had its own stage, but it's of course convenient to place it at the end of the vs, since you can then do it cheaply in SoA format without having to really reload the output values, albeit it's a bit ridiculous if you have things like geometry shaders where it needs to be done manually afterwards anyway as we can't do that there, and of course it needs to be redone if things are clipped in any case.)
> >> >> 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. Looks like the non-llvm paths did all the same luckily... Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
