From 33fe02111605ec8897dbc1c989c137b6e38c02f8 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Tue, 3 Jan 2012 20:41:34 -0800 Subject: [PATCH] mesa: Fix transform feedback of unsubscripted arrays. It is not explicitly stated in the GL 3.0 spec that transform feedback can be performed on a whole varying array (without supplying a subscript). However, it seems clear from context that this was the intent. Section 2.15 (TransformFeedback) says this: When writing varying variables that are arrays, individual array elements are written in order. And section 2.20.3 (Shader Variables), says this, in the description of GetTransformFeedbackVarying: For the selected varying variable, its type is returned into type. The size of the varying is returned into size. The value in size is in units of the type returned in type. If it were not possible to perform transform feedback on an unsubscripted array, the returned size would always be 1. This patch fixes the linker so that transform feedback on an unsubscripted array is supported. Fixes piglit tests "EXT_transform_feedback/builtin-varyings gl_ClipDistance[{4,8}]-no-subscript" and "EXT_transform_feedback/output_type *[2]-no-subscript". Note: on back-ends that set gl_shader_compiler_options::LowerClipDistance (for example i965), tests "EXT_transform_feedback/builtin-varyings gl_ClipDistance[{1,2,3,5,6,7}]" still fail. I hope to address this in a later patch. Reviewed-by: Kenneth Graunke Reviewed-by: Ian Romanick --- src/glsl/linker.cpp | 99 +++++++++++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 45 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index c778d9a..88c81c4 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1426,12 +1426,12 @@ private: /** * True if the declaration in orig_name represents an array. */ - bool is_array; + bool is_subscripted; /** - * If is_array is true, the array index that was specified in orig_name. + * If is_subscripted is true, the subscript that was specified in orig_name. */ - unsigned array_index; + unsigned array_subscript; /** * Which component to extract from the vertex shader output location that @@ -1460,6 +1460,12 @@ private: /** Type of the varying returned by glGetTransformFeedbackVarying() */ GLenum type; + + /** + * If location != -1, the size that should be returned by + * glGetTransformFeedbackVarying(). + */ + unsigned size; }; @@ -1484,14 +1490,14 @@ tfeedback_decl::init(struct gl_context *ctx, struct gl_shader_program *prog, if (bracket) { this->var_name = ralloc_strndup(mem_ctx, input, bracket - input); - if (sscanf(bracket, "[%u]", &this->array_index) != 1) { + if (sscanf(bracket, "[%u]", &this->array_subscript) != 1) { linker_error(prog, "Cannot parse transform feedback varying %s", input); return false; } - this->is_array = true; + this->is_subscripted = true; } else { this->var_name = ralloc_strdup(mem_ctx, input); - this->is_array = false; + this->is_subscripted = false; } /* For drivers that lower gl_ClipDistance to gl_ClipDistanceMESA, we need @@ -1501,8 +1507,10 @@ tfeedback_decl::init(struct gl_context *ctx, struct gl_shader_program *prog, if (ctx->ShaderCompilerOptions[MESA_SHADER_VERTEX].LowerClipDistance && strcmp(this->var_name, "gl_ClipDistance") == 0) { this->var_name = "gl_ClipDistanceMESA"; - this->single_component = this->array_index % 4; - this->array_index /= 4; + if (this->is_subscripted) { + this->single_component = this->array_subscript % 4; + this->array_subscript /= 4; + } } return true; @@ -1518,9 +1526,9 @@ tfeedback_decl::is_same(const tfeedback_decl &x, const tfeedback_decl &y) { if (strcmp(x.var_name, y.var_name) != 0) return false; - if (x.is_array != y.is_array) + if (x.is_subscripted != y.is_subscripted) return false; - if (x.is_array && x.array_index != y.array_index) + if (x.is_subscripted && x.array_subscript != y.array_subscript) return false; if (x.single_component != y.single_component) return false; @@ -1542,37 +1550,39 @@ tfeedback_decl::assign_location(struct gl_context *ctx, { if (output_var->type->is_array()) { /* Array variable */ - if (!this->is_array) { - linker_error(prog, "Transform feedback varying %s found, " - "but array dereference required for varying %s[%d]).", - this->orig_name, - output_var->name, output_var->type->length); - return false; - } - /* Check array bounds. */ - if (this->array_index >= - (unsigned) output_var->type->array_size()) { - linker_error(prog, "Transform feedback varying %s has index " - "%i, but the array size is %i.", - this->orig_name, this->array_index, - output_var->type->array_size()); - return false; - } const unsigned matrix_cols = output_var->type->fields.array->matrix_columns; - this->location = output_var->location + this->array_index * matrix_cols; + + if (this->is_subscripted) { + /* Check array bounds. */ + if (this->array_subscript >= + (unsigned) output_var->type->array_size()) { + linker_error(prog, "Transform feedback varying %s has index " + "%i, but the array size is %i.", + this->orig_name, this->array_subscript, + output_var->type->array_size()); + return false; + } + this->location = + output_var->location + this->array_subscript * matrix_cols; + this->size = 1; + } else { + this->location = output_var->location; + this->size = (unsigned) output_var->type->array_size(); + } this->vector_elements = output_var->type->fields.array->vector_elements; this->matrix_columns = matrix_cols; - this->type = GL_NONE; + this->type = output_var->type->fields.array->gl_type; } else { /* Regular variable (scalar, vector, or matrix) */ - if (this->is_array) { + if (this->is_subscripted) { linker_error(prog, "Transform feedback varying %s found, " "but it's an array ([] expected).", this->orig_name); return false; } this->location = output_var->location; + this->size = 1; this->vector_elements = output_var->type->vector_elements; this->matrix_columns = output_var->type->matrix_columns; this->type = output_var->type->gl_type; @@ -1621,26 +1631,25 @@ tfeedback_decl::store(struct gl_shader_program *prog, this->orig_name); return false; } - for (unsigned v = 0; v < this->matrix_columns; ++v) { - unsigned num_components = - this->single_component >= 0 ? 1 : this->vector_elements; - info->Outputs[info->NumOutputs].OutputRegister = this->location + v; - info->Outputs[info->NumOutputs].NumComponents = num_components; - info->Outputs[info->NumOutputs].OutputBuffer = buffer; - info->Outputs[info->NumOutputs].DstOffset = info->BufferStride[buffer]; - info->Outputs[info->NumOutputs].ComponentOffset = - this->single_component >= 0 ? this->single_component : 0; - ++info->NumOutputs; - info->BufferStride[buffer] += num_components; + for (unsigned index = 0; index < this->size; ++index) { + for (unsigned v = 0; v < this->matrix_columns; ++v) { + unsigned num_components = + this->single_component >= 0 ? 1 : this->vector_elements; + info->Outputs[info->NumOutputs].OutputRegister = + this->location + v + index * this->matrix_columns; + info->Outputs[info->NumOutputs].NumComponents = num_components; + info->Outputs[info->NumOutputs].OutputBuffer = buffer; + info->Outputs[info->NumOutputs].DstOffset = info->BufferStride[buffer]; + info->Outputs[info->NumOutputs].ComponentOffset = + this->single_component >= 0 ? this->single_component : 0; + ++info->NumOutputs; + info->BufferStride[buffer] += num_components; + } } info->Varyings[varying].Name = ralloc_strdup(prog, this->orig_name); info->Varyings[varying].Type = this->type; - /* Since we require that transform feedback varyings dereference - * arrays, there's always only one element of the GL datatype above - * present in this varying. - */ - info->Varyings[varying].Size = 1; + info->Varyings[varying].Size = this->size; info->NumVarying++; return true; -- 2.7.4