Re: [Mesa-dev] [PATCH 1/3] mesa: Fix handling of glCopyBufferSubData() for src == dst.
On Thu, 26 Jan 2012 13:58:43 -0800, Ian Romanick i...@freedesktop.org wrote:
On 01/25/2012 05:51 PM, Eric Anholt wrote:
Fixes piglit ARB_copy_buffer-overlap, which previously assertion failed.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
One other orthogonal question below...
---
src/mesa/main/bufferobj.c | 25 +++--
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 5b6db78..e4f964f 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -526,11 +526,23 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
assert(!_mesa_bufferobj_mapped(src));
assert(!_mesa_bufferobj_mapped(dst));
- srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
- GL_MAP_READ_BIT, src);
- dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
- (GL_MAP_WRITE_BIT |
-GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ if (src == dst) {
+ srcPtr = dstPtr = ctx-Driver.MapBufferRange(ctx, 0, src-Size,
+ GL_MAP_READ_BIT |
+ GL_MAP_WRITE_BIT, src);
+
+ if (!srcPtr)
+return;
+
+ srcPtr += readOffset;
+ dstPtr += writeOffset;
+ } else {
+ srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
+ GL_MAP_READ_BIT, src);
+ dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
+ (GL_MAP_WRITE_BIT |
+ GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ }
/* Note: the src and dst regions will never overlap. Trying to do so
* would generate GL_INVALID_VALUE earlier.
So we have a test for the assertion in this comment? That's the only
case that concerns me in this code.
Piglit ARB_copy_buffer/overlap is quite thorough: for a buffer of size 6
(arbitrary small number), per usage, try every
overlapping/non-overlapping copy, and make sure the right thing happens.
pgpyu1bgFVsky.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Fix handling of glCopyBufferSubData() for src == dst.
On Wed, Jan 25, 2012 at 8:51 PM, Eric Anholt e...@anholt.net wrote:
Fixes piglit ARB_copy_buffer-overlap, which previously assertion failed.
---
src/mesa/main/bufferobj.c | 25 +++--
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 5b6db78..e4f964f 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -526,11 +526,23 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
assert(!_mesa_bufferobj_mapped(src));
assert(!_mesa_bufferobj_mapped(dst));
- srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
- GL_MAP_READ_BIT, src);
- dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
- (GL_MAP_WRITE_BIT |
- GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ if (src == dst) {
+ srcPtr = dstPtr = ctx-Driver.MapBufferRange(ctx, 0, src-Size,
+ GL_MAP_READ_BIT |
+ GL_MAP_WRITE_BIT, src);
+
+ if (!srcPtr)
+ return;
+
+ srcPtr += readOffset;
+ dstPtr += writeOffset;
+ } else {
+ srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
+ GL_MAP_READ_BIT, src);
+ dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
+ (GL_MAP_WRITE_BIT |
+ GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ }
/* Note: the src and dst regions will never overlap. Trying to do so
* would generate GL_INVALID_VALUE earlier.
@@ -539,7 +551,8 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
memcpy(dstPtr, srcPtr, size);
ctx-Driver.UnmapBuffer(ctx, src);
- ctx-Driver.UnmapBuffer(ctx, dst);
+ if (dst != src)
+ ctx-Driver.UnmapBuffer(ctx, dst);
}
Looks good to me. Though, when srcbuf==dstbuf it wouldn't be too hard
to compute the union region to avoid mapping the whole buffer.
There's some places in the swrast code where we could do that too for
renderbuffers.
Reviewed-by: Brian Paul bri...@vmware.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Fix handling of glCopyBufferSubData() for src == dst.
On 01/25/2012 05:51 PM, Eric Anholt wrote:
Fixes piglit ARB_copy_buffer-overlap, which previously assertion failed.
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
One other orthogonal question below...
---
src/mesa/main/bufferobj.c | 25 +++--
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
index 5b6db78..e4f964f 100644
--- a/src/mesa/main/bufferobj.c
+++ b/src/mesa/main/bufferobj.c
@@ -526,11 +526,23 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
assert(!_mesa_bufferobj_mapped(src));
assert(!_mesa_bufferobj_mapped(dst));
- srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
- GL_MAP_READ_BIT, src);
- dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
- (GL_MAP_WRITE_BIT |
-GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ if (src == dst) {
+ srcPtr = dstPtr = ctx-Driver.MapBufferRange(ctx, 0, src-Size,
+ GL_MAP_READ_BIT |
+ GL_MAP_WRITE_BIT, src);
+
+ if (!srcPtr)
+return;
+
+ srcPtr += readOffset;
+ dstPtr += writeOffset;
+ } else {
+ srcPtr = ctx-Driver.MapBufferRange(ctx, readOffset, size,
+ GL_MAP_READ_BIT, src);
+ dstPtr = ctx-Driver.MapBufferRange(ctx, writeOffset, size,
+ (GL_MAP_WRITE_BIT |
+ GL_MAP_INVALIDATE_RANGE_BIT), dst);
+ }
/* Note: the src and dst regions will never overlap. Trying to do so
* would generate GL_INVALID_VALUE earlier.
So we have a test for the assertion in this comment? That's the only
case that concerns me in this code.
@@ -539,7 +551,8 @@ _mesa_copy_buffer_subdata(struct gl_context *ctx,
memcpy(dstPtr, srcPtr, size);
ctx-Driver.UnmapBuffer(ctx, src);
- ctx-Driver.UnmapBuffer(ctx, dst);
+ if (dst != src)
+ ctx-Driver.UnmapBuffer(ctx, dst);
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
