cogl/framebuffer/gl: Move most read restriction to drivers

OpenGL requires more hand holding in the driver regarding what pixel
memory layouts can be written when calling glReadPixels(), compared to
GLES2. Lets move the details of this logic to the corresponding
backends, so in the future, the GLES2 backend can be adapted to handle
more formats, without placing that logic in the generic layer.

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2461>

6 files changed, 54 insertions, 20 deletions

diff --git a/cogl/cogl/cogl-driver.h b/cogl/cogl/cogl-driver.h
index a87e57285..4b639df87 100644
--- a/cogl/cogl/cogl-driver.h
+++ b/cogl/cogl/cogl-driver.h
@@ -71,6 +71,12 @@ struct _CoglDriverVtable
                           GLenum *out_gltype);
 
   gboolean
+  (* read_pixels_format_supported) (CoglContext *ctx,
+                                    GLenum gl_intformat,
+                                    GLenum gl_format,
+                                    GLenum gl_type);
+
+  gboolean
   (* update_features) (CoglContext *context,
                        GError **error);
 
diff --git a/cogl/cogl/cogl-private.h b/cogl/cogl/cogl-private.h
index f842047f0..16d01a651 100644
--- a/cogl/cogl/cogl-private.h
+++ b/cogl/cogl/cogl-private.h
@@ -50,7 +50,7 @@ typedef enum
   COGL_PRIVATE_FEATURE_TEXTURE_FORMAT_HALF_FLOAT,
   COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE,
   COGL_PRIVATE_FEATURE_SAMPLER_OBJECTS,
-  COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT,
+  COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE,
   COGL_PRIVATE_FEATURE_FORMAT_CONVERSION,
   COGL_PRIVATE_FEATURE_QUERY_FRAMEBUFFER_BITS,
   COGL_PRIVATE_FEATURE_QUERY_TEXTURE_PARAMETERS,
diff --git a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c
index dfd9b9913..f809d9382 100644
--- a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c
+++ b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c
@@ -429,6 +429,9 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver  *driver,
   GLenum gl_format;
   GLenum gl_type;
   GLenum gl_pack_enum = GL_FALSE;
+  int bytes_per_pixel;
+  gboolean is_read_pixels_format_supported;
+  gboolean format_mismatch;
   gboolean pack_invert_set;
   int status = FALSE;
 
@@ -466,21 +469,21 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver  *driver,
   else
     pack_invert_set = FALSE;
 
