Re: [Mesa-dev] glCompressedTex(Sub)Image(2,3) on mapped PBOs
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. cheers, Eduardo ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] glCompressedTex(Sub)Image(2,3) on mapped PBOs
On Fri, Feb 20, 2015 at 8:13 AM, Eduardo Lima Mitev el...@igalia.com wrote: Hello, While working on the following dEQP failing tests I ran into an issue that I think deserves clarification from more experienced people: dEQP-GLES3.functional.negative_api.texture.compressedteximage2d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedteximage3d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target All above tests fail because a CompressedTex(Sub)Image(2,3)D operation is performed while there is a pixel buffer object bound to GL_PIXEL_UNPACK_BUFFER and that pixel buffer object is currently mapped. The tests expect this situation to throw an GL_INVALID_OPERATION but no error is returned on i965. 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. 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. However, I found that glTex(Sub)Image(2,3)D do emits an GL_INVALID_OPERATION error when executed while a mapped PBO is bound. But the check is performed in driver code (as opposed to API entry points). (Concretely, in i965/intel_tex_image.c::intelTexImage() and i965/intel_tex_subimage.c::intelTexSubImage() which both call common/meta_tex_subimage.c::_mesa_meta_pbo_TexSubImage()). So, the question is: Shouldn't these operations on compressed texture images have the same restriction as their non-compressed counter-parts? Or is there a reason why this restriction is enforced only on non-compressed images? 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? IMHO, that's exactly what we need to do. I've been toying with writing a patch to do just that. Right now, we're leaving checking if the buffer is mapped and checking if it has enough size up to the device-specific driver backend. This is, in general, not how the rest of mesa does error-checking so it seems very odd. Also, it means that if a driver misses the check it causes a GL API failure not just a texture upload error. To make it worse, they all check it by calling the same function inside mesa/main so there's really no excuse. I haven't seen any good reason why we aren't just calling that in the error-checking section of the entry-point. --Jason cheers, Eduardo ___ 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
Re: [Mesa-dev] glCompressedTex(Sub)Image(2,3) on mapped PBOs
On 02/20/2015 08:13 AM, Eduardo Lima Mitev wrote: Hello, While working on the following dEQP failing tests I ran into an issue that I think deserves clarification from more experienced people: dEQP-GLES3.functional.negative_api.texture.compressedteximage2d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedteximage3d_invalid_buffer_target dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target All above tests fail because a CompressedTex(Sub)Image(2,3)D operation is performed while there is a pixel buffer object bound to GL_PIXEL_UNPACK_BUFFER and that pixel buffer object is currently mapped. The tests expect this situation to throw an GL_INVALID_OPERATION but no error is returned on i965. 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? However, I found that glTex(Sub)Image(2,3)D do emits an GL_INVALID_OPERATION error when executed while a mapped PBO is bound. But the check is performed in driver code (as opposed to API entry points). (Concretely, in i965/intel_tex_image.c::intelTexImage() and i965/intel_tex_subimage.c::intelTexSubImage() which both call common/meta_tex_subimage.c::_mesa_meta_pbo_TexSubImage()). So, the question is: Shouldn't these operations on compressed texture images have the same restriction as their non-compressed counter-parts? Or is there a reason why this restriction is enforced only on non-compressed images? 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. cheers, Eduardo ___ 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
