Hello,
I will fix it.
Thanks for review.
Regards,
Andrii.
On Tue, Dec 11, 2018 at 5:04 PM Emil Velikov <emil.l.veli...@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.w...@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
