On Thu, 2015-02-26 at 12:06 +1100, Timothy Arceri wrote: > ---- Mark Janes wrote ---- > > > Timothy Arceri <t_arc...@yahoo.com.au> writes: > > > > > Use common intrastage array validation for interface blocks. This > change also allows us to support interface blocks that are arrays of > arrays. > > > > Please wrap this message to fit within 80 columns. > > > > > --- > > > src/glsl/link_interface_blocks.cpp | 76 > ++++++++++++++++++-------------------- > > > 1 file changed, 35 insertions(+), 41 deletions(-) > > > > > > diff --git a/src/glsl/link_interface_blocks.cpp > b/src/glsl/link_interface_blocks.cpp > > > index 0ce502d..9000a3c 100644 > > > --- a/src/glsl/link_interface_blocks.cpp > > > +++ b/src/glsl/link_interface_blocks.cpp > > > @@ -50,18 +50,20 @@ struct interface_block_definition > > > * represents either the interface instance (for named > interfaces), or a > > > * member of the interface (for unnamed interfaces). > > > */ > > > - explicit interface_block_definition(const ir_variable *var) > > > - : type(var->get_interface_type()), > > > - instance_name(NULL), > > > - array_size(-1) > > > + explicit interface_block_definition(ir_variable *var) > > > + : var(var), > > > + type(var->get_interface_type()), > > > + instance_name(NULL) > > > { > > > if (var->is_interface_instance()) { > > > instance_name = var->name; > > > - if (var->type->is_array()) > > > - array_size = var->type->length; > > > } > > > explicitly_declared = (var->data.how_declared != > ir_var_declared_implicitly); > > > } > > > + /** > > > + * Interface block ir_variable > > > + */ > > > + ir_variable *var; > > > > > > /** > > > * Interface block type > > > @@ -74,12 +76,6 @@ struct interface_block_definition > > > const char *instance_name; > > > > > > /** > > > - * For an interface block array, the array size (or 0 if > unsized). > > > - * Otherwise -1. > > > - */ > > > - int array_size; > > > - > > > - /** > > > * True if this interface block was explicitly declared in the > shader; > > > * false if it was an implicitly declared built-in interface > block. > > > */ > > > @@ -95,7 +91,8 @@ struct interface_block_definition > > > bool > > > intrastage_match(interface_block_definition *a, > > > const interface_block_definition *b, > > ^^^^^ > > I think the intent of this declaration is that b should not change > (see below). > > > > > - ir_variable_mode mode) > > > + ir_variable_mode mode, > > > + struct gl_shader_program *prog) > > > { > > > /* Types must match. */ > > > if (a->type != b->type) { > > > @@ -120,18 +117,13 @@ intrastage_match(interface_block_definition > *a, > > > return false; > > > } > > > > > > - /* Array vs. nonarray must be consistent, and sizes must be > > > - * consistent, with the exception that unsized arrays match > sized > > > - * arrays. > > > + /* If a block is an array then it must match across the > shader. > > > + * Unsized arrays are also processed and matched agaist sized > arrays. > > > */ > > > - if ((a->array_size == -1) != (b->array_size == -1)) > > > + if (b->var->type != a->var->type && > > > + (b->instance_name != NULL || a->instance_name != NULL) && > > > + !validate_intrastage_arrays(prog, b->var, a->var)) > > > > you pass a b's var member into validate_intrastage_arrays, where it > may > > be modified (due to your second patch in this series). Is that your > intention? > > > > It doesn't seem to affect any piglit tests. > > Patch 2 may have been a result of me passing the a and b values in the > wrong order when I started development, I will check this.
Ok I didn't need to modify the value in patch 2 at all just add the return true. In which case it should be squashed into patch 1. Thanks for looking over this. I'll send a version 2. > > > > > > return false; > > > - if (b->array_size != 0) { > > > - if (a->array_size == 0) > > > - a->array_size = b->array_size; > > > - else if (a->array_size != b->array_size) > > > - return false; > > > - } > > > > > > return true; > > > } > > > @@ -150,12 +142,6 @@ interstage_match(const > interface_block_definition *producer, > > > const interface_block_definition *consumer, > > > bool extra_array_level) > > > { > > > - /* Unsized arrays should not occur during interstage linking. > They > > > - * should have all been assigned a size by > link_intrastage_shaders. > > > - */ > > > - assert(consumer->array_size != 0); > > > - assert(producer->array_size != 0); > > > - > > > /* Types must match. */ > > > if (consumer->type != producer->type) { > > > /* Exception: if both the interface blocks are implicitly > declared, > > > @@ -165,20 +151,27 @@ interstage_match(const > interface_block_definition *producer, > > > if (consumer->explicitly_declared || > producer->explicitly_declared) > > > return false; > > > } > > > + > > > + /* Ignore outermost array if geom shader */ > > > + const glsl_type *consumer_instance_type; > > > if (extra_array_level) { > > > - /* Consumer must be an array, and producer must not. */ > > > - if (consumer->array_size == -1) > > > - return false; > > > - if (producer->array_size != -1) > > > - return false; > > > + consumer_instance_type = consumer->var->type->fields.array; > > > } else { > > > - /* Array vs. nonarray must be consistent, and sizes must be > consistent. > > > - * Since unsized arrays have been ruled out, we can check > this by just > > > - * making sure the sizes are equal. > > > - */ > > > - if (consumer->array_size != producer->array_size) > > > + consumer_instance_type = consumer->var->type; > > > + } > > > + > > > + /* If a block is an array then it must match across shaders. > > > + * Since unsized arrays have been ruled out, we can check this > by just > > > + * making sure the types are equal. > > > + */ > > > + if ((consumer->instance_name != NULL && > > > + consumer_instance_type->is_array()) || > > > + (producer->instance_name != NULL && > > > + producer->var->type->is_array())) { > > > + if (consumer_instance_type != producer->var->type) > > > return false; > > > } > > > + > > > return true; > > > } > > > > > > @@ -298,7 +291,8 @@ validate_intrastage_interface_blocks(struct > gl_shader_program *prog, > > > */ > > > definitions->store(def); > > > } else if (!intrastage_match(prev_def, &def, > > > - (ir_variable_mode) > var->data.mode)) { > > > + (ir_variable_mode) > var->data.mode, > > > + prog)) { > > > linker_error(prog, "definitions of interface block `% > s' do not" > > > " match\n", iface_type->name); > > > return; > > > @@ -374,7 +368,7 @@ validate_interstage_uniform_blocks(struct > gl_shader_program *prog, > > > * uniform matchin rules (for uniforms, it is as > though all > > > * shaders are in the same shader stage). > > > */ > > > - if (!intrastage_match(old_def, &new_def, > ir_var_uniform)) { > > > + if (!intrastage_match(old_def, &new_def, > ir_var_uniform, prog)) { > > > linker_error(prog, "definitions of interface block > `%s' do not " > > > "match\n", > var->get_interface_type()->name); > > > return; > > > -- > > > 2.1.0 > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev