[Mesa-dev] [PATCH] glsl: Fix cross-version linking between VS and GS.

[Ian Romanick]

On 11/19/2013 06:11 PM, Paul Berry wrote:
> Previously, when attempting to link a vertex shader and a geometry
> shader that use different GLSL versions, we would sometimes generate a
> link error due to the implicit declaration of gl_PerVertex being
> different between the two GLSL versions.
> 
> This patch fixes that problem by only requiring interface block
> definitions to match when they are explicitly declared.
> 
> Fixes piglit test "shaders/version-mixing vs-gs".

Which type do you get?  What happens when the linked shader runs?

Do we know what other implementations do for mixed VS / GS versions?

> Cc: "10.0" <mesa-stable at lists.freedesktop.org>
> ---
> 
> This patch depends on "[PATCH v2] glsl: Prohibit illegal mixing of
> redeclarations inside/outside gl_PerVertex.", which was sent out for
> review earlier today.
> 
>  src/glsl/link_interface_blocks.cpp | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/src/glsl/link_interface_blocks.cpp b/src/glsl/link_interface_blocks.cpp
> index a7fceb9..b4379b0 100644
> --- a/src/glsl/link_interface_blocks.cpp
> +++ b/src/glsl/link_interface_blocks.cpp
> @@ -59,6 +59,9 @@ struct interface_block_definition
>           instance_name = var->name;
>           if (var->type->is_array())
>              array_size = var->type->length;
> +         explicitly_declared = (var->how_declared == ir_var_declared_normally);
> +      } else {
> +         explicitly_declared = (var->how_declared == ir_var_declared_in_block);

Would it be easier to just have

   explicitly_declared = (var->how_declared != ir_var_declared_implicitly);

after the existing if-block?

>        }
>     }
>  
> @@ -77,6 +80,12 @@ struct interface_block_definition
>      * 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.
> +    */
> +   bool explicitly_declared;
>  };
>  
>  
> @@ -91,8 +100,14 @@ intrastage_match(interface_block_definition *a,
>                   ir_variable_mode mode)
>  {
>     /* Types must match. */
> -   if (a->type != b->type)
> -      return false;
> +   if (a->type != b->type) {
> +      /* Exception: if both the interface blocks are implicitly declared,
> +       * don't force their types to match.  They might mismatch due to the two
> +       * shaders using different GLSL versions, and that's ok.
> +       */
> +      if (a->explicitly_declared || b->explicitly_declared)
> +         return false;
> +   }
>  
>     /* Presence/absence of interface names must match. */
>     if ((a->instance_name == NULL) != (b->instance_name == NULL))
> @@ -144,8 +159,14 @@ interstage_match(const interface_block_definition *producer,
>     assert(producer->array_size != 0);
>  
>     /* Types must match. */
> -   if (consumer->type != producer->type)
> -      return false;
> +   if (consumer->type != producer->type) {
> +      /* Exception: if both the interface blocks are implicitly declared,
> +       * don't force their types to match.  They might mismatch due to the two
> +       * shaders using different GLSL versions, and that's ok.
> +       */
> +      if (consumer->explicitly_declared || producer->explicitly_declared)
> +         return false;
> +   }
>     if (extra_array_level) {
>        /* Consumer must be an array, and producer must not. */
>        if (consumer->array_size == -1)
>