[Mesa-dev] [PATCH] vbo: Return INVALID_OPERATION during draw with a mapped buffer

[Kenneth Graunke]

On Wednesday, April 27, 2016 11:22:48 AM PDT Marek Olšák wrote:
> On Wed, Apr 27, 2016 at 5:05 AM, Kenneth Graunke <kenneth at whitecape.org> 
wrote:
> > On Monday, April 25, 2016 5:54:28 PM PDT Jordan Justen wrote:
> >> Fixes the OpenGLES 3.1 CTS:
> >>  * ESEXT-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
> >>
> >> Because this is triggering the error message after the normal API
> >> validation phase, we don't have the API function name available, and
> >> therefore we generate an error message that is a little confusing:
> >>
> >> Mesa: User error: GL_INVALID_OPERATION in Vertex buffers are mapped 
during
> > draw call!
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95142
> >> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> >> Cc: Kenneth Graunke <kenneth at whitecape.org>
> >> ---
> >>
> >>  Perhaps it would be better to push the vbo_bind_arrays before API
> >>  validation, and then make this check be part of API validation...
> >
> > This looks fine to me, though it'd be nice to get a review from someone
> > else who's familiar with this code.  Cc'ing Marek and Fredrik in case
> > they have an opinion.
> 
> The patch says it's a GLES fix, but it modifies desktop GL behavior
> too. Is that intentional?
> 
> Marek

I wasn't able to find any specific spec text requiring us to raise an
error for draw calls when VBOs are mapped, which is rather concerning.
However, given that there's a conformance test, it seems likely that
other vendors implement this behavior.

It's certainly legal to make the change in both APIs:

ES 3.2 says:

"6.3.2 Effects of Mapping Buffers on Other GL Commands

 Any GL command which attempts to read from, write to, or change the
 state of a buffer object may generate an INVALID_OPERATION error if all
 or part of the buffer object is mapped. However, only commands which
 explicitly describe this error are required to do so. If an error is not
 generated, using such commands to perform invalid reads, writes, or
 state changes will have undefined results and may
 result in GL interruption or termination.

GL 4.5 has similar text, but mentions MAP_PERSISTENT_BIT:

"6.3.2 Effects of Mapping Buffers on Other GL Commands

 Any GL command which attempts to read from, write to, or change the
 state of a buffer object may generate an INVALID_OPERATION error if all
 or part of the buffer object is mapped, unless it was allocated by a
 call to *BufferStorage with the MAP_PERSISTENT_BIT included in flags.
 However, only commands which explicitly describe this error are
 required to do so. If an error is not generated, such commands will
 have undefined results and may result in GL interruption or
 termination."

It also seems pretty reasonable to expect them to use a PERSISTENT
mapping if they want to leave it mapped on the CPU while drawing
(reading) from it on the GPU.

So I figure...we may as well be consistent and implement the same
behavior in both APIs.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160427/dbf47ed2/attachment.sig>