[Cogl] [PATCH 1/3] Add cogl_buffer_map_range()
Robert Bragg
robert at sixbynine.org
Thu Oct 18 06:26:54 PDT 2012
This series of patches looks good to land to me:
Reviewed-by: Robert Bragg <robert at linux.intel.com>
thanks,
- Robert
On Wed, Oct 17, 2012 at 9:47 PM, Neil Roberts <neil at linux.intel.com> wrote:
> This adds a buffer method to map a subregion of the buffer. This works
> using the GL_ARB_map_buffer_range extension. If the extension is not
> available then it will fallback to using glMapBuffer to map the entire
> buffer and then just add the offset to the returned pointer.
>
> cogl_buffer_map() is now just a wrapper which maps the entire range of
> the buffer. The driver backend functions have been renamed to
> map_range and they now all take the offset and size arguments.
>
> When the COGL_BUFFER_MAP_HINT_DISCARD hint is used and the map range
> extension is available instead of using glBufferData to invalidate the
> buffer it will instead pass the new GL_MAP_HINT_INVALIDATE_BUFFER
> flag. There is now additionally a COGL_BUFFER_MAP_HINT_DISCARD_REGION
> hint which can be used if the application only wants to discard the
> small region that is mapped. glMapBufferRange is always used if it is
> available even if the entire buffer is being mapped because it seems
> more robust to pass those flags then to call glBufferData.
> ---
> cogl/cogl-buffer-private.h | 8 ++--
> cogl/cogl-buffer.c | 33 ++++++++++++---
> cogl/cogl-buffer.h | 52 +++++++++++++++++++++--
> cogl/cogl-driver.h | 8 ++--
> cogl/driver/gl/cogl-buffer-gl-private.h | 8 ++--
> cogl/driver/gl/cogl-buffer-gl.c | 75 ++++++++++++++++++++++++++++-----
> cogl/driver/gl/gl/cogl-driver-gl.c | 2 +-
> cogl/driver/gl/gles/cogl-driver-gles.c | 2 +-
> cogl/gl-prototypes/cogl-all-functions.h | 11 +++++
> doc/reference/cogl2/cogl2-sections.txt | 1 +
> 10 files changed, 168 insertions(+), 32 deletions(-)
>
> diff --git a/cogl/cogl-buffer-private.h b/cogl/cogl-buffer-private.h
> index 59794cd..cf2d9b9 100644
> --- a/cogl/cogl-buffer-private.h
> +++ b/cogl/cogl-buffer-private.h
> @@ -41,9 +41,11 @@ typedef struct _CoglBufferVtable CoglBufferVtable;
>
> struct _CoglBufferVtable
> {
> - void * (* map) (CoglBuffer *buffer,
> - CoglBufferAccess access,
> - CoglBufferMapHint hints);
> + void * (* map_range) (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints);
>
> void (* unmap) (CoglBuffer *buffer);
>
> diff --git a/cogl/cogl-buffer.c b/cogl/cogl-buffer.c
> index 573db0f..7a1a084 100644
> --- a/cogl/cogl-buffer.c
> +++ b/cogl/cogl-buffer.c
> @@ -79,12 +79,14 @@ cogl_is_buffer (void *object)
> */
>
> static void *
> -malloc_map (CoglBuffer *buffer,
> - CoglBufferAccess access,
> - CoglBufferMapHint hints)
> +malloc_map_range (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints)
> {
> buffer->flags |= COGL_BUFFER_FLAG_MAPPED;
> - return buffer->data;
> + return buffer->data + offset;
> }
>
> static void
> @@ -138,7 +140,7 @@ _cogl_buffer_initialize (CoglBuffer *buffer,
>
> if (use_malloc)
> {
> - buffer->vtable.map = malloc_map;
> + buffer->vtable.map_range = malloc_map_range;
> buffer->vtable.unmap = malloc_unmap;
> buffer->vtable.set_data = malloc_set_data;
>
> @@ -146,7 +148,7 @@ _cogl_buffer_initialize (CoglBuffer *buffer,
> }
> else
> {
> - buffer->vtable.map = ctx->driver_vtable->buffer_map;
> + buffer->vtable.map_range = ctx->driver_vtable->buffer_map_range;
> buffer->vtable.unmap = ctx->driver_vtable->buffer_unmap;
> buffer->vtable.set_data = ctx->driver_vtable->buffer_set_data;
>
> @@ -218,13 +220,30 @@ cogl_buffer_map (CoglBuffer *buffer,
> {
> _COGL_RETURN_VAL_IF_FAIL (cogl_is_buffer (buffer), NULL);
>
> + return cogl_buffer_map_range (buffer, 0, buffer->size, access, hints);
> +}
> +
> +void *
> +cogl_buffer_map_range (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints)
> +{
> + _COGL_RETURN_VAL_IF_FAIL (cogl_is_buffer (buffer), NULL);
> +
> if (G_UNLIKELY (buffer->immutable_ref))
> warn_about_midscene_changes ();
>
> if (buffer->flags & COGL_BUFFER_FLAG_MAPPED)
> return buffer->data;
>
> - buffer->data = buffer->vtable.map (buffer, access, hints);
> + buffer->data = buffer->vtable.map_range (buffer,
> + offset,
> + size,
> + access,
> + hints);
> +
> return buffer->data;
> }
>
> diff --git a/cogl/cogl-buffer.h b/cogl/cogl-buffer.h
> index 5ad24b9..217de11 100644
> --- a/cogl/cogl-buffer.h
> +++ b/cogl/cogl-buffer.h
> @@ -161,7 +161,13 @@ typedef enum { /*< prefix=COGL_BUFFER_ACCESS >*/
> /**
> * CoglBufferMapHint:
> * @COGL_BUFFER_MAP_HINT_DISCARD: Tells Cogl that you plan to replace
> - * all the buffer's contents.
> + * all the buffer's contents. When this flag is used to map a
> + * buffer, the entire contents of the buffer become undefined, even
> + * if only a subregion of the buffer is mapped.
> + * @COGL_BUFFER_MAP_HINT_DISCARD_RANGE: Tells Cogl that you plan to
> + * replace all the contents of the mapped region. The contents of
> + * the region specified are undefined after this flag is used to
> + * map a buffer.
> *
> * Hints to Cogl about how you are planning to modify the data once it
> * is mapped.
> @@ -170,7 +176,8 @@ typedef enum { /*< prefix=COGL_BUFFER_ACCESS >*/
> * Stability: unstable
> */
> typedef enum { /*< prefix=COGL_BUFFER_MAP_HINT >*/
> - COGL_BUFFER_MAP_HINT_DISCARD = 1 << 0
> + COGL_BUFFER_MAP_HINT_DISCARD = 1 << 0,
> + COGL_BUFFER_MAP_HINT_DISCARD_RANGE = 1 << 1
> } CoglBufferMapHint;
>
> /**
> @@ -180,7 +187,9 @@ typedef enum { /*< prefix=COGL_BUFFER_MAP_HINT >*/
> * @hints: A mask of #CoglBufferMapHint<!-- -->s that tell Cogl how
> * the data will be modified once mapped.
> *
> - * Maps the buffer into the application address space for direct access.
> + * Maps the buffer into the application address space for direct
> + * access. This is equivalent to calling cogl_buffer_map_range() with
> + * zero as the offset and the size of the entire buffer as the size.
> *
> * It is strongly recommended that you pass
> * %COGL_BUFFER_MAP_HINT_DISCARD as a hint if you are going to replace
> @@ -204,6 +213,43 @@ cogl_buffer_map (CoglBuffer *buffer,
> CoglBufferMapHint hints);
>
> /**
> + * cogl_buffer_map_range:
> + * @buffer: a buffer object
> + * @offset: Offset within the buffer to start the mapping
> + * @size: The size of data to map
> + * @access: how the mapped buffer will be used by the application
> + * @hints: A mask of #CoglBufferMapHint<!-- -->s that tell Cogl how
> + * the data will be modified once mapped.
> + *
> + * Maps a sub-region of the buffer into the application's address space
> + * for direct access.
> + *
> + * It is strongly recommended that you pass
> + * %COGL_BUFFER_MAP_HINT_DISCARD as a hint if you are going to replace
> + * all the buffer's data. This way if the buffer is currently being
> + * used by the GPU then the driver won't have to stall the CPU and
> + * wait for the hardware to finish because it can instead allocate a
> + * new buffer to map. You can pass
> + * %COGL_BUFFER_MAP_HINT_DISCARD_REGION instead if you want the
> + * regions outside of the mapping to be retained.
> + *
> + * The behaviour is undefined if you access the buffer in a way
> + * conflicting with the @access mask you pass. It is also an error to
> + * release your last reference while the buffer is mapped.
> + *
> + * Return value: A pointer to the mapped memory or %NULL is the call fails
> + *
> + * Since: 2.0
> + * Stability: unstable
> + */
> +void *
> +cogl_buffer_map_range (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints);
> +
> +/**
> * cogl_buffer_unmap:
> * @buffer: a buffer object
> *
> diff --git a/cogl/cogl-driver.h b/cogl/cogl-driver.h
> index f922111..36f4c7a 100644
> --- a/cogl/cogl-driver.h
> +++ b/cogl/cogl-driver.h
> @@ -240,9 +240,11 @@ struct _CoglDriverVtable
>
> /* Maps a buffer into the CPU */
> void *
> - (* buffer_map) (CoglBuffer *buffer,
> - CoglBufferAccess access,
> - CoglBufferMapHint hints);
> + (* buffer_map_range) (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints);
>
> /* Unmaps a buffer */
> void
> diff --git a/cogl/driver/gl/cogl-buffer-gl-private.h b/cogl/driver/gl/cogl-buffer-gl-private.h
> index 570e36c..583e0c1 100644
> --- a/cogl/driver/gl/cogl-buffer-gl-private.h
> +++ b/cogl/driver/gl/cogl-buffer-gl-private.h
> @@ -40,9 +40,11 @@ void
> _cogl_buffer_gl_destroy (CoglBuffer *buffer);
>
> void *
> -_cogl_buffer_gl_map (CoglBuffer *buffer,
> - CoglBufferAccess access,
> - CoglBufferMapHint hints);
> +_cogl_buffer_gl_map_range (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints);
>
> void
> _cogl_buffer_gl_unmap (CoglBuffer *buffer);
> diff --git a/cogl/driver/gl/cogl-buffer-gl.c b/cogl/driver/gl/cogl-buffer-gl.c
> index 94f6936..673dbf1 100644
> --- a/cogl/driver/gl/cogl-buffer-gl.c
> +++ b/cogl/driver/gl/cogl-buffer-gl.c
> @@ -57,7 +57,18 @@
> #ifndef GL_READ_WRITE
> #define GL_READ_WRITE 0x88BA
> #endif
> -
> +#ifndef GL_MAP_READ_BIT
> +#define GL_MAP_READ_BIT 0x0001
> +#endif
> +#ifndef GL_MAP_WRITE_BIT
> +#define GL_MAP_WRITE_BIT 0x0002
> +#endif
> +#ifndef GL_MAP_INVALIDATE_RANGE_BIT
> +#define GL_MAP_INVALIDATE_RANGE_BIT 0x0004
> +#endif
> +#ifndef GL_MAP_INVALIDATE_BUFFER_BIT
> +#define GL_MAP_INVALIDATE_BUFFER_BIT 0x0008
> +#endif
>
> void
> _cogl_buffer_gl_create (CoglBuffer *buffer)
> @@ -173,9 +184,11 @@ _cogl_buffer_bind_no_create (CoglBuffer *buffer,
> }
>
> void *
> -_cogl_buffer_gl_map (CoglBuffer *buffer,
> - CoglBufferAccess access,
> - CoglBufferMapHint hints)
> +_cogl_buffer_gl_map_range (CoglBuffer *buffer,
> + size_t offset,
> + size_t size,
> + CoglBufferAccess access,
> + CoglBufferMapHint hints)
> {
> uint8_t *data;
> CoglBufferBindTarget target;
> @@ -194,14 +207,54 @@ _cogl_buffer_gl_map (CoglBuffer *buffer,
>
> gl_target = convert_bind_target_to_gl_target (target);
>
> - /* create an empty store if we don't have one yet. creating the store
> - * lazily allows the user of the CoglBuffer to set a hint before the
> - * store is created. */
> - if (!buffer->store_created || (hints & COGL_BUFFER_MAP_HINT_DISCARD))
> - recreate_store (buffer);
> + /* If the map buffer range extension is supported then we will
> + * always use it even if we are mapping the full range because the
> + * normal mapping function doesn't support passing the discard
> + * hints */
> + if (ctx->glMapBufferRange)
> + {
> + GLbitfield gl_access = 0;
> +
> + if ((access & COGL_BUFFER_ACCESS_READ))
> + gl_access |= GL_MAP_READ_BIT;
> + if ((access & COGL_BUFFER_ACCESS_WRITE))
> + gl_access |= GL_MAP_WRITE_BIT;
> +
> + if ((hints & COGL_BUFFER_MAP_HINT_DISCARD))
> + gl_access |= GL_MAP_INVALIDATE_BUFFER_BIT;
> + if ((hints & COGL_BUFFER_MAP_HINT_DISCARD_RANGE))
> + gl_access |= GL_MAP_INVALIDATE_RANGE_BIT;
> +
> + if (!buffer->store_created)
> + recreate_store (buffer);
> +
> + GE_RET( data,
> + ctx,
> + glMapBufferRange (gl_target,
> + offset,
> + size,
> + gl_access) );
> + }
> + else
> + {
> + /* create an empty store if we don't have one yet. creating the store
> + * lazily allows the user of the CoglBuffer to set a hint before the
> + * store is created. */
> + if (!buffer->store_created ||
> + (hints & COGL_BUFFER_MAP_HINT_DISCARD) ||
> + ((hints & COGL_BUFFER_MAP_HINT_DISCARD_RANGE) &&
> + offset == 0 && size >= buffer->size))
> + recreate_store (buffer);
> +
> + GE_RET( data,
> + ctx,
> + glMapBuffer (gl_target,
> + _cogl_buffer_access_to_gl_enum (access)) );
> +
> + if (data)
> + data += offset;
> + }
>
> - GE_RET( data, ctx, glMapBuffer (gl_target,
> - _cogl_buffer_access_to_gl_enum (access)) );
> if (data)
> buffer->flags |= COGL_BUFFER_FLAG_MAPPED;
>
> diff --git a/cogl/driver/gl/gl/cogl-driver-gl.c b/cogl/driver/gl/gl/cogl-driver-gl.c
> index 4b961b7..142260f 100644
> --- a/cogl/driver/gl/gl/cogl-driver-gl.c
> +++ b/cogl/driver/gl/gl/cogl-driver-gl.c
> @@ -585,7 +585,7 @@ _cogl_driver_gl =
> _cogl_clip_stack_gl_flush,
> _cogl_buffer_gl_create,
> _cogl_buffer_gl_destroy,
> - _cogl_buffer_gl_map,
> + _cogl_buffer_gl_map_range,
> _cogl_buffer_gl_unmap,
> _cogl_buffer_gl_set_data,
> };
> diff --git a/cogl/driver/gl/gles/cogl-driver-gles.c b/cogl/driver/gl/gles/cogl-driver-gles.c
> index c38f090..0e2b465 100644
> --- a/cogl/driver/gl/gles/cogl-driver-gles.c
> +++ b/cogl/driver/gl/gles/cogl-driver-gles.c
> @@ -364,7 +364,7 @@ _cogl_driver_gles =
> _cogl_clip_stack_gl_flush,
> _cogl_buffer_gl_create,
> _cogl_buffer_gl_destroy,
> - _cogl_buffer_gl_map,
> + _cogl_buffer_gl_map_range,
> _cogl_buffer_gl_unmap,
> _cogl_buffer_gl_set_data,
> };
> diff --git a/cogl/gl-prototypes/cogl-all-functions.h b/cogl/gl-prototypes/cogl-all-functions.h
> index eeef4e7..870722e 100644
> --- a/cogl/gl-prototypes/cogl-all-functions.h
> +++ b/cogl/gl-prototypes/cogl-all-functions.h
> @@ -286,3 +286,14 @@ COGL_EXT_FUNCTION (void, glGenVertexArrays,
> (GLsizei n,
> GLuint *arrays))
> COGL_EXT_END ()
> +
> +COGL_EXT_BEGIN (map_region, 3, 0,
> + 0, /* not in either GLES */
> + "ARB\0",
> + "map_buffer_range\0")
> +COGL_EXT_FUNCTION (GLvoid *, glMapBufferRange,
> + (GLenum target,
> + GLintptr offset,
> + GLsizeiptr length,
> + GLbitfield access))
> +COGL_EXT_END ()
> diff --git a/doc/reference/cogl2/cogl2-sections.txt b/doc/reference/cogl2/cogl2-sections.txt
> index e5cb2fd..c02b87b 100644
> --- a/doc/reference/cogl2/cogl2-sections.txt
> +++ b/doc/reference/cogl2/cogl2-sections.txt
> @@ -669,6 +669,7 @@ cogl_buffer_set_update_hint
> cogl_buffer_get_update_hint
> CoglBufferAccess
> cogl_buffer_map
> +cogl_buffer_map_range
> cogl_buffer_unmap
> cogl_buffer_set_data
>
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list