On 11/22/2015 03:43 AM, Emil Velikov wrote: > On 11 November 2015 at 18:07, Emil Velikov <emil.l.veli...@gmail.com> wrote: >> From: Emil Velikov <emil.veli...@collabora.com> >> >> Noticed as some of these have been intermittently failing on llvmpipe, >> resulting in a few "not run" test across mesa release checks. >> >> Signed-off-by: Emil Velikov <emil.veli...@collabora.com> >> --- >> >> XXX: >> At some point we'd want to do a tree-wide: >> - s/GLboolean pass/bool pass/ >> - s/pass = pass && foo/pass &= foo/ >> - s/pass = foo && pass/pass &= foo/
Yes, please... but see below. >> We might want to convert the test to use the piglit_probe_pixels over >> it's custom ones. >> >> -Emil >> >> tests/texturing/texwrap.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/tests/texturing/texwrap.c b/tests/texturing/texwrap.c >> index fbe9068..60ffa73 100644 >> --- a/tests/texturing/texwrap.c >> +++ b/tests/texturing/texwrap.c >> @@ -1134,7 +1134,7 @@ static GLboolean test_format_npot(const struct >> format_desc *format, GLboolean np >> * It has to be enabled on the command line. >> */ >> if (!texture_swizzle && !npot && !test_border_color && >> has_texture_swizzle) { >> - pass = pass && test_format_npot_swizzle(format, >> npot, 1); >> + pass = test_format_npot_swizzle(format, npot, 1) && >> pass; >> } >> } >> return pass; >> @@ -1149,7 +1149,7 @@ static GLboolean test_format(const struct format_desc >> *format) >> } else { >> pass = test_format_npot(format, 0); >> if (has_npot && !test_border_color) { >> - pass = pass && test_format_npot(format, 1); >> + pass = test_format_npot(format, 1) && pass; >> } >> } >> return pass; >> @@ -1163,7 +1163,7 @@ enum piglit_result piglit_display() >> pass = test_format(init_format ? init_format : >> &test->format[0]); >> } else { >> if (init_format) { >> - pass = pass && test_format(init_format); >> + pass = test_format(init_format) && pass; >> } else { >> int i; >> for (i = 0; i < test->num_formats; i++) { >> -- > Any takers on this trivial patch ? I guess we can bikeshed the "pass = I don't think we should use the term bikeshed. While it didn't start that way, it has become a purely pejorative term used to dismiss someone's feedback. It's only purpose these days seems to be to make people mad and start arguments. > foo && pass" vs "pass &= foo" at a later stage. The problem with &= is that it doesn't extend to more than two predicates. As a result, there will always be place in piglit that do pass = foo && bar && asdf && pass; We don't want to have two different idioms for essentially the same thing. That means that test authors and reviews have to stop and think about which idiom should be used in which places. It also means that if a place that used &= is extended with another predicate, you have to change more of the code. So, pass &= foo; would become pass = foo && bar && pass; And everyone has to review it more carefully to be sure it's right. Moreover, "pass = foo && pass;" has been the piglit idiom for *years*. A few new-comers came along and started using the other idiom because they had used it on other projects. Piglit's slack review requirement allowed a bunch of that to slip in. I really don't want to see it spread any further, and I'd be happy to review patches that change the existing uses of &=. > -Emil > _______________________________________________ > Piglit mailing list > Piglit@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/piglit > _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit