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." Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev