[cogl] buffer: Don't set the invalidate hint when requesting read access

[Neil Roberts <nroberts src gnome org>]

commit 986675d6043e8701f2d65415cf72ffc91734debd
Author: Neil Roberts <neil linux intel com>
Date:   Tue Feb 19 13:51:34 2013 +0000

    buffer: Don't set the invalidate hint when requesting read access
    
    glMapBufferRange is documented to fail with GL_INVALID_OPERATION if
    GL_MAP_INVALIDATE_BUFFER_BIT is set as well as GL_MAP_READ_BIT. I
    guess this makes sense when only read access is requested because
    there would be no point in reading back uninitialised data. However,
    Clutter requests read/write access with the discard hint when
    rendering to a CoglBitmap with Cairo. The data is new so the discard
    hint makes sense but it also needs read access so that it can read
    back the data it just wrote for blending.
    
    This patch works around the GL restriction by skipping the discard
    hints if read access is requested. If the buffer discard hint is set
    along with read access it will recreate the buffer store as an
    alternative way to discard the buffer as it does in the case where the
    GL_ARB_map_buffer_range extension is not supported.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=694164
    
    Reviewed-by: Robert Bragg <robert linux intel com>

 cogl/driver/gl/cogl-buffer-gl.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)
---
diff --git a/cogl/driver/gl/cogl-buffer-gl.c b/cogl/driver/gl/cogl-buffer-gl.c
index be4ac88..e8b576c 100644
--- a/cogl/driver/gl/cogl-buffer-gl.c
+++ b/cogl/driver/gl/cogl-buffer-gl.c
@@ -3,7 +3,7 @@
  *
  * An object oriented GL/GLES Abstraction/Utility Layer
  *
- * Copyright (C) 2010,2011,2012 Intel Corporation.
+ * Copyright (C) 2010,2011,2012,2013 Intel Corporation.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -228,6 +228,10 @@ _cogl_buffer_gl_map_range (CoglBuffer *buffer,
 
   gl_target = convert_bind_target_to_gl_target (target);
 
+  if ((hints & COGL_BUFFER_MAP_HINT_DISCARD_RANGE) &&
+      offset == 0 && size >= buffer->size)
+    hints |= COGL_BUFFER_MAP_HINT_DISCARD;
+
   /* 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
@@ -235,6 +239,7 @@ _cogl_buffer_gl_map_range (CoglBuffer *buffer,
   if (ctx->glMapBufferRange)
     {
       GLbitfield gl_access = 0;
+      CoglBool should_recreate_store = !buffer->store_created;
 
       if ((access & COGL_BUFFER_ACCESS_READ))
         gl_access |= GL_MAP_READ_BIT;
@@ -242,11 +247,25 @@ _cogl_buffer_gl_map_range (CoglBuffer *buffer,
         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))
+        {
+          /* glMapBufferRange generates an error if you pass the
+           * discard hint along with asking for read access. However
+           * it can make sense to ask for both if write access is also
+           * requested so that the application can immediately read
+           * back what it just wrote. To work around the restriction
+           * in GL we just recreate the buffer storage in that case
+           * which is an alternative way to indicate that the buffer
+           * contents can be discarded. */
+          if ((access & COGL_BUFFER_ACCESS_READ))
+            should_recreate_store = TRUE;
+          else
+            gl_access |= GL_MAP_INVALIDATE_BUFFER_BIT;
+        }
+      else if ((hints & COGL_BUFFER_MAP_HINT_DISCARD_RANGE) &&
+               !(access & COGL_BUFFER_ACCESS_READ))
         gl_access |= GL_MAP_INVALIDATE_RANGE_BIT;
 
-      if (!buffer->store_created)
+      if (should_recreate_store)
         {
           if (!recreate_store (buffer, error))
             {
@@ -278,9 +297,7 @@ _cogl_buffer_gl_map_range (CoglBuffer *buffer,
        * 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))
+          (hints & COGL_BUFFER_MAP_HINT_DISCARD))
         {
           if (!recreate_store (buffer, error))
             {