[Piglit] [PATCH v3 1/3] piglit-util-gl: Add piglit_get_gl_enum_from_name

[Dylan Baker]

This generally looks good. As long as Ilia has no objections neither do
I. I have a few minor comments, but they're minor and you can take them
or leave them.

Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>

On Mon, Feb 23, 2015 at 01:22:22PM -0800, Jordan Justen wrote:
> v2:
>  * Use key based sorting rather than cmp
>  * Print message to stderr before aborting in
>    piglit_get_gl_enum_from_name
>  * Use const with static data in piglit_get_gl_enum_from_name
> 
> v3:
>  * Convert list comprehesion to for loops
>  * Only gather the default namespace enums once
>  * Remove reduce on enums_by_name since it appears the names are
>    unique as returned from get_enums_in_default_namespace
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Reviewed-by: Chad Versace <chad.versace at intel.com> (v1)
> ---
>  tests/util/gen_dispatch.py                | 29 ++++++++++++++++--------
>  tests/util/piglit-util-gl-enum-gen.c.mako | 37 +++++++++++++++++++++++++++++++
>  tests/util/piglit-util-gl.h               |  9 ++++++++
>  3 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/util/gen_dispatch.py b/tests/util/gen_dispatch.py
> index 4d7d756..73af820 100644
> --- a/tests/util/gen_dispatch.py
> +++ b/tests/util/gen_dispatch.py
> @@ -131,30 +131,41 @@ class EnumCode(object):
>      @classmethod
>      def emit(cls, out_dir, gl_registry):
>          assert isinstance(gl_registry, registry.gl.Registry)
> -        enums = cls.get_unique_enums_in_default_namespace(gl_registry)
> +        enums = cls.get_enums_in_default_namespace(gl_registry)
> +        unique_enums = cls.get_unique_enums(enums)
> +        enums_by_name = cls.get_enums_by_name(enums)
>          render_template(
>              cls.C_TEMPLATE,
>              out_dir,
>              gl_registry=gl_registry,
> -            sorted_unique_enums_in_default_namespace=enums)
> +            sorted_unique_enums_in_default_namespace=unique_enums,
> +            sorted_enums_by_name=enums_by_name)
>  
>      @classmethod
> -    def get_unique_enums_in_default_namespace(cls, gl_registry):
> +    def get_enums_in_default_namespace(cls, gl_registry):
> +        enums = []
> +        for enum_group in gl_registry.enum_groups:
> +            if enum_group.type == 'default_namespace':

Can you just use extend?:
enums.extend(enum_group.enums)

> +                for enum in enum_group.enums:
> +                    enums.append(enum)
> +        return enums
> +
> +    @classmethod
> +    def get_unique_enums(cls, enums):
>          def append_enum_if_new_value(enum_list, enum):
>              if enum_list[-1].num_value < enum.num_value:
>                  enum_list.append(enum)
>              return enum_list
>  
> -        enums = (
> -            enum
> -            for enum_group in gl_registry.enum_groups
> -            if enum_group.type == 'default_namespace'
> -            for enum in enum_group.enums
> -        )
>          enums = sorted(enums)
>          enums = reduce(append_enum_if_new_value, enums[1:], [enums[0]])
>          return enums
>  
> +    @classmethod
> +    def get_enums_by_name(cls, enums):
> +        enums = sorted(enums, key=lambda enum: enum.name)
> +        return enums
> +

you could drop the assignment and just directly return sorted(...)

>  
>  if __name__ == '__main__':
>      main()
> diff --git a/tests/util/piglit-util-gl-enum-gen.c.mako b/tests/util/piglit-util-gl-enum-gen.c.mako
> index e22a75d..9a0fff8 100644
> --- a/tests/util/piglit-util-gl-enum-gen.c.mako
> +++ b/tests/util/piglit-util-gl-enum-gen.c.mako
> @@ -50,4 +50,41 @@ piglit_get_prim_name(GLenum prim)
>  >-------default: return "(unrecognized enum)";
>  >-------}
>  }
> +
> +struct gl_name_to_enum {
> +>-------const char *name;
> +>-------GLenum _enum;
> +};
> +
> +static int
> +compare_enum_name(const void *a, const void *b)
> +{
> +>-------return strcmp(((struct gl_name_to_enum*)a)->name,
> +>-------              ((struct gl_name_to_enum*)b)->name);
> +}
> +
> +GLenum
> +piglit_get_gl_enum_from_name(const char *name)
> +{
> +>-------static const struct gl_name_to_enum names[] = {
> +% for enum in sorted_enums_by_name:
> +>------->-------{ "${enum.name}", ${enum.c_num_literal} },
> +% endfor
> +>-------};
> +>-------struct gl_name_to_enum key = { name, 0 };
> +>-------struct gl_name_to_enum *result;
> +
> +>-------result = (struct gl_name_to_enum*)
> +>------->-------bsearch(&key,
> +>------->-------        names, ARRAY_SIZE(names), sizeof names[0],
> +>------->-------        compare_enum_name);
> +
> +>-------if (result == NULL) {
> +>------->-------fprintf(stderr, "No known enum named %s!\n", name);
> +>------->-------abort();
> +>-------}
> +
> +>-------return result->_enum;
> +}
> +
>  </%block>\
> diff --git a/tests/util/piglit-util-gl.h b/tests/util/piglit-util-gl.h
> index 0f8eb81..fa4a6e4 100644
> --- a/tests/util/piglit-util-gl.h
> +++ b/tests/util/piglit-util-gl.h
> @@ -87,6 +87,15 @@ const char* piglit_get_gl_error_name(GLenum error);
>  const char *piglit_get_gl_enum_name(GLenum param);
>  
>  /**
> + * \brief Convert a string to a GL enum.
> + *
> + * For example, given "GL_INVALID_ENUM", return GL_INVALID_ENUM.
> + *
> + * abort() if the string is not recognized.
> + */
> +GLenum piglit_get_gl_enum_from_name(const char *name);
> +
> +/**
>   * \brief Convert a GL primitive type enum value to a string.
>   *
>   * For example, given GL_POLYGON, return "GL_POLYGON".
> -- 
> 2.1.4
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150223/16d8f260/attachment.sig>