[Mesa-dev] [PATCH 2/6] glsl/tests: Verify vertex shader built-ins generated by _mesa_glsl_initialize_variables

Paul Berry stereotype441 at gmail.com
Fri Oct 18 22:14:10 CEST 2013


On 11 October 2013 11:18, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Checks that the variables generated meet certain criteria.
>
>  - Vertex shader inputs have an explicit location.
>
>  - Vertex shader outputs have an explicit location.
>
>  - Fragment shader-only varying locations are not used.
>
>  - Vertex shader uniforms and system values don't have an explicit
>    location.
>
>  - Vertex shader constants don't have an explicit location and are
>    read-only.
>
>  - No other kinds of vertex variables exist.
>
> It does not verify that an specific variables exist.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/Makefile.am                     |   1 +
>  src/glsl/tests/builtin_variable_test.cpp | 235
> +++++++++++++++++++++++++++++++
>  2 files changed, 236 insertions(+)
>  create mode 100644 src/glsl/tests/builtin_variable_test.cpp
>
> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
> index f43f49d..80949fb 100644
> --- a/src/glsl/Makefile.am
> +++ b/src/glsl/Makefile.am
> @@ -59,6 +59,7 @@ tests_general_ir_test_SOURCES =               \
>         $(top_srcdir)/src/mesa/program/prog_hash_table.c\
>         $(top_srcdir)/src/mesa/program/symbol_table.c   \
>         $(GLSL_SRCDIR)/standalone_scaffolding.cpp \
> +       tests/builtin_variable_test.cpp                 \
>         tests/general_ir_test.cpp
>  tests_general_ir_test_CFLAGS =                         \
>         $(PTHREAD_CFLAGS)
> diff --git a/src/glsl/tests/builtin_variable_test.cpp
> b/src/glsl/tests/builtin_variable_test.cpp
> new file mode 100644
> index 0000000..c4711ab
> --- /dev/null
> +++ b/src/glsl/tests/builtin_variable_test.cpp
> @@ -0,0 +1,235 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +#include <gtest/gtest.h>
> +#include "standalone_scaffolding.h"
> +#include "main/compiler.h"
> +#include "main/mtypes.h"
> +#include "main/macros.h"
> +#include "ralloc.h"
> +#include "ir.h"
> +#include "glsl_parser_extras.h"
> +#include "glsl_symbol_table.h"
> +
> +class common_builtin : public ::testing::Test {
> +public:
> +   common_builtin(GLenum shader_type)
> +      : shader_type(shader_type)
> +   {
> +      /* empty */
> +   }
> +
> +   virtual void SetUp();
> +   virtual void TearDown();
> +
> +   void string_starts_with_prefix(const char *str, const char *prefix);
> +   void names_start_with_gl();
> +   void uniforms_and_system_values_dont_have_explicit_location();
> +   void constants_are_constant();
> +   void no_invalid_variable_modes();
> +
> +   GLenum shader_type;
> +   struct _mesa_glsl_parse_state *state;
> +   struct gl_shader *shader;
> +   void *mem_ctx;
> +   gl_context ctx;
> +   exec_list ir;
> +};
> +
> +void
> +common_builtin::SetUp()
> +{
> +   this->mem_ctx = ralloc_context(NULL);
> +   this->ir.make_empty();
> +
> +   initialize_context_to_defaults(&this->ctx, API_OPENGL_COMPAT);
> +
> +   this->shader = rzalloc(this->mem_ctx, gl_shader);
> +   this->shader->Type = this->shader_type;
> +
> +   this->state =
> +      new(mem_ctx) _mesa_glsl_parse_state(&this->ctx, this->shader->Type,
> +                                          this->shader);
> +
> +   _mesa_glsl_initialize_types(this->state);
> +   _mesa_glsl_initialize_variables(&this->ir, this->state);
> +}
> +
> +void
> +common_builtin::TearDown()
> +{
> +   ralloc_free(this->mem_ctx);
> +   this->mem_ctx = NULL;
> +}
> +
> +void
> +common_builtin::string_starts_with_prefix(const char *str, const char
> *prefix)
> +{
> +   const size_t len = strlen(prefix);
> +   char *const name_prefix = new char[len];
>

This needs to be new char[len + 1], since you assign to its len'th element
below.


> +
> +   strncpy(name_prefix, str, len);
> +   name_prefix[len] = '\0';
> +   EXPECT_STREQ(prefix, name_prefix) << "Bad name " << str;
> +}
> +
> +void
> +common_builtin::names_start_with_gl()
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      string_starts_with_prefix(var->name, "gl_");
> +   }
> +}
> +
> +void
> +common_builtin::uniforms_and_system_values_dont_have_explicit_location()
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      if (var->mode != ir_var_uniform && var->mode != ir_var_system_value)
> +         continue;
> +
> +      EXPECT_FALSE(var->explicit_location);
> +      EXPECT_EQ(-1, var->location);
> +   }
> +}
> +
> +void
> +common_builtin::constants_are_constant()
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      if (var->mode != ir_var_auto)
> +         continue;
> +
> +      EXPECT_FALSE(var->explicit_location);
> +      EXPECT_EQ(-1, var->location);
> +      EXPECT_TRUE(var->read_only);
> +   }
> +}
> +
> +void
> +common_builtin::no_invalid_variable_modes()
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      switch (var->mode) {
> +      case ir_var_auto:
> +      case ir_var_uniform:
> +      case ir_var_shader_in:
> +      case ir_var_shader_out:
> +      case ir_var_system_value:
> +         break;
> +
> +      /* Invalid modes that we know. */
> +      case ir_var_function_in:
> +      case ir_var_function_out:
> +      case ir_var_function_inout:
> +      case ir_var_const_in:
> +      case ir_var_temporary:
> +         EXPECT_NE(ir_var_function_in, var->mode);
> +         EXPECT_NE(ir_var_function_out, var->mode);
> +         EXPECT_NE(ir_var_function_inout, var->mode);
> +         EXPECT_NE(ir_var_const_in, var->mode);
> +         EXPECT_NE(ir_var_temporary, var->mode);
> +         break;
>

This block seems really strange to me.  All of these invalid modes are
adequately covered by the default case, which is easier to read.

With those two things fixed, this patch is:

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


> +
> +      /* Invalid modes that we don't know. */
> +      default:
> +         EXPECT_TRUE(false) << "invalid mode " << int(var->mode);
> +         break;
> +      }
> +   }
> +}
> +
> +/************************************************************/
> +
> +class vertex_builtin : public common_builtin {
> +public:
> +   vertex_builtin()
> +      : common_builtin(GL_VERTEX_SHADER)
> +   {
> +      /* empty */
> +   }
> +};
> +
> +TEST_F(vertex_builtin, names_start_with_gl)
> +{
> +   common_builtin::names_start_with_gl();
> +}
> +
> +TEST_F(vertex_builtin, inputs_have_explicit_location)
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      if (var->mode != ir_var_shader_in)
> +         continue;
> +
> +      EXPECT_TRUE(var->explicit_location);
> +      EXPECT_NE(-1, var->location);
> +      EXPECT_GT(VERT_ATTRIB_GENERIC0, var->location);
> +      EXPECT_EQ(0u, var->location_frac);
> +   }
> +}
> +
> +TEST_F(vertex_builtin, outputs_have_explicit_location)
> +{
> +   foreach_list(node, &this->ir) {
> +      ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      if (var->mode != ir_var_shader_out)
> +         continue;
> +
> +      EXPECT_TRUE(var->explicit_location);
> +      EXPECT_NE(-1, var->location);
> +      EXPECT_GT(VARYING_SLOT_VAR0, var->location);
> +      EXPECT_EQ(0u, var->location_frac);
> +
> +      /* Several varyings only exist in the fragment shader.  Be sure
> that no
> +       * outputs with these locations exist.
> +       */
> +      EXPECT_NE(VARYING_SLOT_PNTC, var->location);
> +      EXPECT_NE(VARYING_SLOT_FACE, var->location);
> +      EXPECT_NE(VARYING_SLOT_PRIMITIVE_ID, var->location);
> +   }
> +}
> +
> +TEST_F(vertex_builtin,
> uniforms_and_system_values_dont_have_explicit_location)
> +{
> +
> common_builtin::uniforms_and_system_values_dont_have_explicit_location();
> +}
> +
> +TEST_F(vertex_builtin, constants_are_constant)
> +{
> +   common_builtin::constants_are_constant();
> +}
> +
> +TEST_F(vertex_builtin, no_invalid_variable_modes)
> +{
> +   common_builtin::no_invalid_variable_modes();
> +}
> --
> 1.8.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131018/e8596cd9/attachment.html>


More information about the mesa-dev mailing list