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

[Marek Olšák]

On Mon, Feb 20, 2017 at 8:29 PM, Axel Davy <axel.davy at ens.fr> wrote:
> 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 ?

We still need index_size and it's only 8 bits as opposed to 64 bits.

>>>
>>>      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.

offset = start * index_size;

> But we really want to have a byte offset.

The offset should be aligned to index_size, otherwise hardware won't work.

>>>
>>> +   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.

Not true. It's there only because the interface is based on DX10.

Marek