Am 21.02.2017 um 11:46 schrieb Marek Olšák: > On Tue, Feb 21, 2017 at 4:36 AM, Roland Scheidegger <srol...@vmware.com> > wrote: >> Am 20.02.2017 um 21:58 schrieb Marek Olšák: >>> On Mon, Feb 20, 2017 at 9:28 PM, Roland Scheidegger <srol...@vmware.com> >>> wrote: >>>> Am 20.02.2017 um 20:56 schrieb Marek Olšák: >>>>> On Mon, Feb 20, 2017 at 8:29 PM, Axel Davy <axel.d...@ens.fr> wrote: >>>>>> On 20/02/2017 20:11, Ilia Mirkin wrote: >>>>>>> >>>>>>> On Mon, Feb 20, 2017 at 2:01 PM, Marek Olšák <mar...@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. >>>> FWIW at least in d3d10 you can actually have indexed rendering without >>>> an index buffer bound. This is perfectly valid, you're just expected to >>>> return always zero for all indices... Albeit I believe we actually deal >>>> with this with a dummy buffer. >>>> >>>>> >>>>>>>> >>>>>>>> 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. >>>> Are you sure of that? d3d10 doesn't seem to have such a requirement, or >>>> if it has I can't find it now (so, the startIndex really is in terms of >>>> index units, but the offset of the buffer is in terms of bytes, and the >>>> docs don't seem to mention it's limited to index alignment). >>>> I don't actually see such a limitation in GL neither, albeit some quick >>>> googling seems to suggest YMMV (e.g. >>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__irrlicht.sourceforge.net_forum_viewtopic.php-3Ff-3D7-26t-3D51444&d=DwIFaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=_nDDEb8aspFcYmYCdx9G-Pfs4rRVzx4rodfxnJNkNyc&s=XPduNXrH7SGk7lVUD2izbWAOfERG60bJWTsI600UWCg&e= >>>> ). >>>> So, I can't quite tell right now if we really need byte offsets... >>> >>> It's a natural requirement of hardware. It doesn't have to be >>> documented IMO. CPUs might not support it either. >> I did some quick tests and I believe d3d10 doesn't actually require >> offsets not aligned to index size, but don't quote me on that. >> Nevertheless, I've got the feeling this might be expected to work with >> GL - doesn't look like it would be an error if your indices "pointer" in >> glDrawElements() isn't aligned, and if it's not an error I don't see why >> it wouldn't be well defined? >> (FWIW x86 supports this fine, but indeed not all cpu archs might. Even >> AVX2 gather supports non-aligned lookups - of course just for uint >> indices since gather doesn't support smaller than 32bit gathers.) > > From GL 4.4 Core, section 6.2: > > "Clients must align data elements consistent with the requirements of > the client platform, with an additional base-level requirement that an > offset within a buffer to a datum comprising N basic machine units be > a multiple of N." >
Ah, nice find (not in the section I was looking for...). I suppose it isn't expected to work then (albeit it's interesting that it's still not an actual gl error anywhere). Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev