[Piglit] [PATCH 1/5] util: Move command-line parsing out of piglit_gl_test_config_init

Paul Berry stereotype441 at gmail.com
Mon Oct 21 20:11:26 CEST 2013


On 15 October 2013 17:32, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Create a new function, piglit_gl_process_args, that does the
> command-line parsing.  This happens after the test code between
> PIGLIT_GL_TEST_CONFIG_BEGIN and PIGLIT_GL_TEST_CONFIG_END.  By having an
> explicit function that does this, tests can call it inside the BEGIN/END
> block.  This may be useful for tests that expect certain arguments to be
> in specific positions.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Chad Versace <chad.versace at linux.intel.com>
>

I spent a while worrying about the fact that this change causes some tests
to call piglit_gl_process_args() twice (once explicitly, and once
implicitly from PIGLIT_GL_TEST_CONFIG_END).  Would you mind adding a note
to the commit message explaining that piglit_gl_process_args() is
idempotent (since it removes the args it processes), so that it's safe to
call more than once from a single test?

With that change, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


> ---
>  tests/texturing/shaders/texelFetch.c  | 10 ++++++----
>  tests/texturing/shaders/textureSize.c | 10 ++++++----
>  tests/util/piglit-framework-gl.c      | 24 ++++++++++++++----------
>  tests/util/piglit-framework-gl.h      | 10 +++++++---
>  4 files changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/tests/texturing/shaders/texelFetch.c
> b/tests/texturing/shaders/texelFetch.c
> index ebd48cd..bc6cc8f 100644
> --- a/tests/texturing/shaders/texelFetch.c
> +++ b/tests/texturing/shaders/texelFetch.c
> @@ -84,6 +84,12 @@ static enum shader_target test_stage = UNKNOWN;
>
>  PIGLIT_GL_TEST_CONFIG_BEGIN
>
> +       config.window_width = 900;
> +       config.window_height = 600;
> +       config.window_visual = PIGLIT_GL_VISUAL_RGBA |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +       piglit_gl_process_args(&argc, argv, &config);
> +
>         parse_args(argc, argv);
>         if (test_stage == GS) {
>                 config.supports_gl_compat_version = 32;
> @@ -93,10 +99,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>                 config.supports_gl_core_version = 31;
>         }
>
> -       config.window_width = 900;
> -       config.window_height = 600;
> -       config.window_visual = PIGLIT_GL_VISUAL_RGBA |
> PIGLIT_GL_VISUAL_DOUBLE;
> -
>  PIGLIT_GL_TEST_CONFIG_END
>
>  #define MAX_LOD_OR_SAMPLES  10.0
> diff --git a/tests/texturing/shaders/textureSize.c
> b/tests/texturing/shaders/textureSize.c
> index f010d9c..ee64b07 100644
> --- a/tests/texturing/shaders/textureSize.c
> +++ b/tests/texturing/shaders/textureSize.c
> @@ -52,6 +52,12 @@ static enum shader_target test_stage = UNKNOWN;
>
>  PIGLIT_GL_TEST_CONFIG_BEGIN
>
> +       config.window_width = 150;
> +       config.window_height = 30;
> +       config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +       piglit_gl_process_args(&argc, argv, &config);
> +
>         parse_args(argc, argv);
>         if (test_stage == GS) {
>                 config.supports_gl_compat_version = 32;
> @@ -61,10 +67,6 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
>                 config.supports_gl_core_version = 31;
>         }
>
> -       config.window_width = 150;
> -       config.window_height = 30;
> -       config.window_visual = PIGLIT_GL_VISUAL_RGB |
> PIGLIT_GL_VISUAL_DOUBLE;
> -
>  PIGLIT_GL_TEST_CONFIG_END
>
>  static int lod_location;
> diff --git a/tests/util/piglit-framework-gl.c
> b/tests/util/piglit-framework-gl.c
> index 2a315be..dd2e6a5 100644
> --- a/tests/util/piglit-framework-gl.c
> +++ b/tests/util/piglit-framework-gl.c
> @@ -43,18 +43,9 @@ static void
>  process_args(int *argc, char *argv[], unsigned *force_samples);
>
>  void
> -piglit_gl_test_config_init(int *argc, char *argv[],
> -                          struct piglit_gl_test_config *config)
> +piglit_gl_test_config_init(struct piglit_gl_test_config *config)
>  {
> -       unsigned force_samples = 0;
> -
>         memset(config, 0, sizeof(*config));
> -
> -       process_args(argc, argv, &force_samples);
> -
> -       if (force_samples > 1)
> -               config->window_samples = force_samples;
> -
>  }
>
>  static void
> @@ -125,6 +116,19 @@ process_args(int *argc, char *argv[], unsigned
> *force_samples)
>  }
>
>  void
> +piglit_gl_process_args(int *argc, char *argv[],
> +                      struct piglit_gl_test_config *config)
> +{
> +       unsigned force_samples = 0;
> +
> +       process_args(argc, argv, &force_samples);
> +
> +       if (force_samples > 1)
> +               config->window_samples = force_samples;
> +
> +}
> +
> +void
>  piglit_gl_test_run(int argc, char *argv[],
>                    const struct piglit_gl_test_config *config)
>  {
> diff --git a/tests/util/piglit-framework-gl.h
> b/tests/util/piglit-framework-gl.h
> index 7520f38..fcc1594 100644
> --- a/tests/util/piglit-framework-gl.h
> +++ b/tests/util/piglit-framework-gl.h
> @@ -175,8 +175,11 @@ struct piglit_gl_test_config {
>   * from command line arguments.
>   */
>  void
> -piglit_gl_test_config_init(int *argc, char *argv[],
> -                          struct piglit_gl_test_config *config);
> +piglit_gl_test_config_init(struct piglit_gl_test_config *config);
> +
> +void
> +piglit_gl_process_args(int *argc, char *argv[],
> +                      struct piglit_gl_test_config *config);
>
>  /**
>   * Run the OpenGL test described by @a config. Does not return.
> @@ -210,7 +213,7 @@ piglit_gl_test_run(int argc, char *argv[],
>          {
>    \
>                  struct piglit_gl_test_config config;
>     \
>
>     \
> -                piglit_gl_test_config_init(&argc, argv, &config);
>    \
> +                piglit_gl_test_config_init(&config);
>     \
>
>     \
>                  config.init = piglit_init;
>     \
>                  config.display = piglit_display;
>     \
> @@ -230,6 +233,7 @@ piglit_gl_test_run(int argc, char *argv[],
>  #define PIGLIT_GL_TEST_CONFIG_END
>    \
>                  }
>    \
>
>     \
> +                piglit_gl_process_args(&argc, argv, &config);
>    \
>                  piglit_gl_test_run(argc, argv, &config);
>     \
>
>     \
>                  assert(false);
>     \
> --
> 1.8.1.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131021/e480b0a7/attachment-0001.html>


More information about the Piglit mailing list