On 2 July 2012 15:12, Vadim Girlin <<a href="mailto:vadimgirlin@gmail.com" target="_blank">vadimgirlin@gmail.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On Mon, 2012-07-02 at 14:19 -0700, Paul Berry wrote: > On 2 July 2012 13:45, Vadim Girlin <<a href="mailto:vadimgirlin@gmail.com">vadimgirlin@gmail.com</a>> wrote: > > > On Mon, 2012-07-02 at 10:08 -0700, Paul Berry wrote: > > > On 2 July 2012 08:04, Vadim Girlin <<a href="mailto:vadimgirlin@gmail.com">vadimgirlin@gmail.com</a>> wrote: > > > On Sun, 2012-06-24 at 11:18 +0200, Olivier Galibert wrote: > > > > That old bug was hidden but the clipper always interpolating > > > in 3d > > > > space no matter what it should have been doing. Now that > > > the > > > > interpolation has been fixed, the bug shows up. > > > > > > > > Fixes bugzilla 51364. > > > > > > > > Signed-off-by: Olivier Galibert <<a href="mailto:galibert@pobox.com">galibert@pobox.com</a>> > > > > > > > > diff --git a/src/mesa/state_tracker/st_program.c > > > b/src/mesa/state_tracker/st_program.c > > > > index e6664fb..9f98298 100644 > > > > --- a/src/mesa/state_tracker/st_program.c > > > > +++ b/src/mesa/state_tracker/st_program.c > > > > @@ -569,12 +569,12 @@ st_translate_fragment_program(struct > > > st_context *st, > > > > case FRAG_ATTRIB_CLIP_DIST0: > > > > input_semantic_name[slot] = > > > TGSI_SEMANTIC_CLIPDIST; > > > > input_semantic_index[slot] = 0; > > > > - interpMode[slot] = TGSI_INTERPOLATE_LINEAR; > > > > + interpMode[slot] = > > > TGSI_INTERPOLATE_PERSPECTIVE; > > > > break; > > > > case FRAG_ATTRIB_CLIP_DIST1: > > > > input_semantic_name[slot] = > > > TGSI_SEMANTIC_CLIPDIST; > > > > input_semantic_index[slot] = 1; > > > > - interpMode[slot] = TGSI_INTERPOLATE_LINEAR; > > > > + interpMode[slot] = > > > TGSI_INTERPOLATE_PERSPECTIVE; > > > > > > > > > At first glance this change doesn't seem correct, GLSL 1.30 > > > spec says > > > that clip distances are interpolated linearly. Am I missing > > > something? > > > > > > I think when the spec says "The clip distances will be linearly > > > interpolated across the primitive...", they mean "linear in 3D space". > > > Interpolating clip distances in screen space would produce really bad > > > artifacts, IMHO. > > > > Looking into it, I think you are right, sorry for the noise. > > > > Though then I have some doubts about the "fs-clip-distance-interpolated" > > test. AFAICS r600g in fact uses different interpolation for ClipDistance > > inputs with and without the patch, but the test does pass in both > > cases. > > > > Yeah, fs-clip-distance-interpolated only checks that the fragment shader > sees properly interpolated values for gl_ClipDistance. It doesn't check > that the implementation uses the correct form of interpolation to decide > where to clip the primitive. I suspect that's the reason the test passes > in both cases. I wasn't thinking carefully enough when I wrote the test to > realize that these two things ought to be tested separately. </div></div>AFAIK on r600 we have no control over the interpolation of the clip distances for clipping, so it should be always correct, this patch only affects the interpolation of the values that are passed to the fragment shader. </blockquote><div> Oh, ok. I misunderstood. Sandy Bridge and Ivy Bridge are similar. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> I think the test should draw something with perspective to check whether the interpolation is perspective-correct. </blockquote><div> Yeah, that makes sense. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div class="im"> > If you would like to add a piglit test to cover the case I missed, I would > be delighted :) </div>I'll try to find the time :) </blockquote></div>