[Piglit] [PATCH] Rework the PIGLIT_GL_VISUAL flags, fix RGB vs RGBA vs ALPHA confusion

[Chad Versace]

The conversion code below looks good to me. If that is added to your original patch,
and removed is the comment above `enum piglit_gl_visual` that reads
    * Each enum has the same value of its corresponding GLUT enum. That is, for
    * each X, `PIGLIT_GL_VISUAL_X == GLUT_X`.

Then the patch has my
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>

On 04/13/2013 01:01 AM, Marek Olšák wrote:> OK, but I would like to commit this conversion code instead of hacking the bits:
 >
 > diff --git a/tests/util/piglit-framework-gl/piglit_glut_framework.c 
b/tests/util/piglit-framework-gl/piglit_glut_framework.c
 > index ee5ac01..cfadf77 100644
 > --- a/tests/util/piglit-framework-gl/piglit_glut_framework.c
 > +++ b/tests/util/piglit-framework-gl/piglit_glut_framework.c
 > @@ -98,12 +98,27 @@ init_glut(void)
 >          const struct piglit_gl_test_config *test_config = glut_fw.gl_fw.test_config;
 >          char *argv[] = {"piglit"};
 >          int argc = 1;
 > +       unsigned flags = GLUT_RGB;
 > +
 > +       if (test_config->window_visual & PIGLIT_GL_VISUAL_RGBA)
 > +               flags |= GLUT_ALPHA;
 > +       if (test_config->window_visual & PIGLIT_GL_VISUAL_DEPTH)
 > +               flags |= GLUT_DEPTH;
 > +       if (test_config->window_visual & PIGLIT_GL_VISUAL_STENCIL)
 > +               flags |= GLUT_STENCIL;
 > +       if (test_config->window_visual & PIGLIT_GL_VISUAL_ACCUM)
 > +               flags |= GLUT_ACCUM;
 > +
 > +       if (test_config->window_visual & PIGLIT_GL_VISUAL_DOUBLE)
 > +               flags |= GLUT_DOUBLE;
 > +       else
 > +               flags |= GLUT_SINGLE;
 >
 >          glutInit(&argc, argv);
 >          glutInitWindowPosition(0, 0);
 >          glutInitWindowSize(test_config->window_width,
 >                             test_config->window_height);
 > -       glutInitDisplayMode(test_config->window_visual);
 > +       glutInitDisplayMode(flags);
 >          glut_fw.window = glutCreateWindow("Piglit");
 >
 >          glutDisplayFunc(display);
 >
 >
 > Marek
 >
 >
 >
 > On Tue, Apr 2, 2013 at 7:16 PM, Chad Versace <chad.versace at linux.intel.com <mailto:chad.versace at linux.intel.com>> wrote:
 >
 >     On Tue, Mar 26, 2013 at 06:13:59PM +0100, Marek Olšák wrote:
 >      > Hi,
 >      >
 >      > would anyone like to comment on this? Or is it still stuck in the
 >      > moderation queue?
 >
 >     The hunk below will break Piglit on Windows and Mac. Comments below.
 >
 >      > diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
 >      > index bc3a3cd..f6ad839 100644
 >      > --- a/tests/util/piglit-framework-gl.h
 >      > +++ b/tests/util/piglit-framework-gl.h
 >      > @@ -34,25 +34,15 @@
 >      >   * Each enum has the same value of its corresponding GLUT enum. That is, for
 >      >   * each X, `PIGLIT_GL_VISUAL_X == GLUT_X`.
 >      >   *
 >      > - * Note that PIGLIT_GL_VISUAL_RGBA is an alias for PIGLIT_GL_VISUAL_RGB and is
 >      > - * always selected. From the documentation of glutInitDisplayMode in
 >      > - * Kilgard's GLUT:
 >      > - *
 >      > - * Note that GLUT_RGBA selects the RGBA color model, but it does not
 >      > - * request any bits of alpha (sometimes called an alpha buffer or
 >      > - * destination alpha) be allocated. To request alpha, specify GLUT_ALPHA.
 >      > - *
 >      >   * \see piglit_gl_test_config::window_visual
 >      >   */
 >      >   enum piglit_gl_visual {
 >      > -     PIGLIT_GL_VISUAL_RGB = 0,
 >      > -     PIGLIT_GL_VISUAL_RGBA = 0,
 >      > -     PIGLIT_GL_VISUAL_SINGLE = 0,
 >      > -     PIGLIT_GL_VISUAL_DOUBLE = 1 << 1,
 >      > -     PIGLIT_GL_VISUAL_ACCUM = 1 << 2,
 >      > -     PIGLIT_GL_VISUAL_ALPHA = 1 << 3,
 >      > +     PIGLIT_GL_VISUAL_RGB = 1 << 0,
 >      > +     PIGLIT_GL_VISUAL_RGBA = 1 << 1,
 >      > +     PIGLIT_GL_VISUAL_DOUBLE = 1 << 2,
 >      > +     PIGLIT_GL_VISUAL_ACCUM = 1 << 3,
 >      >       PIGLIT_GL_VISUAL_DEPTH = 1 << 4,
 >      > -     PIGLIT_GL_VISUAL_STENCIL = 1 << 5,
 >      > +     PIGLIT_GL_VISUAL_STENCIL = 1 << 5
 >      > };
 >      > /**
 >
 >     Currently, Waffle is used only on Linux. Mac and Windows still use GLUT. So,
 >     any change in the PIGLIT_GL_VISUAL values must be compatible with GLUT.
 >
 >     I think the hunk below won't regress GLUT users, but I haven't tested it.
 >
 >     diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
 >        enum piglit_gl_visual {
 >            PIGLIT_GL_VISUAL_RGB      = 0,
 >     -     PIGLIT_GL_VISUAL_RGBA     = 0,
 >     -     PIGLIT_GL_VISUAL_SINGLE   = 0,
 >            PIGLIT_GL_VISUAL_DOUBLE   = 1 << 1,
 >            PIGLIT_GL_VISUAL_ACCUM    = 1 << 2,
 >     -     PIGLIT_GL_VISUAL_ALPHA    = 1 << 3,
 >     +     PIGLIT_GL_VISUAL_RGBA     = 1 << 3,
 >            PIGLIT_GL_VISUAL_DEPTH    = 1 << 4,
 >            PIGLIT_GL_VISUAL_STENCIL  = 1 << 5,
 >
 >
 >
 >
 >
 >
 > _______________________________________________
 > Piglit mailing list
 > Piglit at lists.freedesktop.org
 > http://lists.freedesktop.org/mailman/listinfo/piglit
 >