On 11 October 2013 14:13, Matt Turner <matts...@gmail.com> wrote: > On Fri, Oct 11, 2013 at 1:27 PM, Paul Berry <stereotype...@gmail.com> > wrote: > > Ivy Bridge's "reorder enable" bit gives us a binary choice for the > > order in which vertices from triangle strips are delivered to the > > geometry shader. Neither choice follows the OpenGL spec, but setting > > the bit is better, because it gets triangle orientation correct. > > > > Haswell replaces the "reorder enable" bit with a new "reorder mode" > > bit (which occupies the same location in the command packet). This > > bit gives us a different binary choice, which affects both triangle > > strips and triangle strips with adjacency. Setting the bit gives the > > proper order according to the OpenGL spec. > > > > So in either case we want to set the bit. > > > > On Ivy Bridge, fixes piglit test "triangle-strip-orientation". > > > > On Haswell, fixes piglit tests "glsl-1.50-geometry-primitive-types > > {GL_TRIANGLE_STRIP,GL_TRIANGLE_STRIP_ADJACENCY}" and > > "glsl-1.50-geometry-tri-strip-ordering-with-prim-restart *". > > --- > > src/mesa/drivers/dri/i965/brw_defines.h | 1 + > > src/mesa/drivers/dri/i965/gen7_gs_state.c | 28 > ++++++++++++++++++++++++++++ > > 2 files changed, 29 insertions(+) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > b/src/mesa/drivers/dri/i965/brw_defines.h > > index c1e7f31..902b7e8 100644 > > --- a/src/mesa/drivers/dri/i965/brw_defines.h > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > > @@ -1381,6 +1381,7 @@ enum brw_message_target { > > # define GEN6_GS_SO_STATISTICS_ENABLE (1 << 9) > > # define GEN6_GS_RENDERING_ENABLE (1 << 8) > > # define GEN7_GS_INCLUDE_PRIMITIVE_ID (1 << 4) > > +# define GEN7_GS_REORDER_MODE (1 << 2) > > # define GEN7_GS_ENABLE (1 << 0) > > /* DW6 */ > > # define HSW_GS_CONTROL_DATA_FORMAT_SHIFT 31 > > diff --git a/src/mesa/drivers/dri/i965/gen7_gs_state.c > b/src/mesa/drivers/dri/i965/gen7_gs_state.c > > index 3dd5896..bd5b666 100644 > > --- a/src/mesa/drivers/dri/i965/gen7_gs_state.c > > +++ b/src/mesa/drivers/dri/i965/gen7_gs_state.c > > @@ -105,6 +105,33 @@ upload_gs_state(struct brw_context *brw) > > (0 << GEN6_GS_URB_ENTRY_READ_OFFSET_SHIFT) | > > (prog_data->dispatch_grf_start_reg << > > GEN6_GS_DISPATCH_START_GRF_SHIFT); > > + > > + /* Note: the meaning of the GEN7_GS_REORDER_MODE bit changes > between Ivy > > + * Bridge and Haswell. > > + * > > + * On Ivy bridge, setting this bit causes the vertices of a > triangle > > You've capitalized Bridge everywhere else, so I suppose you meant to > here as well. >
Fixed, thanks. > > > + * strip to be delivered to the geometry shader in an order that > does > > + * not strictly follow the OpenGL spec, but preserves triangle > > + * orientation. For example, if the vertices are (1, 2, 3, 4, > 5), then > > + * the geometry shader sees triangles: > > + * > > + * (1, 2, 3), (2, 4, 3), (3, 4, 5) > > + * > > + * (Clearing the bit is even worse, because it fails to preserve > > + * orientation). > > + * > > + * Triangle strips with adjacency always ordered in a way that > preserves > > + * triangle orientation but does not strictly follow the OpenGL > spec, > > + * regardless of the setting of this bit. > > + * > > + * On Haswell, both triangle strips and triangle strips with > adjacency > > + * are always ordered in a way that preserves triangle > orientation. > > + * Setting this bit causes the ordering to strictly follow the > OpenGL > > + * spec. > > + * > > + * So in either case we want to set the bit. Unfortunately on Ivy > > + * Bridge this will get the order close to correct but not > perfect. > > + */ > > uint32_t dw5 = > > ((brw->max_gs_threads - 1) << max_threads_shift) | > > (brw->gs.prog_data->control_data_header_size_hwords << > > @@ -113,6 +140,7 @@ upload_gs_state(struct brw_context *brw) > > GEN6_GS_STATISTICS_ENABLE | > > (brw->gs.prog_data->include_primitive_id ? > > GEN7_GS_INCLUDE_PRIMITIVE_ID : 0) | > > + GEN7_GS_REORDER_MODE | > > The actual 1-bit is called REORDER_TRAILING on Haswell. Should we call > it that instead? > Yeah, that's a good point. I've renamed it. > > In any case: > > Reviewed-by: Matt Turner <matts...@gmail.com> > Thanks!
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev