[Piglit] [PATCH 1/2] arb_shader_stencil_export: move to standard location

[Ben Widawsky]

On Tue, Nov 10, 2015 at 04:37:09PM +0000, Emil Velikov wrote:
> Hi Ben,
> 
> Please feed these through -M so that git can detect the code movement.
> Otherwise people start running away :P

What does -M do? I am not seeing such a flag in any of the relevant git
subcommands (add, rm, commit, mv). I thought git detects moves automatically,
which is what it sounds like you're getting at.

> 
> But seriously, just a small comment below.
> 
> On 23 October 2015 at 06:00, Ben Widawsky <benjamin.widawsky at intel.com> wrote:
> 
> > --- /dev/null
> > +++ b/tests/spec/arb_shader_stencil_export/glsl-fs-shader-stencil-export.c
> 
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > +       GLboolean pass = GL_TRUE;
> > +       float p[4];
> > +
> > +       glClearColor(0.5, 0.5, 0.5, 0.5);
> > +       glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT);
> > +
> > +       glEnable(GL_STENCIL_TEST);
> > +       glStencilOp(GL_REPLACE, GL_REPLACE, GL_REPLACE);
> > +
> > +       piglit_draw_rect(-1, -1, 2, 2);
> > +
> > +       glReadPixels(0, 0, 3, 1, GL_STENCIL_INDEX, GL_FLOAT, p);
> 
> The original test uses rect(0, 0 piglit_width, piglit_height) +
> ortho_projection. As you've changed the former and dropped the latter
> the glReadPixels() should be updated as well ?
> 

I apologize for leaving the ortho change out of the commit message. It really
should have been in there...

I admit incompetence here, but I didn't think glReadPixels needs to be updated.
Maybe I need a glViewport call... Could you enlighten me as to what it should
be?

> You'd also need to update all.py. Currently it will attempt to run the
> old, missing test. Something like
> 
> diff --git a/tests/all.py b/tests/all.py
> index c28ae2b..03ee52e 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -508,7 +508,6 @@ with profile.group_manager(PiglitGLTest, 'shaders') as g:
>     g(['glsl-fs-pointcoord'])
>     g(['glsl-fs-raytrace-bug27060'])
>     g(['glsl-fs-sampler-numbering'])
> -    g(['glsl-fs-shader-stencil-export'])
>     g(['glsl-fs-sqrt-branch'])
>     g(['glsl-fs-texturecube'])
>     g(['glsl-fs-texturecube', '-bias'], 'glsl-fs-texturecube-bias')
> @@ -2551,6 +2550,10 @@ with profile.group_manager(
>         'GL_DEPTH32F_STENCIL8', 0)
> 
> with profile.group_manager(
> +        PiglitGLTest, grouptools.join('spec',
> 'arb_shader_stencil_export')) as g:
> +    g(['glsl-fs-shader-stencil-export'])
> +
> +with profile.group_manager(

Sure. I can do this. Not sure how useful it is with just the one test, but the
plan is to add more.

> 
> On the really nitpicky side one can rename the test to something
> better, but I'm out of ideas :-\

Eventually if I get a chance to add more tests, I will do that, but for now,
laziness wins.

> 
> With this (and the similar comment for 2/2) the batch is
> Reviewed-by: Emil Velikov <emil.velikov at collabora.co.uk>
> 
> -Emil
-- 
Ben Widawsky, Intel Open Source Technology Center