[Mesa-dev] glCompressedTex(Sub)Image(2,3) on mapped PBOs
Eduardo Lima Mitev
elima at igalia.com
Tue Feb 24 08:59:52 PST 2015
On 02/20/2015 08:12 PM, Ian Romanick wrote:
> On 02/20/2015 08:13 AM, Eduardo Lima Mitev wrote:
>> In the spec GLES 3.1 spec, page 57, there is this: (and similar wording
>> in GL 4.5 spec, page 76)
>> "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."
> This language is intentionally loose. It is often perfectly safe to
> have a portion of a buffer mapped while a GL command operates on a
> different part of the buffer. Determining whether a particular
> operation is safe can be expensive. Imagine trying to determine whether
> a glDrawElements command will source data from a mapped region of a
> vertex buffer object. Many vendors objected to having to put expensive
> (or cheap) checks in performance critical paths.
> I don't think an of the TexSubImage commands are performance critical
> enough to warrant excluding the test. I believe some benchmarks use
> some of the TexSubImage commands, so it's probably worth measuring them
> on, say, Baytrail with and without the checks. My expectation is that
> there's enough CPU wasted in those paths that the checks will be lost in
> the noise. We won't know until we measure.
>> There is no further restriction in gl(Compressed)Tex(Sub)Image(2,3)D
>> that makes throwing an error mandatory, hence I suspect these tests
>> might be wrong.
> I'm also inclined to think they are wrong... or at least overly strict.
> Do you know of any commands that require this error be generated?
In GLES 3.1, there is explicit mention to this error for
glBufferSubData, glCopyBufferSubData at least. Similar for GL 4.5 but
only if the buffer was not mapped with the MAP_PERSISTENT_BIT set:
"An INVALID_OPERATION error is generated if the source buffer
object is currently mapped, unless it was mapped with
MAP_PERSISTENT_BIT set in the Map*BufferRange access flags."
>> Also, shouldn't this restriction be moved to common API entry points, or
>> are there drivers that do write to PBOs while they are mapped?
> Looking at that function, I find it odd that either of the errors it
> generates are generated there. We should either do these checks in a
> single, common place, or we should not do them at all.
>From yours and Jason's comments, it looks like we should at least move
those checks out of the driver's code.
Regarding the checks for overlapping, it seems doable but I wonder if
that effort pays off. Even if we manage to implement it with negligible
or none overhead, how many applications will actually use that? Right
now these potential applications have to unmap the buffer, so it will
require changing the code, re-building, etc. I guess it will all depend
on the performance gain from operating in several parts of a buffer in
parallel versus doing it synchronously.
So, what I propose:
1) We write a patch to move the checks to API entry points (I have that
partially done already), and remove current checks in the driver. It
would be done for both compressed and non-compressed Tex(Sub)Image APIs.
2) We implement the checks for buffer overlapping as a 2nd step, if we
decide is worth.
For 1) I can provide a patch this week, perhaps coordinating with Jason
since he already gave it a thought. For 2) I could use some help.
More information about the mesa-dev