[Mesa-dev] [PATCH] gallium: add PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION

Jose Fonseca jfonseca at vmware.com
Tue Feb 7 08:24:21 PST 2012


I'm not familiar enough with draw internals to tell whether your changes are enough or not.

But I'm OK with this change as is, given that most drivers will not advertise PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION, so only those that do advertise need to be tested to ensure that things are working correctly.

Is there a piglit test for QuadsFollowProvokingVertexConvention? It's probably a good idea to have one piglit test that tests it and draw clipped primitives w/ different fill modes.

Jose

----- Original Message -----
> Just let the hardware do it if it can and avoid drivers having to
> check for the special case on each draw call.
> 
> v2: update the draw module
> ---
>  src/gallium/auxiliary/draw/draw_context.c       |    7 ++++-
>  src/gallium/auxiliary/draw/draw_decompose_tmp.h |   26
>  +++++++++++++++-------
>  src/gallium/auxiliary/draw/draw_gs_tmp.h        |    1 +
>  src/gallium/auxiliary/draw/draw_private.h       |    2 +
>  src/gallium/auxiliary/draw/draw_pt_decompose.h  |    2 +
>  src/gallium/auxiliary/draw/draw_so_emit_tmp.h   |    1 +
>  src/gallium/docs/source/cso/rasterizer.rst      |    7 +++--
>  src/gallium/docs/source/screen.rst              |    2 +
>  src/gallium/drivers/i915/i915_screen.c          |    1 +
>  src/gallium/drivers/llvmpipe/lp_screen.c        |    2 +
>  src/gallium/drivers/nv50/nv50_screen.c          |    1 +
>  src/gallium/drivers/nvc0/nvc0_screen.c          |    1 +
>  src/gallium/drivers/nvfx/nvfx_screen.c          |    1 +
>  src/gallium/drivers/r300/r300_screen.c          |    1 +
>  src/gallium/drivers/r600/r600_pipe.c            |    1 +
>  src/gallium/drivers/softpipe/sp_screen.c        |    2 +
>  src/gallium/drivers/svga/svga_screen.c          |    3 ++
>  src/gallium/include/pipe/p_defines.h            |    3 +-
>  src/mesa/state_tracker/st_extensions.c          |    4 +-
>  19 files changed, 52 insertions(+), 16 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_context.c
> b/src/gallium/auxiliary/draw/draw_context.c
> index 3c0b1aa..ad49ce7 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -93,11 +93,11 @@ draw_create_context(struct pipe_context *pipe,
> boolean try_llvm,
>     }
>  #endif
>  
> +   draw->pipe = pipe;
> +
>     if (!draw_init(draw))
>        goto err_destroy;
>  
> -   draw->pipe = pipe;
> -
>     return draw;
>  
>  err_destroy:
> @@ -168,6 +168,9 @@ boolean draw_init(struct draw_context *draw)
>     if (!draw_gs_init( draw ))
>        return FALSE;
>  
> +   draw->quads_always_flatshade_last =
> !draw->pipe->screen->get_param(
> +      draw->pipe->screen,
> PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION);
> +
>     return TRUE;
>  }
>  
> diff --git a/src/gallium/auxiliary/draw/draw_decompose_tmp.h
> b/src/gallium/auxiliary/draw/draw_decompose_tmp.h
> index a142563..ee6877d 100644
> --- a/src/gallium/auxiliary/draw/draw_decompose_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_decompose_tmp.h
> @@ -193,13 +193,18 @@ FUNC(FUNC_VARS)
>              flags = DRAW_PIPE_RESET_STIPPLE |
>                      DRAW_PIPE_EDGE_FLAG_0 |
>                      DRAW_PIPE_EDGE_FLAG_1;
> -            /* XXX should always emit idx[0] first */
> -            /* always emit idx[3] first */
> -            TRIANGLE(flags, idx[3], idx[0], idx[1]);
> +            /* always emit idx[3] / idx[0] first */
> +            if (quads_flatshade_last)
> +               TRIANGLE(flags, idx[3], idx[0], idx[1]);
> +            else
> +               TRIANGLE(flags, idx[0], idx[1], idx[2]);
>  
>              flags = DRAW_PIPE_EDGE_FLAG_1 |
>                      DRAW_PIPE_EDGE_FLAG_2;
> -            TRIANGLE(flags, idx[3], idx[1], idx[2]);
> +            if (quads_flatshade_last)
> +               TRIANGLE(flags, idx[3], idx[1], idx[2]);
> +            else
> +               TRIANGLE(flags, idx[0], idx[2], idx[3]);
>           }
>        }
>        break;
> @@ -237,13 +242,18 @@ FUNC(FUNC_VARS)
>                 flags = DRAW_PIPE_RESET_STIPPLE |
>                         DRAW_PIPE_EDGE_FLAG_0 |
>                         DRAW_PIPE_EDGE_FLAG_1;
> -               /* XXX should always emit idx[0] first */
> -               /* always emit idx[3] first */
> -               TRIANGLE(flags, idx[3], idx[2], idx[0]);
> +               /* always emit idx[3] / idx[0 first */
> +               if (quads_flatshade_last)
> +                  TRIANGLE(flags, idx[3], idx[2], idx[0]);
> +               else
> +                  TRIANGLE(flags, idx[0], idx[3], idx[2]);
>  
>                 flags = DRAW_PIPE_EDGE_FLAG_1 |
>                         DRAW_PIPE_EDGE_FLAG_2;
> -               TRIANGLE(flags, idx[3], idx[0], idx[1]);
> +               if (quads_flatshade_last)
> +                  TRIANGLE(flags, idx[3], idx[0], idx[1]);
> +               else
> +                  TRIANGLE(flags, idx[0], idx[1], idx[3]);
>              }
>           }
>        }
> diff --git a/src/gallium/auxiliary/draw/draw_gs_tmp.h
> b/src/gallium/auxiliary/draw/draw_gs_tmp.h
> index de7b026..92b6fb7 100644
> --- a/src/gallium/auxiliary/draw/draw_gs_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_gs_tmp.h
> @@ -9,6 +9,7 @@
>     const unsigned prim = input_prims->prim;                       \
>     const unsigned prim_flags = input_prims->flags;                \
>     const unsigned count = input_prims->count;                     \
> +   const boolean quads_flatshade_last = FALSE;                    \
>     const boolean last_vertex_last = TRUE;                         \
>     do {                                                           \
>        debug_assert(input_prims->primitive_count == 1);            \
> diff --git a/src/gallium/auxiliary/draw/draw_private.h
> b/src/gallium/auxiliary/draw/draw_private.h
> index c3eca97..9e63803 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -204,6 +204,8 @@ struct draw_context
>        boolean guard_band_xy;
>     } driver;
>  
> +   boolean quads_always_flatshade_last;
> +
>     boolean flushing;         /**< debugging/sanity */
>     boolean suspend_flushing; /**< internally set */
>  
> diff --git a/src/gallium/auxiliary/draw/draw_pt_decompose.h
> b/src/gallium/auxiliary/draw/draw_pt_decompose.h
> index 3127aad..8637085 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_decompose.h
> +++ b/src/gallium/auxiliary/draw/draw_pt_decompose.h
> @@ -1,5 +1,7 @@
>  #define LOCAL_VARS                           \
>     char *verts = (char *) vertices;          \
> +   const boolean quads_flatshade_last =      \
> +      draw->quads_always_flatshade_last;     \
>     const boolean last_vertex_last =          \
>        !(draw->rasterizer->flatshade &&       \
>          draw->rasterizer->flatshade_first);
> diff --git a/src/gallium/auxiliary/draw/draw_so_emit_tmp.h
> b/src/gallium/auxiliary/draw/draw_so_emit_tmp.h
> index 7fafde9..ec31c3f 100644
> --- a/src/gallium/auxiliary/draw/draw_so_emit_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_so_emit_tmp.h
> @@ -9,6 +9,7 @@
>     /* declare more local vars */                                  \
>     const unsigned prim = input_prims->prim;                       \
>     const unsigned prim_flags = input_prims->flags;                \
> +   const boolean quads_flatshade_last = FALSE;                    \
>     const boolean last_vertex_last = TRUE;                         \
>     do {                                                           \
>        debug_assert(input_prims->primitive_count == 1);            \
> diff --git a/src/gallium/docs/source/cso/rasterizer.rst
> b/src/gallium/docs/source/cso/rasterizer.rst
> index 482b1ea..150e6df 100644
> --- a/src/gallium/docs/source/cso/rasterizer.rst
> +++ b/src/gallium/docs/source/cso/rasterizer.rst
> @@ -59,13 +59,14 @@ flatshade_first
>  Whether the first vertex should be the provoking vertex, for most
>  primitives.
>  If not set, the last vertex is the provoking vertex.
>  
> -There are several important exceptions to the specification of this
> rule.
> +There are a few important exceptions to the specification of this
> rule.
>  
>  * ``PIPE_PRIMITIVE_POLYGON``: The provoking vertex is always the
>  first
>    vertex. If the caller wishes to change the provoking vertex, they
>    merely
>    need to rotate the vertices themselves.
> -* ``PIPE_PRIMITIVE_QUAD``, ``PIPE_PRIMITIVE_QUAD_STRIP``: This
> option has no
> -  effect; the provoking vertex is always the last vertex.
> +* ``PIPE_PRIMITIVE_QUAD``, ``PIPE_PRIMITIVE_QUAD_STRIP``: The option
> only has
> +  an effect if ``PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION``
> is true.
> +  If it is not, the provoking vertex is always the last vertex.
>  * ``PIPE_PRIMITIVE_TRIANGLE_FAN``: When set, the provoking vertex is
>  the
>    second vertex, not the first. This permits each segment of the fan
>    to have
>    a different color.
> diff --git a/src/gallium/docs/source/screen.rst
> b/src/gallium/docs/source/screen.rst
> index 2af9f74..51d9464 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -96,6 +96,8 @@ The integer capabilities:
>    controlled through pipe_rasterizer_state.
>  * ``PIPE_CAP_GLSL_FEATURE_LEVEL``: Whether the driver supports
>  features
>    equivalent to a specific GLSL version. E.g. for GLSL 1.3, report
>    130.
> +* ``PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION``: Whether
> quads adhere to
> +  the flatshade_first setting in ``pipe_rasterizer_state``.
>  
>  
>  
> diff --git a/src/gallium/drivers/i915/i915_screen.c
> b/src/gallium/drivers/i915/i915_screen.c
> index aef0ed6..a37241f 100644
> --- a/src/gallium/drivers/i915/i915_screen.c
> +++ b/src/gallium/drivers/i915/i915_screen.c
> @@ -204,6 +204,7 @@ i915_get_param(struct pipe_screen *screen, enum
> pipe_cap cap)
>     case PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS:
>     case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
>     case PIPE_CAP_VERTEX_COLOR_UNCLAMPED:
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>        return 0;
>  
>     /* Features we can lie about (boolean caps). */
> diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c
> b/src/gallium/drivers/llvmpipe/lp_screen.c
> index fd6e439..7f0f17e 100644
> --- a/src/gallium/drivers/llvmpipe/lp_screen.c
> +++ b/src/gallium/drivers/llvmpipe/lp_screen.c
> @@ -157,6 +157,8 @@ llvmpipe_get_param(struct pipe_screen *screen,
> enum pipe_cap param)
>     case PIPE_CAP_MIXED_COLORBUFFER_FORMATS:
>     case PIPE_CAP_CONDITIONAL_RENDER:
>        return 1;
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
> +      return 0;
>     default:
>        return 0;
>     }
> diff --git a/src/gallium/drivers/nv50/nv50_screen.c
> b/src/gallium/drivers/nv50/nv50_screen.c
> index 904f39a..1d53593 100644
> --- a/src/gallium/drivers/nv50/nv50_screen.c
> +++ b/src/gallium/drivers/nv50/nv50_screen.c
> @@ -146,6 +146,7 @@ nv50_screen_get_param(struct pipe_screen
> *pscreen, enum pipe_cap param)
>     case PIPE_CAP_MIXED_COLORBUFFER_FORMATS:
>     case PIPE_CAP_CONDITIONAL_RENDER:
>     case PIPE_CAP_TEXTURE_BARRIER:
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>        return 1;
>     case PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS:
>     case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
> diff --git a/src/gallium/drivers/nvc0/nvc0_screen.c
> b/src/gallium/drivers/nvc0/nvc0_screen.c
> index 676af76..abc04ab 100644
> --- a/src/gallium/drivers/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nvc0/nvc0_screen.c
> @@ -132,6 +132,7 @@ nvc0_screen_get_param(struct pipe_screen
> *pscreen, enum pipe_cap param)
>     case PIPE_CAP_MIXED_COLORBUFFER_FORMATS:
>     case PIPE_CAP_CONDITIONAL_RENDER:
>     case PIPE_CAP_TEXTURE_BARRIER:
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>        return 1;
>     case PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS:
>     case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
> diff --git a/src/gallium/drivers/nvfx/nvfx_screen.c
> b/src/gallium/drivers/nvfx/nvfx_screen.c
> index ba1a242..d47558e 100644
> --- a/src/gallium/drivers/nvfx/nvfx_screen.c
> +++ b/src/gallium/drivers/nvfx/nvfx_screen.c
> @@ -98,6 +98,7 @@ nvfx_screen_get_param(struct pipe_screen *pscreen,
> enum pipe_cap param)
>  	case PIPE_CAP_MAX_STREAM_OUTPUT_INTERLEAVED_COMPONENTS:
>  	case PIPE_CAP_TGSI_CAN_COMPACT_VARYINGS:
>  	case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
> +	case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>                  return 0;
>  	default:
>  		NOUVEAU_ERR("Warning: unknown PIPE_CAP %d\n", param);
> diff --git a/src/gallium/drivers/r300/r300_screen.c
> b/src/gallium/drivers/r300/r300_screen.c
> index eb233a0..6b3b6c1 100644
> --- a/src/gallium/drivers/r300/r300_screen.c
> +++ b/src/gallium/drivers/r300/r300_screen.c
> @@ -140,6 +140,7 @@ static int r300_get_param(struct pipe_screen*
> pscreen, enum pipe_cap param)
>          case PIPE_CAP_MAX_STREAM_OUTPUT_INTERLEAVED_COMPONENTS:
>          case PIPE_CAP_STREAM_OUTPUT_PAUSE_RESUME:
>          case PIPE_CAP_FRAGMENT_COLOR_CLAMPED:
> +        case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>              return 0;
>  
>          /* SWTCL-only features. */
> diff --git a/src/gallium/drivers/r600/r600_pipe.c
> b/src/gallium/drivers/r600/r600_pipe.c
> index 140ae11..bfb01f5 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -391,6 +391,7 @@ static int r600_get_param(struct pipe_screen*
> pscreen, enum pipe_cap param)
>  	case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
>  	case PIPE_CAP_FRAGMENT_COLOR_CLAMPED:
>  	case PIPE_CAP_VERTEX_COLOR_CLAMPED:
> +	case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
>  		return 0;
>  
>  	/* Stream output. */
> diff --git a/src/gallium/drivers/softpipe/sp_screen.c
> b/src/gallium/drivers/softpipe/sp_screen.c
> index 6d61d00..5e50bfb 100644
> --- a/src/gallium/drivers/softpipe/sp_screen.c
> +++ b/src/gallium/drivers/softpipe/sp_screen.c
> @@ -134,6 +134,8 @@ softpipe_get_param(struct pipe_screen *screen,
> enum pipe_cap param)
>        return 1;
>     case PIPE_CAP_GLSL_FEATURE_LEVEL:
>        return 130;
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
> +      return 0;
>     default:
>        return 0;
>     }
> diff --git a/src/gallium/drivers/svga/svga_screen.c
> b/src/gallium/drivers/svga/svga_screen.c
> index 7359165..8d47e69 100644
> --- a/src/gallium/drivers/svga/svga_screen.c
> +++ b/src/gallium/drivers/svga/svga_screen.c
> @@ -203,6 +203,9 @@ svga_get_param(struct pipe_screen *screen, enum
> pipe_cap param)
>     case PIPE_CAP_MIXED_COLORBUFFER_FORMATS:
>        return 0;
>  
> +   case PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION:
> +      return 0;
> +
>     default:
>        return 0;
>     }
> diff --git a/src/gallium/include/pipe/p_defines.h
> b/src/gallium/include/pipe/p_defines.h
> index 88bd66c..4155178 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -488,7 +488,8 @@ enum pipe_cap {
>     PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS = 59, /* temporary */
>     PIPE_CAP_VERTEX_COLOR_UNCLAMPED = 60,
>     PIPE_CAP_VERTEX_COLOR_CLAMPED = 61,
> -   PIPE_CAP_GLSL_FEATURE_LEVEL = 62
> +   PIPE_CAP_GLSL_FEATURE_LEVEL = 62,
> +   PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION = 63
>  };
>  
>  /**
> diff --git a/src/mesa/state_tracker/st_extensions.c
> b/src/mesa/state_tracker/st_extensions.c
> index 443cb4b..fb36a68 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -146,8 +146,8 @@ void st_init_limits(struct st_context *st)
>        = CLAMP(screen->get_param(screen,
>        PIPE_CAP_MAX_RENDER_TARGETS),
>                1, MAX_DRAW_BUFFERS);
>  
> -   /* Quads always follow GL provoking rules. */
> -   c->QuadsFollowProvokingVertexConvention = GL_FALSE;
> +   c->QuadsFollowProvokingVertexConvention = screen->get_param(
> +      screen, PIPE_CAP_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION);
>  
>     for (sh = 0; sh < MESA_SHADER_TYPES; ++sh) {
>        struct gl_shader_compiler_options *options =
> --
> 1.7.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


More information about the mesa-dev mailing list