[Mesa-dev] Gallium: Removal of set_index_buffer (discussion)

[Axel Davy]

On 20/02/2017 20:11, Ilia Mirkin wrote:
> On Mon, Feb 20, 2017 at 2:01 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> Hi,
>>
>> I'd like to remove pipe_context::set_index_buffer. It's not useful to
>> most drivers and the interface is inconvenient for Mesa/OpenGL,
>> because it's a draw state that is set with a separate driver callback,
>> which is an unnecessary driver roundtrip taking some CPU cycles. I'd
>> prefer to pass the index buffer via pipe_draw_info.
>>
>> I'm aware that the interface was inherited from DX10, but I don't
>> think that makes any difference here. DX10 state trackers can pass the
>> index buffer via pipe_draw_info too.
>>
>> This is my proposal:
>>
>> iff --git a/src/gallium/include/pipe/p_state.h
>> b/src/gallium/include/pipe/p_state.h
>> index ce19b92..cffbb33 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -635,7 +635,7 @@ struct pipe_index_buffer
>>    */
>>   struct pipe_draw_info
>>   {
>> -   boolean indexed;  /**< use index buffer */
>> +   ubyte index_size;  /**< 0 = non-indexed */
Isn't that enough to say non-index when index_buffer and user_indices 
are NULL ?
>>      enum pipe_prim_type mode;  /**< the mode of the primitive */
>>      boolean primitive_restart;
>>      ubyte vertices_per_patch; /**< the number of vertices per patch */
>> @@ -666,12 +666,18 @@ struct pipe_draw_info
>>
>>      unsigned indirect_params_offset; /**< must be 4 byte aligned */
>>
>> +   /**
>> +    * Index buffer. Only one can be non-NULL.
>> +    */
>> +   struct pipe_resource *index_buffer; /* "start" is the offset */
> Works for me. Is start the offset in bytes or is start * index_size
> the offset in bytes?
Same question here. My understanding is that start is in terms of start 
* index_size bytes.
But we really want to have a byte offset.
>> +   void *user_indices;
>> +
>>      /* Pointers must be at the end for an optimal structure layout on 64-bit. */
>>
>>
>> Comments welcome,
>>
>> Marek
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

My understanding of the current interface is that setting the index 
buffer may be costly. Thus you want to set once, and use for several 
draw calls with different start or other changes.

If setting the index buffer is indeed costly, and independant of the 
draw calls, the proposed change makes it a bit harder for the driver 
(it's as if we are setting the index buffer ever draw call, even if it 
didn't really change). For the state tracker, it doesn't make much 
difference.


Axel


Axel