[Mesa-dev] [PATCH 1/3] gallium/tgsi: add missing string for layer semantic

[Brian Paul]

On Wed, Jun 5, 2013 at 10:11 PM, Roland Scheidegger <sroland at vmware.com>wrote:

> Am 06.06.2013 04:16, schrieb Roland Scheidegger:
> > Am 06.06.2013 02:52, schrieb Brian Paul:
> >> On 06/05/2013 05:44 PM, sroland at vmware.com wrote:
> >>> From: Roland Scheidegger <sroland at vmware.com>
> >>>
> >>> Also report if a shader writes the layer semantic
> >>> ---
> >>>   src/gallium/auxiliary/draw/draw_context.c |    2 +-
> >>>   src/gallium/auxiliary/tgsi/tgsi_scan.c    |    5 +++++
> >>>   src/gallium/auxiliary/tgsi/tgsi_scan.h    |    1 +
> >>>   src/gallium/auxiliary/tgsi/tgsi_strings.c |    1 +
> >>>   4 files changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/gallium/auxiliary/draw/draw_context.c
> >>> b/src/gallium/auxiliary/draw/draw_context.c
> >>> index 58ce270..35063b9 100644
> >>> --- a/src/gallium/auxiliary/draw/draw_context.c
> >>> +++ b/src/gallium/auxiliary/draw/draw_context.c
> >>> @@ -548,7 +548,7 @@ draw_get_shader_info(const struct draw_context
> *draw)
> >>>    * function to find those attributes.
> >>>    *
> >>>    * -1 is returned if the attribute is not found since this is
> >>> - * an undefined situtation. Note, that zero is valid and can
> >>> + * an undefined situation. Note, that zero is valid and can
> >>>    * be used by any of the attributes, because position is not
> >>>    * required to be attribute 0 or even at all present.
> >>>    */
> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c
> >>> b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> >>> index 0230267..d331257 100644
> >>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
> >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> >>> @@ -217,6 +217,11 @@ tgsi_scan_shader(const struct tgsi_token *tokens,
> >>>                         TGSI_SEMANTIC_VIEWPORT_INDEX) {
> >>>                        info->writes_viewport_index = TRUE;
> >>>                     }
> >>> +                  if (procType == TGSI_PROCESSOR_GEOMETRY &&
> >>> +                      fulldecl->Semantic.Name ==
> >>> +                      TGSI_SEMANTIC_LAYER) {
> >>> +                     info->writes_layer = TRUE;
> >>> +                  }
> >>>                  }
> >>>
> >>>                }
> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.h
> >>> b/src/gallium/auxiliary/tgsi/tgsi_scan.h
> >>> index 676abf0..a5b7024 100644
> >>> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.h
> >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.h
> >>> @@ -76,6 +76,7 @@ struct tgsi_shader_info
> >>>      boolean pixel_center_integer;
> >>>      boolean color0_writes_all_cbufs;
> >>>      boolean writes_viewport_index;
> >>> +   boolean writes_layer;
> >>>
> >>>      unsigned num_written_clipdistance;
> >>>      /**
> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_strings.c
> >>> b/src/gallium/auxiliary/tgsi/tgsi_strings.c
> >>> index 6abf927..625107c 100644
> >>> --- a/src/gallium/auxiliary/tgsi/tgsi_strings.c
> >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_strings.c
> >>> @@ -80,6 +80,7 @@ const char *tgsi_semantic_names[TGSI_SEMANTIC_COUNT]
> =
> >>>      "TEXCOORD",
> >>>      "PCOORD",
> >>>      "VIEWPORT_INDEX"
> >>> +   "LAYER"
> >>>   };
> >>>
> >>>   const char *tgsi_texture_names[TGSI_TEXTURE_COUNT] =
> >>>
> >>
> >> Hmm, there's a STATIC_ASSERT later in the file which should have
> >> detected a missing string in the array.  I bet we need to remove the
> >> TGSI_SEMANTIC_COUNT from the array size declaration to make that work.
> >> Can you check into that.
> > That is odd indeed. The patch was actually buggy (no comma). All of
> > these name arrays there are indeed fixed size which looks like defeating
> > the purpose of the STATIC_ASSERT. Even if I change this though it never
> > triggers no matter what's in there. Hmm.
> >
>
> So this can't work. The size of these arrays is declared elsewhere, what
> we put in in this file here doesn't really matter for the Elements used
> in STATIC_ASSERT at all. And we can't just make the size dynamic in
> there in tgsi_string.h as sizeof() used for Elements can't work with
> that. So I've no idea how to make this actually work.
>

OK, I see the problem now.  I'm going to post a patch which replaces the
tgsi_file_names[] array with a function.  I think this is nicer and it
gives us a working static assertions.

If you like it, I could create a bugzilla entry/task for someone to give
the same treatment to the other tgsi string arrays.

-Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130606/061f9741/attachment.html>