[Mesa-dev] [PATCH RFC] gallium: interface changes necessary to implement transform feedback
Brian Paul
brianp at vmware.com
Thu Oct 6 07:50:59 PDT 2011
On 10/04/2011 06:51 PM, Marek Olšák wrote:
> Namely:
> - EXT_transform_feedback
> - ARB_transform_feedback2
> - ARB_transform_feedback_instanced
>
> The old interface was unusable for OpenGL and had to go away.
>
> This interface was originally designed for OpenGL, but additional
> changes have been made in order to make st/d3d1x support easier.
>
> The most notable change is the stream-out info must be linked
> with a vertex or geometry shader and cannot be set independently.
> This is due to limitations of existing hardware (special shader
> instructions must be used to write into stream-out buffers),
> and it's also how OpenGL works (stream outputs must be specified
> prior to linking shaders).
>
> Other than that, each stream output buffer has a "view" into it that
> internally maintains the number of bytes which have been written
> into it. (one buffer can be bound in several different transform
> feedback objects in OpenGL, so we must be able to have several views
> around) This counter can optionally be re-set to zero. Also,
> the counter can optionally be used to provide the vertex count
> for draw_vbo. Note that the counter is supposed to be stored in device
> memory and the CPU never gets to know its value.
>
> OpenGL way | Gallium way
> ------------------------------------
> BeginTF = reset + bind
> PauseTF = unbind
> ResumeTF = bind
> EndTF = unbind
> DrawTF = use pipe_draw_info::count_from_stream_output
Thanks for digging into this. I was working on xformfb a while back
(and still have some patches sitting around) but was far from
finishing it. If you have any interest in those patches I could send
them to you.
> ---
> src/gallium/auxiliary/draw/draw_context.c | 4 +-
> src/gallium/auxiliary/draw/draw_context.h | 2 +-
> src/gallium/auxiliary/draw/draw_private.h | 2 +-
> src/gallium/auxiliary/draw/draw_pt_so_emit.c | 2 +-
> src/gallium/drivers/llvmpipe/lp_state.h | 2 +-
> src/gallium/drivers/llvmpipe/lp_state_so.c | 2 +-
> src/gallium/drivers/nvc0/nvc0_state.c | 2 +-
> src/gallium/drivers/softpipe/sp_state.h | 2 +-
> src/gallium/drivers/softpipe/sp_state_so.c | 2 +-
> src/gallium/include/pipe/p_context.h | 47 +++++++++-----
> src/gallium/include/pipe/p_defines.h | 5 +-
> src/gallium/include/pipe/p_state.h | 83 ++++++++++++++++++++------
> 12 files changed, 109 insertions(+), 46 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
> index 6a85b79..6be0e23 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -721,11 +721,11 @@ draw_set_mapped_so_buffers(struct draw_context *draw,
>
> void
> draw_set_so_state(struct draw_context *draw,
> - struct pipe_stream_output_state *state)
> + struct pipe_stream_output_info *state)
> {
> memcpy(&draw->so.state,
> state,
> - sizeof(struct pipe_stream_output_state));
> + sizeof(struct pipe_stream_output_info));
> }
>
> void
> diff --git a/src/gallium/auxiliary/draw/draw_context.h b/src/gallium/auxiliary/draw/draw_context.h
> index 799eb94..93577d0 100644
> --- a/src/gallium/auxiliary/draw/draw_context.h
> +++ b/src/gallium/auxiliary/draw/draw_context.h
> @@ -202,7 +202,7 @@ draw_set_mapped_so_buffers(struct draw_context *draw,
> unsigned num_buffers);
> void
> draw_set_so_state(struct draw_context *draw,
> - struct pipe_stream_output_state *state);
> + struct pipe_stream_output_info *state);
>
>
> /***********************************************************************
> diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
> index ef77266..533b058 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -270,7 +270,7 @@ struct draw_context
>
> /** Stream output (vertex feedback) state */
> struct {
> - struct pipe_stream_output_state state;
> + struct pipe_stream_output_info state;
> void *buffers[PIPE_MAX_SO_BUFFERS];
> uint num_buffers;
> } so;
> diff --git a/src/gallium/auxiliary/draw/draw_pt_so_emit.c b/src/gallium/auxiliary/draw/draw_pt_so_emit.c
> index c86bdd9..6465e5c 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_so_emit.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_so_emit.c
> @@ -130,7 +130,7 @@ static void so_emit_prim(struct pt_so_emit *so,
> unsigned input_vertex_stride = so->input_vertex_stride;
> struct draw_context *draw = so->draw;
> const float (*input_ptr)[4];
> - const struct pipe_stream_output_state *state =
> + const struct pipe_stream_output_info *state =
> &draw->so.state;
> float **buffer = 0;
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_state.h b/src/gallium/drivers/llvmpipe/lp_state.h
> index 7893e9c..97ca172 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state.h
> +++ b/src/gallium/drivers/llvmpipe/lp_state.h
> @@ -86,7 +86,7 @@ struct lp_velems_state
> };
>
> struct lp_so_state {
> - struct pipe_stream_output_state base;
> + struct pipe_stream_output_info base;
> };
Maybe call this lp_so_info to avoid mixing "state" and "info"?
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_so.c b/src/gallium/drivers/llvmpipe/lp_state_so.c
> index 35de52c..4874bd6 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_so.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_so.c
> @@ -35,7 +35,7 @@
>
> static void *
> llvmpipe_create_stream_output_state(struct pipe_context *pipe,
> - const struct pipe_stream_output_state *templ)
> + const struct pipe_stream_output_info *templ)
> {
> struct lp_so_state *so;
> so = (struct lp_so_state *) CALLOC_STRUCT(lp_so_state);
> diff --git a/src/gallium/drivers/nvc0/nvc0_state.c b/src/gallium/drivers/nvc0/nvc0_state.c
> index 1a37d04..ebeeb7f 100644
> --- a/src/gallium/drivers/nvc0/nvc0_state.c
> +++ b/src/gallium/drivers/nvc0/nvc0_state.c
> @@ -749,7 +749,7 @@ nvc0_vertex_state_bind(struct pipe_context *pipe, void *hwcso)
>
> static void *
> nvc0_tfb_state_create(struct pipe_context *pipe,
> - const struct pipe_stream_output_state *pso)
> + const struct pipe_stream_output_info *pso)
> {
> struct nvc0_transform_feedback_state *so;
> int n = 0;
> diff --git a/src/gallium/drivers/softpipe/sp_state.h b/src/gallium/drivers/softpipe/sp_state.h
> index ec4c8cf..bdda294 100644
> --- a/src/gallium/drivers/softpipe/sp_state.h
> +++ b/src/gallium/drivers/softpipe/sp_state.h
> @@ -122,7 +122,7 @@ struct sp_velems_state {
> };
>
> struct sp_so_state {
> - struct pipe_stream_output_state base;
> + struct pipe_stream_output_info base;
> };
Again, maybe call this sp_so_info.
> diff --git a/src/gallium/drivers/softpipe/sp_state_so.c b/src/gallium/drivers/softpipe/sp_state_so.c
> index 40e5634..b0449cd 100644
> --- a/src/gallium/drivers/softpipe/sp_state_so.c
> +++ b/src/gallium/drivers/softpipe/sp_state_so.c
> @@ -36,7 +36,7 @@
>
> static void *
> softpipe_create_stream_output_state(struct pipe_context *pipe,
> - const struct pipe_stream_output_state *templ)
> + const struct pipe_stream_output_info *templ)
> {
> struct sp_so_state *so;
> so = (struct sp_so_state *) CALLOC_STRUCT(sp_so_state);
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index b2d5f95..9a4d3ee 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -56,7 +56,7 @@ struct pipe_sampler_view;
> struct pipe_scissor_state;
> struct pipe_shader_state;
> struct pipe_stencil_ref;
> -struct pipe_stream_output_state;
> +struct pipe_stream_output_target;
> struct pipe_surface;
> struct pipe_vertex_buffer;
> struct pipe_vertex_element;
> @@ -86,11 +86,6 @@ struct pipe_context {
> /*@{*/
> void (*draw_vbo)( struct pipe_context *pipe,
> const struct pipe_draw_info *info );
> -
> - /**
> - * Draw the stream output buffer at index 0
> - */
> - void (*draw_stream_output)( struct pipe_context *pipe, unsigned mode );
> /*@}*/
>
> /**
> @@ -179,11 +174,6 @@ struct pipe_context {
> void (*bind_vertex_elements_state)(struct pipe_context *, void *);
> void (*delete_vertex_elements_state)(struct pipe_context *, void *);
>
> - void * (*create_stream_output_state)(struct pipe_context *,
> - const struct pipe_stream_output_state *);
> - void (*bind_stream_output_state)(struct pipe_context *, void *);
> - void (*delete_stream_output_state)(struct pipe_context*, void*);
> -
> /*@}*/
>
> /**
> @@ -237,13 +227,36 @@ struct pipe_context {
> void (*set_index_buffer)( struct pipe_context *pipe,
> const struct pipe_index_buffer * );
>
> - void (*set_stream_output_buffers)(struct pipe_context *,
> - struct pipe_resource **buffers,
> - int *offsets, /*array of offsets
> - from the start of each
> - of the buffers */
> - int num_buffers);
> + /*@}*/
> +
> + /**
> + * Stream output functions.
> + */
> + /*@{*/
> +
> + struct pipe_stream_output_target *(*create_stream_output_target)(
> + struct pipe_context *,
> + struct pipe_resource *,
> + unsigned buffer_offset,
> + unsigned buffer_size);
> +
> + void (*stream_output_target_destroy)(struct pipe_context *,
> + struct pipe_stream_output_target *);
The naming of pipe_context methods has become a bit mixed up.
Initially, we were using "create_xxx()" and "delete_xxx()" but now
we've got "destroy_xxx()" and "xxx_destroy()" too. You're being
consistent with some of the newer methods but someday was should
probably clean up our naming conventions.
> + void (*set_stream_output_targets)(struct pipe_context *,
> + unsigned num_targets,
> + struct pipe_stream_output_target **targets);
> +
> + /**
> + * By default, new data is appended at the end of stream output buffers.
> + * This call resets the internal offset to 0, such that new data is
> + * written starting at the beginning of the range specified by
> + * pipe_stream_output_target.
> + *
> + * This function doesn't change any context state or bindings. */
> + void (*reset_stream_output_targets)(struct pipe_context *,
> + unsigned num_targets,
> + struct pipe_stream_output_target **targets);
> /*@}*/
>
>
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index acae4b1..a825fcd 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -472,7 +472,10 @@ enum pipe_cap {
> PIPE_CAP_MIN_TEXEL_OFFSET = 50,
> PIPE_CAP_MAX_TEXEL_OFFSET = 51,
> PIPE_CAP_CONDITIONAL_RENDER = 52,
> - PIPE_CAP_TEXTURE_BARRIER = 53
> + PIPE_CAP_TEXTURE_BARRIER = 53,
> + PIPE_CAP_MAX_STREAM_OUTPUT_SEPARATE_ATTRIBS = 54,
> + PIPE_CAP_MAX_STREAM_OUTPUT_SEPARATE_COMPONENTS = 55,
> + PIPE_CAP_MAX_STREAM_OUTPUT_INTERLEAVED_COMPONENTS = 56
> };
>
> /* Shader caps not specific to any single stage */
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 37e0679..517d9bb 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -121,6 +121,12 @@ struct pipe_rasterizer_state
> */
> unsigned gl_rasterization_rules:1;
>
> + /**
> + * When true, rasterization is disabled and no pixels are written.
> + * This only makes sense with the Stream Out functionality.
> + */
> + unsigned rasterizer_discard:1;
> +
> unsigned line_stipple_factor:8; /**< [1..256] actually */
> unsigned line_stipple_pattern:16;
>
> @@ -164,9 +170,28 @@ struct pipe_clip_state
> };
>
>
> +/**
> + * Stream output for vertex transform feedback.
> + */
> +struct pipe_stream_output_info
> +{
> + /** number of the output buffer to insert each element into */
> + unsigned output_buffer[PIPE_MAX_SHADER_OUTPUTS];
> + /** which register OUT[i] to grab each output from */
> + unsigned register_index[PIPE_MAX_SHADER_OUTPUTS];
> + /** TGSI_WRITEMASK signifying which components to output */
> + ubyte register_mask[PIPE_MAX_SHADER_OUTPUTS];
> + /** number of outputs */
> + unsigned num_outputs;
> + /** stride for an entire vertex, only used if all output_buffers are 0 */
> + unsigned stride;
> +};
> +
> +
> struct pipe_shader_state
> {
> const struct tgsi_token *tokens;
> + struct pipe_stream_output_info stream_output;
> };
>
>
> @@ -375,24 +400,6 @@ struct pipe_resource
>
>
> /**
> - * Stream output for vertex transform feedback.
> - */
> -struct pipe_stream_output_state
> -{
> - /** number of the output buffer to insert each element into */
> - int output_buffer[PIPE_MAX_SHADER_OUTPUTS];
> - /** which register to grab each output from */
> - int register_index[PIPE_MAX_SHADER_OUTPUTS];
> - /** TGSI_WRITEMASK signifying which components to output */
> - ubyte register_mask[PIPE_MAX_SHADER_OUTPUTS];
> - /** number of outputs */
> - int num_outputs;
> - /** stride for an entire vertex, only used if all output_buffers are 0 */
> - unsigned stride;
> -};
> -
> -
> -/**
> * Transfer object. For data transfer to/from a resource.
> */
> struct pipe_transfer
> @@ -422,6 +429,30 @@ struct pipe_vertex_buffer
>
>
> /**
> + * A stream output target. The structure specifies the range vertices can
> + * be written to.
> + *
> + * In addition to that, the structure should internally maintain the offset
> + * into the buffer, which should be incremented everytime something is written
> + * (appended) to it. The internal offset is buffer_offset + how many bytes
> + * have been written. The internal offset can be stored on the device
> + * and the CPU actually doesn't have to query it.
> + *
> + * Use PIPE_QUERY_SO_STATISTICS to know how much vertices have
> + * actually been written.
> + */
> +struct pipe_stream_output_target
> +{
> + struct pipe_reference reference;
> + struct pipe_resource *buffer; /**< buffer into which this is a target view */
> + struct pipe_context *context; /**< context this view belongs to */
> +
> + unsigned buffer_offset; /**< offset where data should be written, in bytes */
> + unsigned buffer_size; /**< how much data is allowed to be written */
When would the buffer_size be less than the buffer/resource's size?
> +};
> +
> +
> +/**
> * Information to describe a vertex attribute (position, color, etc)
> */
> struct pipe_vertex_element
> @@ -481,6 +512,22 @@ struct pipe_draw_info
> */
> boolean primitive_restart;
> unsigned restart_index;
> +
> + /**
> + * Stream output target. If not NULL, it's used to provide the 'count'
> + * parameter based on the number vertices captured by the stream output
> + * stage. (or generally, based on the number of bytes captured)
> + *
> + * Only 'mode', 'start_instance', and 'instance_count' are taken into
> + * account, all the other variables from pipe_draw_info are ignored.
> + *
> + * 'start' is implicitly 0 and 'count' is set as discussed above.
> + * The draw command is non-indexed.
> + *
> + * Note that this only provides the count. The vertex buffers must
> + * be set via set_vertex_buffers manually.
> + */
> + struct pipe_stream_output_target *count_from_stream_output;
> };
I'd have to do some digging, but I seem to recall that
pipe_vertex_element might be used as a hash key somewhere. I'm
wondering if adding a pointer to this struct could upset the hashing
(artificially increase the number of hash keys/entries).
Looks good otherwise. I assume you'll update the docs too if nobody
suggests other changes.
-Brian
More information about the mesa-dev
mailing list