On 10/24/2011 02:16 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> ---
> src/glsl/Makefile | 1 +
> src/glsl/link_uniforms.cpp | 116
> ++++++++++++++++++++++++++++++++++++++++++++
> src/glsl/linker.h | 45 +++++++++++++++++
> 3 files changed, 162 insertions(+), 0 deletions(-)
> create mode 100644 src/glsl/link_uniforms.cpp
>
> diff --git a/src/glsl/Makefile b/src/glsl/Makefile
> index b2efb2a..504f1fb 100644
> --- a/src/glsl/Makefile
> +++ b/src/glsl/Makefile
> @@ -53,6 +53,7 @@ CXX_SOURCES = \
> ir_variable_refcount.cpp \
> linker.cpp \
> link_functions.cpp \
> + link_uniforms.cpp \
> loop_analysis.cpp \
> loop_controls.cpp \
> loop_unroll.cpp \
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> new file mode 100644
> index 0000000..38bc00c
> --- /dev/null
> +++ b/src/glsl/link_uniforms.cpp
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright © 2011 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 "main/core.h"
> +#include "ir.h"
> +#include "linker.h"
> +#include "glsl_symbol_table.h"
> +#include "program/hash_table.h"
> +
> +/**
> + * \file link_uniforms.cpp
> + * Assign locations for GLSL uniforms.
> + *
> + * \author Ian Romanick <ian.d.roman...@intel.com>
> + */
> +
> +void uniform_field_visitor::process(ir_variable *var)
> +{
> + char *name = const_cast<char *>(var->name);
I loathe const_cast<>---it's just evil. I declare my variables const
for a reason, and I don't want people screwing them up. Guess how fun
it is to debug issues caused by your supposedly const values changing
out from under you?
I see that you're trying to avoid a strdup in the common case, which is
good, but I'd prefer a less dirty solution.
> + unsigned len = 0;
> +
> + recursion(var->type, &name, strlen(name), len);
> + if (name != var->name)
> + free(name);
> +}
> +
> +void uniform_field_visitor::recursion(const glsl_type *t, char **name,
> + unsigned name_length,
> + unsigned &name_buffer_length)
> +{
> + /* Records need to have each field processed individually.
> + *
> + * Arrays of records need to have each array element processed
> + * individually, then each field of the resulting array elements processed
> + * individually.
> + */
> + if (t->is_record()) {
> + for (unsigned i = 0; i < t->length; i++) {
> + const unsigned len = strlen(t->fields.structure[i].name);
> +
> + /* Size of "name.field\0".
> + */
> + const unsigned field_length = name_length + 1 + len + 1;
> +
> + /* Grow the name buffer if it is not large enough to hold the fully
> + * qualified name of this field.
> + */
> + if (name_buffer_length < field_length) {
> + if (name_buffer_length == 0) {
> + name_buffer_length = field_length + 16;
> + char *tmp = (char *) malloc(name_buffer_length);
> + strcpy(tmp, *name);
> +
> + *name = tmp;
> + } else {
> + name_buffer_length = field_length + 16;
> + *name = (char *) realloc(*name, name_buffer_length);
> + }
> + }
> +
> + /* Append '.field' to the current uniform name.
> + */
> + (*name)[name_length] = '.';
> + memcpy(& (*name)[name_length+1], t->fields.structure[i].name, len);
> + (*name)[field_length - 1] = '\0';
Maybe it's just me, but I find this code rather impenetrable what with
all the buffer management and string munging. mallocs, reallocs,
memcpys, strcpys, extra parameters, the magic constant + 16...
Yet, all it does is generate names such as
some_struct.foo
some_struct.bar
some_struct.baz
and call a function for each of those. That's really simple.
I understand that you're trying to avoid unnecessary strlen() calls, but
there has to be a better way.
I've added some new functionality to ralloc that should make this a lot
cleaner. I'll send it out shortly.
> + recursion(t->fields.structure[i].type,
> + name, field_length - 1, name_buffer_length);
> + }
> + } else if (t->is_array() && t->fields.array->is_record()) {
> + const unsigned extra = unsigned(ceil(log10(float(t->length)))) + 2;
> + const unsigned field_length = name_length + extra;
> +
> + if (name_buffer_length < field_length) {
> + if (name_buffer_length == 0) {
> + name_buffer_length = field_length + 16;
> + char *tmp = (char *) malloc(name_buffer_length);
> + strcpy(tmp, *name);
> +
> + *name = tmp;
> + } else {
> + name_buffer_length = field_length + 16;
> + *name = (char *) realloc(*name, name_buffer_length);
> + }
> + }
> +
> + for (unsigned i = 0; i < t->length; i++) {
> + const unsigned len = name_length
> + + sprintf(&(*name)[name_length], "[%d]", i);
> +
> + recursion(t->fields.array, name, len, name_buffer_length);
> + }
> + } else {
> + this->visit_field(t, *name);
> + }
> +}
> diff --git a/src/glsl/linker.h b/src/glsl/linker.h
> index 769cf68..2bd2b47 100644
> --- a/src/glsl/linker.h
> +++ b/src/glsl/linker.h
> @@ -1,3 +1,4 @@
> +/* -*- c++ -*- */
> /*
> * Copyright © 2010 Intel Corporation
> *
> @@ -29,4 +30,48 @@ extern bool
> link_function_calls(gl_shader_program *prog, gl_shader *main,
> gl_shader **shader_list, unsigned num_shaders);
>
> +
> +/**
> + * Class for processing all of the leaf fields of an uniform
> + *
> + * Leaves are, roughly speaking, the parts of the uniform that the
> application
> + * could query with \c glGetUniformLocation (or that could be returned by
> + * \c glGetActiveUniforms).
> + *
> + * Classes my derive from this class to implement specific functionality.
> + * This class only provides the mechanism to iterate over the leaves.
> Derived
> + * classes must implement \c ::visit_field and may override \c ::process.
> + */
> +class uniform_field_visitor {
> +public:
> + /**
> + * Begin processing a uniform
> + *
> + * Classes that overload this function should call \c ::process from the
> + * base class to start the recursive processing of the uniform.
> + *
> + * \param var The uniform variable that is to be processed
> + *
> + * Calls \c ::visit_field for each leaf of the uniform.
> + */
> + void process(ir_variable *var);
> +
> +protected:
> + /**
> + * Method invoked for each leaf of the uniform
> + *
> + * \param type Type of the field.
> + * \param name Fully qualified name of the field.
> + */
> + virtual void visit_field(const glsl_type *type, const char *name) = 0;
> +
> +private:
> + /**
> + * \param name_length Length of the current name \b not including the
> + * terminating \c NUL character.
> + */
> + void recursion(const glsl_type *t, char **name, unsigned name_length,
> + unsigned &name_buffer_length);
> +};
> +
> #endif /* GLSL_LINKER_H */
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
