The use of __attribute(section()), __start_test_section, and __stop_test_section is not portable. Could you please follow this up with a change that allows rendercheck to continue to function on non-ELF platforms as well? Preferably by just having alternate implementations of the DECLARE_RENDERCHECK_ARG_TEST and for_each_test macros.
Thanks, Jeremy > On Feb 1, 2016, at 13:48, Eric Anholt <[email protected]> wrote: > > Managing the group logic was really error-prone (forget to edit > success_mask when copy and pasting? Forget to printf a description of > the group?). Most new tests being written can be described as a single > call that does a couple subtests. > > This doesn't convert all of the tests. Some of the remaining ones use > "win" for presenting results (which we may want to just put in a > global?), and the rest use the pre-created pictures, which would need > some much bigger refactoring if we want to move their test logic into > their test files. > > Signed-off-by: Eric Anholt <[email protected]> > --- > main.c | 47 ++++++++++++++++++++++++++++++++++------------- > rendercheck.h | 51 ++++++++++++++++++++++++++++++++++++++++++++------- > t_gtk_argb_xbgr.c | 17 ++++++++++++----- > t_libreoffice_xrgb.c | 18 ++++++++++++++++-- > tests.c | 37 ++++++++++++++----------------------- > 5 files changed, 120 insertions(+), 50 deletions(-) > > diff --git a/main.c b/main.c > index b5d67cc..4cec99b 100644 > --- a/main.c > +++ b/main.c > @@ -121,27 +121,38 @@ struct { > {TEST_REPEAT, "repeat"}, > {TEST_TRIANGLES, "triangles"}, > {TEST_BUG7366, "bug7366"}, > - {TEST_GTK_ARGB_XBGR, "gtk_argb_xbgr"}, > - {TEST_LIBREOFFICE_XRGB, "libreoffice_xrgb"}, > {0, NULL} > }; > > +static void > +print_test_separator(int i) > +{ > + if (i % 5 == 0) { > + if(i != 0) > + putc('\n', stderr); > + putc('\t', stderr); > + } else { > + fprintf(stderr, ", "); > + } > +} > + > void print_tests(FILE *file, int tests) { > int i, j; > > for (i=0, j=0; available_tests[i].name; i++) { > if (!(available_tests[i].flag & tests)) > continue; > - if (j % 5 == 0) { > - if(j != 0) > - putc('\n', stderr); > - putc('\t', stderr); > - } else { > - fprintf(stderr, ", "); > - } > + print_test_separator(j++); > fprintf(stderr, "%s", available_tests[i].name); > j++; > } > + for_each_test(test) { > + if (!(test->bit & tests)) > + continue; > + print_test_separator(j++); > + fprintf(stderr, "%s", test->arg_name); > + j++; > + } > if (j) > fprintf(file, "\n"); > } > @@ -169,7 +180,7 @@ int main(int argc, char **argv) > XSetWindowAttributes as; > picture_info window; > char *display = NULL; > - char *test, *format, *opname, *nextname; > + char *test_name, *format, *opname, *nextname; > > static struct option longopts[] = { > { "display", required_argument, NULL, 'd' }, > @@ -239,15 +250,25 @@ int main(int argc, char **argv) > /* disable all tests */ > enabled_tests = 0; > > - while ((test = strsep(&nextname, ",")) != NULL) { > + while ((test_name = strsep(&nextname, ",")) != NULL) { > int i; > + bool found = false; > for(i=0; available_tests[i].name; i++) { > - if(strcmp(test, > available_tests[i].name) == 0) { > + if(strcmp(test_name, > available_tests[i].name) == 0) { > enabled_tests |= > available_tests[i].flag; > + found = true; > + break; > + } > + } > + for_each_test(test) { > + if (strcmp(test_name, > + test->arg_name) == 0) { > + enabled_tests |= test->bit; > + found = true; > break; > } > } > - if(available_tests[i].name == NULL) > + if (!found) > usage(argv[0]); > } > > diff --git a/rendercheck.h b/rendercheck.h > index 2195cb4..f0fa7b7 100644 > --- a/rendercheck.h > +++ b/rendercheck.h > @@ -64,6 +64,19 @@ struct op_info { > bool disabled; > }; > > +struct rendercheck_test_result { > + int tests; > + int passed; > +}; > + > +static inline void > +record_result(struct rendercheck_test_result *result, bool success) > +{ > + result->tests++; > + if (success) > + result->passed++; > +} > + > #define TEST_FILL 0x0001 > #define TEST_DSTCOORDS 0x0002 > #define TEST_SRCCOORDS 0x0004 > @@ -77,8 +90,27 @@ struct op_info { > #define TEST_REPEAT 0x0400 > #define TEST_TRIANGLES 0x0800 > #define TEST_BUG7366 0x1000 > -#define TEST_GTK_ARGB_XBGR 0x2000 > -#define TEST_LIBREOFFICE_XRGB 0x4000 > +#define TEST_gtk_argb_xbgr 0x2000 > +#define TEST_libreoffice_xrgb 0x4000 > + > +struct rendercheck_test { > + int bit; > + const char *arg_name; > + const char *long_name; > + struct rendercheck_test_result (*func)(Display *dpy); > +}; > + > +#define DECLARE_RENDERCHECK_TEST(name) \ > + const struct rendercheck_test test_desc_##name \ > + __attribute__ ((section ("test_section"))) > + > +#define DECLARE_RENDERCHECK_ARG_TEST(arg_name_, long_name_, func_) > \ > + DECLARE_RENDERCHECK_TEST(arg_name_) = { \ > + .bit = TEST_##arg_name_, \ > + .arg_name = #arg_name_, \ > + .long_name = long_name_, \ > + .func = func_, \ > + } > > struct render_format { > XRenderPictFormat *format; > @@ -88,6 +120,12 @@ struct render_format { > extern struct render_format *formats; > extern int nformats; > > +/* Storage that will point at the start and end of the ELF section for test > + * structs. These are automatically set up by the linker when placing things > + * in their sections. > + */ > +extern struct rendercheck_test __start_test_section, __stop_test_section; > + > extern int pixmap_move_iter; > extern int win_width, win_height; > extern struct op_info ops[]; > @@ -226,8 +264,7 @@ trifan_test(Display *dpy, picture_info *win, picture_info > *dst, int op, > bool > bug7366_test(Display *dpy); > > -bool > -gtk_argb_xbgr_test(Display *dpy); > - > -bool > -libreoffice_xrgb_test(Display *dpy, bool invert); > +#define for_each_test(test) \ > + for (struct rendercheck_test *test = &__start_test_section; \ > + test < &__stop_test_section; \ > + test++) > diff --git a/t_gtk_argb_xbgr.c b/t_gtk_argb_xbgr.c > index 2b004d5..f19dfea 100644 > --- a/t_gtk_argb_xbgr.c > +++ b/t_gtk_argb_xbgr.c > @@ -31,8 +31,8 @@ > #define PIXEL_ABGR 0xff886644 > #define PIXEL_RGB 0x446688 > > -bool > -gtk_argb_xbgr_test(Display *dpy) > +static struct rendercheck_test_result > +test_gtk_argb_xbgr(Display *dpy) > { > int x, y; > Pixmap pix_32; > @@ -46,6 +46,7 @@ gtk_argb_xbgr_test(Display *dpy) > XRenderPictFormat *pic_rgb_format; > GC gc_32; > XImage *image_24, *image_32; > + struct rendercheck_test_result result = {}; > > templ.type = PictTypeDirect; > templ.depth = 32; > @@ -95,7 +96,8 @@ gtk_argb_xbgr_test(Display *dpy) > > if (!pic_argb_format || !pic_xbgr_format || !pic_rgb_format) { > printf("Couldn't find xBGR and ARGB formats\n"); > - return false; > + record_result(&result, false); > + return result; > } > > pix_32 = XCreatePixmap(dpy, RootWindow(dpy, DefaultScreen(dpy)), > @@ -141,10 +143,15 @@ gtk_argb_xbgr_test(Display *dpy) > printf("fail: pixel value is %08lx " > "should be %08x\n", > pixel, PIXEL_RGB); > - return false; > + record_result(&result, false); > + return result; > } > } > } > > - return true; > + record_result(&result, true); > + return result; > } > + > +DECLARE_RENDERCHECK_ARG_TEST(gtk_argb_xbgr, "GTK xRGB/ABGR same picture", > + test_gtk_argb_xbgr); > diff --git a/t_libreoffice_xrgb.c b/t_libreoffice_xrgb.c > index 9efca58..1398a01 100644 > --- a/t_libreoffice_xrgb.c > +++ b/t_libreoffice_xrgb.c > @@ -39,8 +39,8 @@ > #define PIXEL_ARGB 0xff886644 > #define INVERT_PIXEL_ARGB 0xff7799bb > > -bool > -libreoffice_xrgb_test(Display *dpy, bool invert) > +static bool > +libreoffice_xrgb_test_one(Display *dpy, bool invert) > { > int x, y; > Pixmap src_pix, dst_pix; > @@ -163,3 +163,17 @@ libreoffice_xrgb_test(Display *dpy, bool invert) > > return true; > } > + > +static struct rendercheck_test_result > +test_libreoffice_xrgb(Display *dpy) > +{ > + struct rendercheck_test_result result = { }; > + > + record_result(&result, libreoffice_xrgb_test_one(dpy, false)); > + record_result(&result, libreoffice_xrgb_test_one(dpy, true)); > + > + return result; > +} > + > +DECLARE_RENDERCHECK_ARG_TEST(libreoffice_xrgb, "LibreOffice xRGB", > + test_libreoffice_xrgb); > diff --git a/tests.c b/tests.c > index 456e778..730acbf 100644 > --- a/tests.c > +++ b/tests.c > @@ -462,6 +462,20 @@ do { > \ > test_dst[num_test_dst++] = &pictures_solid[i]; > } > > + for_each_test(test) { > + struct rendercheck_test_result result; > + > + if (!(enabled_tests & test->bit)) > + continue; > + > + result = test->func(dpy); > + tests_total += result.tests; > + tests_passed += result.passed; > + > + if (result.tests == result.passed) > + success_mask |= test->bit; > + } > + > if (enabled_tests & TEST_FILL) { > bool ok, group_ok = true; > > @@ -733,29 +747,6 @@ do { > \ > success_mask |= TEST_BUG7366; > } > > - if (enabled_tests & TEST_GTK_ARGB_XBGR) { > - bool ok, group_ok = true; > - > - ok = gtk_argb_xbgr_test(dpy); > - RECORD_RESULTS(); > - > - if (group_ok) > - success_mask |= TEST_GTK_ARGB_XBGR; > - } > - > - if (enabled_tests & TEST_LIBREOFFICE_XRGB) { > - bool ok, group_ok = true; > - > - ok = libreoffice_xrgb_test(dpy, false); > - RECORD_RESULTS(); > - > - ok = libreoffice_xrgb_test(dpy, true); > - RECORD_RESULTS(); > - > - if (group_ok) > - success_mask |= TEST_LIBREOFFICE_XRGB; > - } > - > free(test_ops); > free(test_src); > free(test_mask); > -- > 2.7.0 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
