On Tue, 3 Mar 2015 15:24:18 +0000 Ben Avison <bavi...@riscosopen.org> wrote:
> This reduces code size and eliminates the possibility of cut-and-paste errors > when extending the enums. It also highlighted a mismatch between the set of > formats accepted by format_name() and format_from_string(). For now I have > left the mismatch in place. > --- > test/utils.c | 359 > +++++++++++++++++++++++----------------------------------- > 1 files changed, 144 insertions(+), 215 deletions(-) > > diff --git a/test/utils.c b/test/utils.c > index efec632..e350878 100644 > --- a/test/utils.c > +++ b/test/utils.c > @@ -29,6 +29,15 @@ > #include <png.h> > #endif > > +typedef struct > +{ > + const char *name; > + uint32_t number; > +} name_to_number_t; > + > +#define CAT(x,y) x##y > +#define NAME_TO_NUMBER_INITIALIZER(prefix,name) { #name, CAT(prefix,name) } > + > /* Random number generator state > */ > > @@ -949,106 +958,119 @@ initialize_palette (pixman_indexed_t *palette, > uint32_t depth, int is_rgb) > } > } > > -static const pixman_op_t op_list[] = > -{ > - PIXMAN_OP_CLEAR, > - PIXMAN_OP_SRC, > - PIXMAN_OP_DST, > - PIXMAN_OP_OVER, > - PIXMAN_OP_OVER_REVERSE, > - PIXMAN_OP_IN, > - PIXMAN_OP_IN_REVERSE, > - PIXMAN_OP_OUT, > - PIXMAN_OP_OUT_REVERSE, > - PIXMAN_OP_ATOP, > - PIXMAN_OP_ATOP_REVERSE, > - PIXMAN_OP_XOR, > - PIXMAN_OP_ADD, > - PIXMAN_OP_SATURATE, > - > - PIXMAN_OP_DISJOINT_CLEAR, > - PIXMAN_OP_DISJOINT_SRC, > - PIXMAN_OP_DISJOINT_DST, > - PIXMAN_OP_DISJOINT_OVER, > - PIXMAN_OP_DISJOINT_OVER_REVERSE, > - PIXMAN_OP_DISJOINT_IN, > - PIXMAN_OP_DISJOINT_IN_REVERSE, > - PIXMAN_OP_DISJOINT_OUT, > - PIXMAN_OP_DISJOINT_OUT_REVERSE, > - PIXMAN_OP_DISJOINT_ATOP, > - PIXMAN_OP_DISJOINT_ATOP_REVERSE, > - PIXMAN_OP_DISJOINT_XOR, > - > - PIXMAN_OP_CONJOINT_CLEAR, > - PIXMAN_OP_CONJOINT_SRC, > - PIXMAN_OP_CONJOINT_DST, > - PIXMAN_OP_CONJOINT_OVER, > - PIXMAN_OP_CONJOINT_OVER_REVERSE, > - PIXMAN_OP_CONJOINT_IN, > - PIXMAN_OP_CONJOINT_IN_REVERSE, > - PIXMAN_OP_CONJOINT_OUT, > - PIXMAN_OP_CONJOINT_OUT_REVERSE, > - PIXMAN_OP_CONJOINT_ATOP, > - PIXMAN_OP_CONJOINT_ATOP_REVERSE, > - PIXMAN_OP_CONJOINT_XOR, > - > - PIXMAN_OP_MULTIPLY, > - PIXMAN_OP_SCREEN, > - PIXMAN_OP_OVERLAY, > - PIXMAN_OP_DARKEN, > - PIXMAN_OP_LIGHTEN, > - PIXMAN_OP_COLOR_DODGE, > - PIXMAN_OP_COLOR_BURN, > - PIXMAN_OP_HARD_LIGHT, > - PIXMAN_OP_SOFT_LIGHT, > - PIXMAN_OP_DIFFERENCE, > - PIXMAN_OP_EXCLUSION, > - PIXMAN_OP_HSL_HUE, > - PIXMAN_OP_HSL_SATURATION, > - PIXMAN_OP_HSL_COLOR, > - PIXMAN_OP_HSL_LUMINOSITY > +static const name_to_number_t op_list[] = > +{ > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CLEAR), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_SRC), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DST), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_OVER), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_OVER_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_IN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_IN_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_OUT), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_OUT_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_ATOP), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_ATOP_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_XOR), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_ADD), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_SATURATE), > + > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_CLEAR), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_SRC), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_DST), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_OVER), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_OVER_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_IN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_IN_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_OUT), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_OUT_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_ATOP), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_ATOP_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DISJOINT_XOR), > + > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_CLEAR), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_SRC), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_DST), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_OVER), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_OVER_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_IN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_IN_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_OUT), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_OUT_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_ATOP), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_ATOP_REVERSE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_CONJOINT_XOR), > + > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_MULTIPLY), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_SCREEN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_OVERLAY), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DARKEN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_LIGHTEN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_COLOR_DODGE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_COLOR_BURN), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_HARD_LIGHT), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_SOFT_LIGHT), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_DIFFERENCE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_EXCLUSION), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_HSL_HUE), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_HSL_SATURATION), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_HSL_COLOR), > + NAME_TO_NUMBER_INITIALIZER(,PIXMAN_OP_HSL_LUMINOSITY), > }; Hi, this is mostly a stylistic issue and I wonder what the maintainers think... NAME_TO_NUMBER_INITIALIZER is very long to read IMO, I've seen some projects #define _(...) and then #undef _ right after the array is initialized. Would that be appropriate for Pixman? (Please read my reply to patch 4 first.) If we need some aliases, this would need to change somehow to allow them nicely. Wonder if we should just record two strings per entry: { PIXMAN_OP_OVER_REVERSE, "PIXMAN_OP_OVER_REVERSE", "over_reverse" }, { PIXMAN_OP_OVER_REVERSE, "PIXMAN_OP_OVER_REVERSE", "overrev" }, so that operator_name () can just return the first string and operator_from_string () can attempt to match both. Eh, we should just see what needs to be done to patch 4, and then think about this. Nice find about the format mismatches. After this patch series there is: > const char * > format_name (pixman_format_code_t format) > { > /* First check for one of the formats in format_list */ > int i; > > for (i = 0; i < ARRAY_LENGTH (format_list); ++i) > { > if (format == format_list[i].number) > return format_list[i].name; > } > > /* Also permit some additional formats */ > switch (format) > { > /* 8bpp formats */ > #if 0 > case PIXMAN_x4c4: return "x4c4"; > case PIXMAN_g8: return "g8"; > #endif > case PIXMAN_c8: return "x4c4 / c8"; > case PIXMAN_x4g4: return "x4g4 / g8"; > > /* 4bpp formats */ > case PIXMAN_c4: return "c4"; > case PIXMAN_g4: return "g4"; > > /* 1bpp formats */ > case PIXMAN_g1: return "g1"; > > /* YUV formats */ > case PIXMAN_yuy2: return "yuy2"; > case PIXMAN_yv12: return "yv12"; > > default: break; > }; Wonder if those should be added to the format table in a follow-up or not. Thanks, pq _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman