[Mesa-dev] [PATCH 03/11] gallium: interface changes necessary to implement transform feedback (v5)

[Jose Fonseca]

----- Original Message -----
> From: Marek Olšák <maraeo at gmail.com>
> 
> Namely:
> - EXT_transform_feedback
> - ARB_transform_feedback2
> - ARB_transform_feedback_instanced
> 
> The old interface was not useful for OpenGL and had to be reworked.
> 
> 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) The set_stream_output_targets function contains a parameter
> saying whether new data should be appended or not.
> 
> Also, the view can optionally be used to provide the vertex
> count for draw_vbo. Note that the count is supposed to be stored
> in device memory and the CPU never gets to know its value.
> 
> OpenGL way | Gallium way
> ------------------------------------
> BeginTF    = set_so_targets(append_bitmask = 0)
> PauseTF    = set_so_targets(num_targets = 0)
> ResumeTF   = set_so_targets(append_bitmask = ~0)
> EndTF      = set_so_targets(num_targets = 0)
> DrawTF     = use pipe_draw_info::count_from_stream_output
> 
> v2: * removed the reset_stream_output_targets function
>     * added a parameter append_bitmask to set_stream_output_targets,
>       each bit specifies whether new data should be appended to each
>       buffer or not.
> v3: * added PIPE_CAP_STREAM_OUTPUT_PAUSE_RESUME for ARB_tfb2,
>       note that the draw-auto subset is always required (for d3d10),
>       only the pause/resume functionality is limited if the CAP is
>       not
>       advertised
> v4: * update gallium/docs
> v5: * compactified struct pipe_stream_output_info, updated dump/trace
[...]

Thanks for the update. Just a minor suggestion:

> @@ -267,6 +268,24 @@ void trace_dump_shader_state(const struct
> pipe_shader_state *state)
>     trace_dump_string(str);
>     trace_dump_member_end();
>  
> +   trace_dump_member_begin("stream_output");
> +   trace_dump_struct_begin("pipe_stream_output_info");
> +   trace_dump_member(uint, &state->stream_output, num_outputs);
> +   trace_dump_member(uint, &state->stream_output, stride);
> +   trace_dump_array_begin();
> +   for(i = 0; i < Elements(state->stream_output.output); ++i) {

It should be state->stream_output.num_outputs IIUC.

Other than that, 

Reviewed-By: Jose Fonseca <jfonseca at vmware.com>

Jose