Am 21.02.2017 um 16:49 schrieb Axel Davy: > On 21/02/2017 16:00, Roland Scheidegger wrote: >> 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 >> > What is a basic machine unit ? I_ it supposed to be index_size, or just > the required alignment for the platform ?
Basic machine unit is always one byte for any known implementation. Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev