Re: [Mesa-dev] [PATCH] gallium: reorder fields in pipe_draw_info

[Marek Olšák]

On Mon, Feb 20, 2017 at 8:36 PM, Roland Scheidegger  wrote:
> Am 20.02.2017 um 20:08 schrieb Marek Olšák:
>> On Mon, Feb 20, 2017 at 8:03 PM, Roland Scheidegger  
>> wrote:
>>> This doesn't quite just do what the commit log says, since at least one
>>> parameter is changed from unsigned to ubyte (I hope vertices_per_patch
>>> can't exceed that for any driver...)
>>
>> vertices_per_patch must be in [1, 32]. Only 5 bits are necessary to
>> represent it.
>
> Well as far as I can tell at a very quick glance this is an
> implementation dependent limit.
> But I don't have any objections making it a ubyte if no implementations
> could handle more than 32 (or even 64, 128,...) anyway (should just
> mention it in the log that it's changed to ubyte).

Yeah, I've already updated the commit message.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium: reorder fields in pipe_draw_info

[Roland Scheidegger]

Am 20.02.2017 um 20:08 schrieb Marek Olšák:
> On Mon, Feb 20, 2017 at 8:03 PM, Roland Scheidegger  
> wrote:
>> This doesn't quite just do what the commit log says, since at least one
>> parameter is changed from unsigned to ubyte (I hope vertices_per_patch
>> can't exceed that for any driver...)
> 
> vertices_per_patch must be in [1, 32]. Only 5 bits are necessary to
> represent it.

Well as far as I can tell at a very quick glance this is an
implementation dependent limit.
But I don't have any objections making it a ubyte if no implementations
could handle more than 32 (or even 64, 128,...) anyway (should just
mention it in the log that it's changed to ubyte).

Roland

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium: reorder fields in pipe_draw_info

[Marek Olšák]

On Mon, Feb 20, 2017 at 8:03 PM, Roland Scheidegger  wrote:
> This doesn't quite just do what the commit log says, since at least one
> parameter is changed from unsigned to ubyte (I hope vertices_per_patch
> can't exceed that for any driver...)

vertices_per_patch must be in [1, 32]. Only 5 bits are necessary to
represent it.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium: reorder fields in pipe_draw_info

[Roland Scheidegger]

This doesn't quite just do what the commit log says, since at least one
parameter is changed from unsigned to ubyte (I hope vertices_per_patch
can't exceed that for any driver...)

Otherwise seems reasonable.

Roland



Am 20.02.2017 um 19:35 schrieb Marek Olšák:
> From: Marek Olšák 
> 
> sizeof(struct pipe_draw_info) = 104 -> 88
> ---
>  src/gallium/include/pipe/p_state.h | 49 
> --
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/src/gallium/include/pipe/p_state.h 
> b/src/gallium/include/pipe/p_state.h
> index cb72310..ce19b92 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -629,60 +629,51 @@ struct pipe_index_buffer
> const void *user_buffer;  /**< pointer to a user buffer if buffer == NULL 
> */
>  };
>  
>  
>  /**
>   * Information to describe a draw_vbo call.
>   */
>  struct pipe_draw_info
>  {
> boolean indexed;  /**< use index buffer */
> -
> enum pipe_prim_type mode;  /**< the mode of the primitive */
> +   boolean primitive_restart;
> +   ubyte vertices_per_patch; /**< the number of vertices per patch */
> +
> unsigned start;  /**< the index of the first vertex */
> unsigned count;  /**< number of vertices */
>  
> unsigned start_instance; /**< first instance id */
> unsigned instance_count; /**< number of instances */
>  
> unsigned drawid; /**< id of this draw in a multidraw */
>  
> -   unsigned vertices_per_patch; /**< the number of vertices per patch */
> -
> /**
>  * For indexed drawing, these fields apply after index lookup.
>  */
> int index_bias; /**< a bias to be added to each index */
> unsigned min_index; /**< the min index */
> unsigned max_index; /**< the max index */
>  
> /**
>  * Primitive restart enable/index (only applies to indexed drawing)
>  */
> -   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;
> +   unsigned indirect_offset; /**< must be 4 byte aligned */
> +   unsigned indirect_stride; /**< must be 4 byte aligned */
> +   unsigned indirect_count; /**< number of indirect draws */
> +
> +   unsigned indirect_params_offset; /**< must be 4 byte aligned */
> +
> +   /* Pointers must be at the end for an optimal structure layout on 64-bit. 
> */
>  
> /* Indirect draw parameters resource: If not NULL, most values are taken
>  * from this buffer instead, which is laid out as follows:
>  *
>  * if indexed is TRUE:
>  *  struct {
>  * uint32_t count;
>  * uint32_t instance_count;
>  * uint32_t start;
>  * int32_t index_bias;
> @@ -690,30 +681,42 @@ struct pipe_draw_info
>  *  };
>  * otherwise:
>  *  struct {
>  * uint32_t count;
>  * uint32_t instance_count;
>  * uint32_t start;
>  * uint32_t start_instance;
>  *  };
>  */
> struct pipe_resource *indirect;
> -   unsigned indirect_offset; /**< must be 4 byte aligned */
> -   unsigned indirect_stride; /**< must be 4 byte aligned */
> -   unsigned indirect_count; /**< number of indirect draws */
>  
> /* Indirect draw count resource: If not NULL, contains a 32-bit value 
> which
>  * is to be used as the real indirect_count. In that case indirect_count
>  * becomes the maximum possible value.
>  */
> struct pipe_resource *indirect_params;
> -   unsigned indirect_params_offset; /**< must be 4 byte aligned */
> +
> +   /**
> +* 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;
>  };
>  
>  
>  /**
>   * Information to describe a blit call.