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>

cogl/cogl/cogl-driver.h

@@ -70,6 +70,12 @@ struct _CoglDriverVtable
                           GLenum *out_glformat,
                           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);

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,

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
+  bytes_per_pixel = cogl_pixel_format_get_bytes_per_pixel (format, 0);
-     implementation specific format under
+  is_read_pixels_format_supported =
-     GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES and
+    ctx->driver_vtable->read_pixels_format_supported (ctx,
-     GL_IMPLEMENTATION_COLOR_READ_TYPE_OES is supported. We could try
+                                                      gl_intformat,
-     to be more clever and check if the requested type matches that
+                                                      gl_format,
-     but we would need some reliable functions to convert from GL
+                                                      gl_type);
-     types to Cogl types. For now, lets just always read in
+  format_mismatch =
-     GL_RGBA/GL_UNSIGNED_BYTE and convert if necessary. We also need
+    (required_format & ~COGL_PREMULT_BIT) !=
-     to use this intermediate buffer if the rowstride has padding
+    (format & ~COGL_PREMULT_BIT);
-     because GLES does not support setting GL_ROW_LENGTH */
+
-  if ((!_cogl_has_private_feature
+  if (!is_read_pixels_format_supported ||
-       (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT) &&
+      format_mismatch ||
-       (gl_format != GL_RGBA || gl_type != GL_UNSIGNED_BYTE ||
+      (!_cogl_has_private_feature (ctx,
-        cogl_bitmap_get_rowstride (bitmap) != 4 * width)) ||
+                                   COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE) &&
-      (required_format & ~COGL_PREMULT_BIT) != (format & ~COGL_PREMULT_BIT))
+       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
+      if (is_read_pixels_format_supported)
-          (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT))
+        {
-        read_format = required_format;
+          read_format = required_format;
+        }
       else
         {
           read_format = COGL_PIXEL_FORMAT_RGBA_8888;

cogl/cogl/driver/gl/gl/cogl-driver-gl.c

@@ -354,6 +354,15 @@ _cogl_driver_pixel_format_to_gl (CoglContext     *context,
   return required_format;
 }
 
+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,
@@ -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,

cogl/cogl/driver/gl/gles/cogl-driver-gles.c

@@ -271,6 +271,18 @@ _cogl_driver_pixel_format_to_gl (CoglContext     *context,
   return required_format;
 }
 
+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,
@@ -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,

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,