-  /* Under GLES only GL_RGBA with GL_UNSIGNED_BYTE as well as an
-     implementation specific format under
-     GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES and
-     GL_IMPLEMENTATION_COLOR_READ_TYPE_OES is supported. We could try
-     to be more clever and check if the requested type matches that
-     but we would need some reliable functions to convert from GL
-     types to Cogl types. For now, lets just always read in
-     GL_RGBA/GL_UNSIGNED_BYTE and convert if necessary. We also need
-     to use this intermediate buffer if the rowstride has padding
-     because GLES does not support setting GL_ROW_LENGTH */
-  if ((!_cogl_has_private_feature
-       (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT) &&
-       (gl_format != GL_RGBA || gl_type != GL_UNSIGNED_BYTE ||
-        cogl_bitmap_get_rowstride (bitmap) != 4 * width)) ||
-      (required_format & ~COGL_PREMULT_BIT) != (format & ~COGL_PREMULT_BIT))
+  bytes_per_pixel = cogl_pixel_format_get_bytes_per_pixel (format, 0);
+  is_read_pixels_format_supported =
+    ctx->driver_vtable->read_pixels_format_supported (ctx,
+                                                      gl_intformat,
+                                                      gl_format,
+                                                      gl_type);
+  format_mismatch =
+    (required_format & ~COGL_PREMULT_BIT) !=
+    (format & ~COGL_PREMULT_BIT);
+
+  if (!is_read_pixels_format_supported ||
+      format_mismatch ||
+      (!_cogl_has_private_feature (ctx,
+                                   COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE) &&
+       cogl_bitmap_get_rowstride (bitmap) != bytes_per_pixel * width))
     {
       CoglBitmap *tmp_bmp;
       CoglPixelFormat read_format;
@@ -488,9 +491,10 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver  *driver,
       uint8_t *tmp_data;
       gboolean succeeded;
 
-      if (_cogl_has_private_feature
-          (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT))
-        read_format = required_format;
+      if (is_read_pixels_format_supported)
+        {
+          read_format = required_format;
+        }
       else
         {
           read_format = COGL_PIXEL_FORMAT_RGBA_8888;
diff --git a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c
index 7c9449f5b..a05fe61b6 100644
--- a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c
+++ b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c
@@ -355,6 +355,15 @@ _cogl_driver_pixel_format_to_gl (CoglContext     *context,
 }
 
 static gboolean
+_cogl_driver_read_pixels_format_supported (CoglContext *context,
+                                           GLenum       glintformat,
+                                           GLenum       glformat,
+                                           GLenum       gltype)
+{
+  return TRUE;
+}
+
+static gboolean
 _cogl_get_gl_version (CoglContext *ctx,
                       int *major_out,
                       int *minor_out)
@@ -517,7 +526,7 @@ _cogl_driver_update_features (CoglContext *ctx,
     }
 
   COGL_FLAGS_SET (private_features,
-                  COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT, TRUE);
+                  COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE, TRUE);
   COGL_FLAGS_SET (private_features, COGL_PRIVATE_FEATURE_ANY_GL, TRUE);
   COGL_FLAGS_SET (private_features,
                   COGL_PRIVATE_FEATURE_FORMAT_CONVERSION, TRUE);
@@ -582,6 +591,7 @@ _cogl_driver_gl =
     _cogl_gl_get_graphics_reset_status,
     _cogl_driver_pixel_format_from_gl_internal,
     _cogl_driver_pixel_format_to_gl,
+    _cogl_driver_read_pixels_format_supported,
     _cogl_driver_update_features,
     _cogl_driver_gl_create_framebuffer_driver,
     _cogl_driver_gl_flush_framebuffer_state,
diff --git a/cogl/cogl/driver/gl/gles/cogl-driver-gles.c b/cogl/cogl/driver/gl/gles/cogl-driver-gles.c
index 59cf0c07e..41387a3eb 100644
--- a/cogl/cogl/driver/gl/gles/cogl-driver-gles.c
+++ b/cogl/cogl/driver/gl/gles/cogl-driver-gles.c
@@ -272,6 +272,18 @@ _cogl_driver_pixel_format_to_gl (CoglContext     *context,
 }
 
 static gboolean
+_cogl_driver_read_pixels_format_supported (CoglContext *context,
+                                           GLenum       glintformat,
+                                           GLenum       glformat,
+                                           GLenum       gltype)
+{
+  if (glformat == GL_RGBA && gltype == GL_UNSIGNED_BYTE)
+    return TRUE;
+
+  return FALSE;
+}
+
+static gboolean
 _cogl_get_gl_version (CoglContext *ctx,
                       int *major_out,
                       int *minor_out)
@@ -477,6 +489,7 @@ _cogl_driver_gles =
     _cogl_gl_get_graphics_reset_status,
     _cogl_driver_pixel_format_from_gl_internal,
     _cogl_driver_pixel_format_to_gl,
+    _cogl_driver_read_pixels_format_supported,
     _cogl_driver_update_features,
     _cogl_driver_gl_create_framebuffer_driver,
     _cogl_driver_gl_flush_framebuffer_state,
diff --git a/cogl/cogl/driver/nop/cogl-driver-nop.c b/cogl/cogl/driver/nop/cogl-driver-nop.c
index 2cde8576b..f18e78873 100644
--- a/cogl/cogl/driver/nop/cogl-driver-nop.c
+++ b/cogl/cogl/driver/nop/cogl-driver-nop.c
@@ -95,6 +95,7 @@ _cogl_driver_nop =
     NULL, /* get_graphics_reset_status */
     NULL, /* pixel_format_from_gl_internal */
     NULL, /* pixel_format_to_gl */
+    NULL, /* read_pixels_format_supported */
     _cogl_driver_update_features,
     _cogl_driver_nop_create_framebuffer_driver,
     _cogl_driver_nop_flush_framebuffer_state,