On Mon, 2014-11-10 at 06:09 -0800, Ian Romanick wrote:
> On 11/06/2014 11:40 PM, Kenneth Graunke wrote:
> > On Thursday, November 06, 2014 08:09:18 PM Ian Romanick wrote:
> >> While working on some other things, I came across some bounds checking
> >> code in _mesa_validate_DrawElements (and related functions) in
> >> api_validate.c.
> >>
> >> /* use indices in the buffer object */
> >> /* make sure count doesn't go outside buffer bounds */
> >> if (index_bytes(type, count) > ctx->Array.VAO->IndexBufferObj->Size)
> >> {
> >> _mesa_warning(ctx, "glDrawElements index out of buffer bounds");
> >> return GL_FALSE;
> >> }
> >>
> >> index_bytes calculates how many bytes of data "count" indices will
> >> occupy based on the type. The problem is that this doesn't consider
> >> the base pointer. As far as I can tell, if I had a 64 byte buffer
> >> object for my index data, and I did
> >>
> >> glDrawElements(GL_POINTS, 16, GL_UNSIGNED_INT, 60);
> >>
> >> _mesa_validate_DrawElements would say, "Ok!"
> >>
> >> Am I missing something, or is this just broken?
> >
> > It sure seems broken to me - but, thankfully, in a conservative fashion.
> > (It
> > will say some invalid things are OK, but won't say legal things are
> > invalid.)
> >
> > Software drivers may be relying on this working to avoid a crash.
> >
> > I checked the Ivybridge documentation, and found:
> >
> > "Software is responsible for ensuring that accesses outside the IB do not
> > occur. This is possible as software can compute the range of IB values
> > referenced by a 3DPRIMITIVE command (knowing the StartVertexLocation,
> > InstanceCount, and VerticesPerInstance values) and can then compare this
> > range to the IB extent."
> >
> > which makes it sound like an accurate computation is necessary. But, right
> > below that, it says:
> >
> > "this field contains the address of the last valid byte in the index buffer.
> > Any index buffer reads past this address returns an index value of 0 (as if
> > the index buffer was zero-extended)."
> >
> > So the earlier statement is false; i965 will draw the in-bounds elements
> > correctly, and then repeat element 0 over and over for any out-of-bounds
> > data,
> > resulting in one strange primitive and a lot of degenerate ones.
> >
> > It's proabbly worth fixing, but I doubt it's critical either.
>
> Hmm... I came across this while looking at cachegrind traces of GL
> applications. Time spent in _mesa_validate_Draw* was non-trivial.
> Since at least some hardware doesn't need this check, I think I want to
> move it down into drivers that actually need the check... which is kind
> of a bummer since I came up with a clever optimization for index_bytes.
I'm not sure what applications you looked at in cachegrind but OpenArena
and Nexuiz both spend a lot of time in here. On my Ivybridge:
OpenArena 5.4% out of a total 27.94% in i965_dri.so
Nexuiz 3.28% out of a total of 29.4% in i965_dri.so
For those not up to speed are you able to give a one line explanation of
why some hardware can get away without this check?
>
> > A more interesting thing to fix, I think, would be enforcing alignment
> > restrictions (i.e. your offset has to be a multiple of the IB element size).
>
> That would probably be useful in debug builds, but I'm pretty sure the
> GL spec says the behavior is undefined specifically to avoid the check
> in the hot path.
>
> > --Ken
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
