[Mesa-dev] [PATCH 2/6] mesa: Introduce a compiler enum for tessellation spacing.

[Kenneth Graunke]

On Monday, January 9, 2017 9:47:36 AM PST Nicolai Hähnle wrote:
> On 07.01.2017 09:02, Kenneth Graunke wrote:
> > It feels weird using GL_* enums in a Vulkan driver.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/compiler/glsl/ast.h                  |  2 +-
> >  src/compiler/glsl/glsl_parser.yy         |  8 ++++----
> >  src/compiler/glsl/glsl_parser_extras.cpp |  2 +-
> >  src/compiler/glsl/linker.cpp             |  6 +++---
> >  src/compiler/shader_enums.h              |  8 ++++++++
> >  src/compiler/shader_info.h               |  2 +-
> >  src/mesa/drivers/dri/i965/brw_tcs.c      |  4 ++--
> >  src/mesa/drivers/dri/i965/brw_tes.c      | 20 +++++++-------------
> >  src/mesa/main/mtypes.h                   |  8 +++-----
> >  src/mesa/main/shaderapi.c                | 18 ++++++++++++++++--
> >  src/mesa/state_tracker/st_program.c      | 23 ++++++++---------------
> >  11 files changed, 54 insertions(+), 47 deletions(-)
> >
> [snip]
> > diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c
> > index 7d548d51c13..544cbb0bbd5 100644
> > --- a/src/mesa/state_tracker/st_program.c
> > +++ b/src/mesa/state_tracker/st_program.c
> > @@ -1607,21 +1607,14 @@ st_translate_tesseval_program(struct st_context *st,
> >        ureg_property(ureg, TGSI_PROPERTY_TES_PRIM_MODE,
> >                      sttep->Base.info.tes.primitive_mode);
> >
> > -   switch (sttep->Base.info.tes.spacing) {
> > -   case GL_EQUAL:
> > -      ureg_property(ureg, TGSI_PROPERTY_TES_SPACING, PIPE_TESS_SPACING_EQUAL);
> > -      break;
> > -   case GL_FRACTIONAL_EVEN:
> > -      ureg_property(ureg, TGSI_PROPERTY_TES_SPACING,
> > -                    PIPE_TESS_SPACING_FRACTIONAL_EVEN);
> > -      break;
> > -   case GL_FRACTIONAL_ODD:
> > -      ureg_property(ureg, TGSI_PROPERTY_TES_SPACING,
> > -                    PIPE_TESS_SPACING_FRACTIONAL_ODD);
> > -      break;
> > -   default:
> > -      assert(0);
> > -   }
> > +   STATIC_ASSERT((TESS_SPACING_EQUAL - 2) % 3 == PIPE_TESS_SPACING_EQUAL);
> > +   STATIC_ASSERT((TESS_SPACING_FRACTIONAL_ODD - 2) % 3 ==
> > +                 PIPE_TESS_SPACING_FRACTIONAL_ODD);
> > +   STATIC_ASSERT((TESS_SPACING_FRACTIONAL_EVEN - 2) % 3 ==
> > +                 PIPE_TESS_SPACING_FRACTIONAL_EVEN);
> > +
> > +   ureg_property(ureg, TGSI_PROPERTY_TES_SPACING,
> > +                 (sttep->Base.info.tes.spacing - 2) % 3);
> 
> Using + 1 instead of - 2 should work just as well, right? I'd prefer 
> that -- modulo with negative values makes me uncomfortable. Apart from 
> that, patches 1 & 2 are
> 
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> (the non-i965-parts, anyway)

I pushed these already, sorry :(  Thanks for looking at them though!

I did change it to + 1 instead of - 2.  It turns out that negative
values here did do the wrong thing, causing all the static asserts to
trip.  (In Python - which I use for quick calculator stuff - it gives
positive values, but in C...not so much...)  Fortunately, as you
suggested, + 1 works fine.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170109/fa1e3417/attachment-0001.sig>