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 ?


Axel

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to