[Piglit] [PATCH v2] miptree: test ability to use upside down miptree

[andrey simiklit]

Hello,

I will fix it.
Thanks for review.

Regards,
Andrii.

On Tue, Dec 11, 2018 at 5:04 PM Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> Hi Andrii,
>
> Pardon for joining so late. The patch looks ok, although a few bits I'm
> unsure
> about - see the "Note" below.
>
> There's a bunch of cosmetic suggestions as well - I won't read too much
> into
> them though.
>
> On Mon, 3 Dec 2018 at 15:07, <asimiklit.work at gmail.com> wrote:
>
> [snip]
> > +#define TW 64
> > +#define TH 64
> #define LEVELS 7 // aka log2(min(TW,TH)) + 1 ?
> Then swap nlevels with LEVELS throughout the file.
>
> static const GLubyte fancy_pixel = [255, 128, 64, 32];
>




>
> > +static void
> > +get_rect_bounds(int pos, int *x, int *y, int *w, int *h)
> > +{
> > +       *x = pos * (piglit_width / 3) + 5;
> > +       *y = 5;
> > +       *w = piglit_width / 3 - 10;
> > +       *h = piglit_height - 10;
> > +}
> Note: The variable "pos" is always 0. If that's intentional - I'd just
> drop and simplify.
> Alternatively
>
>
> > +       const GLfloat expected[4] = { oneby255 * 255.0,
> > +
>                oneby255 * 128.0,
> > +
>                oneby255 * 64.0,
> > +
>                oneby255 * 32.0 };
> Use fancy_pixel for expected[]
>
>
> [snip]
> > +       for(level = 1; level < nlevels && pass; ++level)
> > +       {
> Style nits: space after "for", "{" should be trailing on previous
> line, post increment
>
> > +               glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL,
> level);
> > +               glClear(GL_COLOR_BUFFER_BIT);
> > +               /* If we the draw_rect function doesn't cause
> crash/assertion
> > +               * it means everything is okay and test will be marked
> > +               * as pass
> > +               */
> > +               draw_rect(pos);
> > +               /** Just in case we check it
> > +                */
> > +               pass = pass && probe_pos(pos, expected);
>
> Note: In piglit we try to run all the tests, even if one fails. Some tests
> could
> be doing it wrong though :-(
>
> Personally I would drop the "&& pass" from the loop condition and use:
>     pass = foo() && pass;
>
> [snip]
> > +       for (i = 0; i < TH; i++) {
> > +               for (j = 0; j < TW; j++) {
> > +                       img[i][j][0] = 255;
> > +                       img[i][j][1] = 128;
> > +                       img[i][j][2] = 64;
> > +                       img[i][j][3] = 32;
>
> for (i = 0; i < TH; i++)
>     for (j = 0; j < TW; j++)
>         img[i][j] = fancy_pixel;
>
>
> [snip]
>
> > +       w = TW;
> > +       h = TH;
> > +       for (nlevels = 0; w > 0 && h > 0; nlevels++) {
> > +               w /= 2;
> > +               h /= 2;
> > +       }
> > +       w = TW;
> > +       h = TH;
> > +       for (i = 0; w > 0 && h > 0; i++) {
> > +               glTexImage2D(GL_TEXTURE_2D, nlevels - 1 - i, GL_RGBA, w,
> h, 0,
> > +                            GL_RGBA, GL_UNSIGNED_BYTE, img);
> > +               w /= 2;
> > +               h /= 2;
> > +       }
> > +
> With the LEVELS, all the above becomes:
>
>        for (i = 0; i < LEVELS; i++) {
>                glTexImage2D(GL_TEXTURE_2D, nlevels - 1 - i, GL_RGBA, w, h,
> 0,
>                             GL_RGBA, GL_UNSIGNED_BYTE, img);
>                w /= 2;
>                h /= 2;
>        }
>
>
> HTH
> Emil
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20181212/8685c97e/attachment.html>