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

Christoph Bumiller e0425955 at student.tuwien.ac.at
Tue Feb 7 08:51:18 PST 2012


On 02/07/2012 05:24 PM, Jose Fonseca wrote:
> 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.
> 

glean's clipFlat tests this, with this patch nv50 and nvc0 will finally
pass.

I tested drawing a clipped quad with softpipe with both the cap enabled
and disabled and it looks like the draw change works.

So, I'll push it soon, then - r600 will get to profit from the cap too,
as Marek already mentioned.

> 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