Fix up downloader selection

The mmap downloader is now added last, so we checking the downlaoders
list in order no longer prefers the mmap downloader. To fix that, just
select it explicitly if the format is linear.

gdk/gdkdisplay.c

@@ -1891,14 +1891,15 @@ gdk_display_init_dmabuf (GdkDisplay *self)
 #ifdef HAVE_DMABUF
   if (!GDK_DEBUG_CHECK (DMABUF_DISABLE))
     {
-      gdk_display_prepare_gl (self, NULL);
-
-      gdk_display_add_dmabuf_downloader (self, gdk_dmabuf_get_direct_downloader (), builder);
-
 #ifdef HAVE_EGL
       if (gdk_display_prepare_gl (self, NULL))
+        {
           gdk_display_add_dmabuf_downloader (self, gdk_dmabuf_get_egl_downloader (), builder);
+          gdk_dmabuf_formats_builder_next_priority (builder);
+        }
 #endif
+
+      gdk_display_add_dmabuf_downloader (self, gdk_dmabuf_get_direct_downloader (), builder);
     }
 #endif
 
@@ -1907,6 +1908,29 @@ gdk_display_init_dmabuf (GdkDisplay *self)
   GDK_DISPLAY_DEBUG (self, DMABUF,
                      "Initialized support for %zu dmabuf formats",
                      gdk_dmabuf_formats_get_n_formats (self->dmabuf_formats));
+
+#ifdef G_ENABLE_DEBUG
+  if (GDK_DISPLAY_DEBUG_CHECK (self, DMABUF))
+    {
+      for (gsize i = 0; i < gdk_dmabuf_formats_get_n_formats (self->dmabuf_formats); i++)
+        {
+          guint32 fourcc;
+          guint64 modifier;
+
+          gdk_dmabuf_formats_get_format (self->dmabuf_formats, i, &fourcc, &modifier);
+
+          gdk_debug_message ("dmabuf format %.4s:%#" G_GINT64_MODIFIER "x%s",
+                             (char *) &fourcc,
+                             modifier,
+                             gdk_dmabuf_formats_contains (self->egl_external_formats, fourcc, modifier) ? " (external)" : "");
+
+          if (i + 1 < gdk_dmabuf_formats_get_n_formats (self->dmabuf_formats) &&
+              gdk_dmabuf_formats_next_priority (self->dmabuf_formats, i) == i + 1)
+            gdk_debug_message ("------");
+
+        }
+    }
+#endif
 }
 
 /**

gdk/gdkdmabuf.c

@@ -376,10 +376,6 @@ gdk_dmabuf_direct_downloader_add_formats (const GdkDmabufDownloader *downloader,
 
   for (i = 0; i < G_N_ELEMENTS (supported_formats); i++)
     {
-      GDK_DEBUG (DMABUF, "%s dmabuf format %.4s:%#" G_GINT64_MODIFIER "x",
-                 downloader->name,
-                 (char *) &supported_formats[i].fourcc, (guint64) DRM_FORMAT_MOD_LINEAR);
-
       gdk_dmabuf_formats_builder_add_format (builder,
                                              supported_formats[i].fourcc,
                                              DRM_FORMAT_MOD_LINEAR);

gdk/gdkdmabufegl.c

@@ -99,25 +99,17 @@ gdk_dmabuf_egl_downloader_collect_formats (const GdkDmabufDownloader *downloader
 
       for (int j = 0; j < num_modifiers; j++)
         {
-          /* All linear formats we support are already added my the mmap downloader.
+          if (!external_only[j])
-           * We don't add external formats, unless we can use them (via GLES)
-           */
-          if (modifiers[j] != DRM_FORMAT_MOD_LINEAR &&
-              (!external_only[j] || gdk_gl_context_get_use_es (context)))
             {
-              GDK_DEBUG (DMABUF, "%s%s dmabuf format %.4s:%#" G_GINT64_MODIFIER "x",
-                         external_only[j] ? "external " : "",
-                         downloader->name,
-                         (char *) &fourccs[i],
-                         modifiers[j]);
-
               gdk_dmabuf_formats_builder_add_format (formats, fourccs[i], modifiers[j]);
-            }
-          if (external_only[j])
-            gdk_dmabuf_formats_builder_add_format (external, fourccs[i], modifiers[j]);
-          else
               all_external = FALSE;
             }
+          else
+            {
+              if (gdk_gl_context_get_use_es (context))
+                gdk_dmabuf_formats_builder_add_format (external, fourccs[i], modifiers[j]);
+            }
+        }
 
       /* Accept implicit modifiers as long as we accept the format at all.
        * This is a bit of a crapshot, but unfortunately needed for a bunch
@@ -126,9 +118,9 @@ gdk_dmabuf_egl_downloader_collect_formats (const GdkDmabufDownloader *downloader
        * As an extra wrinkle, treat the implicit modifier as 'external only'
        * if all formats with the same fourcc are 'external only'.
        */
-      if (!all_external || gdk_gl_context_get_use_es (context))
+      if (!all_external)
         gdk_dmabuf_formats_builder_add_format (formats, fourccs[i], DRM_FORMAT_MOD_INVALID);
-      if (all_external)
+      else if (gdk_gl_context_get_use_es (context))
         gdk_dmabuf_formats_builder_add_format (external, fourccs[i], DRM_FORMAT_MOD_INVALID);
     }
 
@@ -144,6 +136,7 @@ gdk_dmabuf_egl_downloader_add_formats (const GdkDmabufDownloader *downloader,
                                        GdkDisplay                *display,
                                        GdkDmabufFormatsBuilder   *builder)
 {
+  GdkGLContext *context = gdk_display_get_gl_context (display);
   GdkDmabufFormatsBuilder *formats;
   GdkDmabufFormatsBuilder *external;
   gboolean retval = FALSE;
@@ -160,6 +153,11 @@ gdk_dmabuf_egl_downloader_add_formats (const GdkDmabufDownloader *downloader,
   display->egl_external_formats = gdk_dmabuf_formats_builder_free_to_formats (external);
 
   gdk_dmabuf_formats_builder_add_formats (builder, display->egl_dmabuf_formats);
+  if (gdk_gl_context_get_use_es (context))
+    {
+      gdk_dmabuf_formats_builder_next_priority (builder);
+      gdk_dmabuf_formats_builder_add_formats (builder, display->egl_external_formats);
+    }
 
   return retval;
 }

gdk/gdkdmabufformats.c

@@ -151,6 +151,35 @@ gdk_dmabuf_formats_get_format (GdkDmabufFormats *formats,
   *modifier = format->modifier;
 }
 
+/**
+ * gdk_dmabuf_formats_next_priority:
+ * @formats: a `GdkDmabufFormats`
+ * @idx: the index of the format to query
+*
+ * Returns the index of the next-lower-priority format.
+ *
+ * The formats in a `GdkDmabufFormats` are sorted by decreasing
+ * priority. This function lets you identify formats with the
+ * same priority: all the formats between @idx and the return
+ * value of this function have the same priority.
+ *
+ * Returns: the index of the next lower priority format
+ *
+ * Since: 4.14
+ */
+gsize
+gdk_dmabuf_formats_next_priority (GdkDmabufFormats *formats,
+                                  gsize             idx)
+{
+  GdkDmabufFormat *format;
+
+  g_return_val_if_fail (idx < formats->n_formats, G_MAXSIZE);
+
+  format = &formats->formats[idx];
+
+  return format->next_priority;
+}
+
 /**
  * gdk_dmabuf_format_contains:
  * @formats: a `GdkDmabufFormats`

gdk/gdkdmabufformats.h

@@ -46,6 +46,10 @@ void               gdk_dmabuf_formats_get_format      (GdkDmabufFormats *formats
                                                        guint32          *fourcc,
                                                        guint64          *modifier);
 
+GDK_AVAILABLE_IN_4_14
+gsize              gdk_dmabuf_formats_next_priority   (GdkDmabufFormats *formats,
+                                                       gsize             idx);
+
 GDK_AVAILABLE_IN_4_14
 gboolean           gdk_dmabuf_formats_contains        (GdkDmabufFormats *formats,
                                                        guint32           fourcc,

gdk/gdkdmabufformatsbuilder.c

@@ -52,7 +52,9 @@ gdk_dmabuf_format_compare (gconstpointer data_a,
   const GdkDmabufFormat *a = data_a;
   const GdkDmabufFormat *b = data_b;
 
-  if (a->fourcc == b->fourcc)
+  if (a->next_priority != b->next_priority)
+    return (a->next_priority < b->next_priority) ? -1 : 1;
+  else if (a->fourcc == b->fourcc)
     return (a->modifier - b->modifier) >> 8 * (sizeof (gint64) - sizeof (gint));
   else
     return a->fourcc - b->fourcc;
@@ -79,31 +81,13 @@ gdk_dmabuf_formats_builder_sort (GdkDmabufFormatsBuilder *self)
          gdk_dmabuf_format_compare);
 }
 
-/* list must be sorted */
-static void
-gdk_dmabuf_formats_builder_remove_duplicates (GdkDmabufFormatsBuilder *self)
-{
-  gsize i, j;
-
-  for (i = 1, j = 0; i < gdk_dmabuf_formats_builder_get_size (self); i++)
-    {
-      if (gdk_dmabuf_format_equal (gdk_dmabuf_formats_builder_get (self, i),
-                                   gdk_dmabuf_formats_builder_get (self, j)))
-        continue;
-
-      j++;
-      if (i != j)
-        *gdk_dmabuf_formats_builder_index (self, j) = *gdk_dmabuf_formats_builder_index (self, i);
-    }
-}
-
 GdkDmabufFormats *
 gdk_dmabuf_formats_builder_free_to_formats (GdkDmabufFormatsBuilder *self)
 {
   GdkDmabufFormats *formats;
 
+  gdk_dmabuf_formats_builder_next_priority (self);
   gdk_dmabuf_formats_builder_sort (self);
-  gdk_dmabuf_formats_builder_remove_duplicates (self);
 
   formats = gdk_dmabuf_formats_new (gdk_dmabuf_formats_builder_get_data (self),
                                     gdk_dmabuf_formats_builder_get_size (self));
@@ -118,17 +102,41 @@ gdk_dmabuf_formats_builder_add_format (GdkDmabufFormatsBuilder *self,
                                        guint32                  fourcc,
                                        guint64                  modifier)
 {
-  gdk_dmabuf_formats_builder_append (self, &(GdkDmabufFormat) { fourcc, modifier });
+  GdkDmabufFormat format = { fourcc, modifier, G_MAXSIZE };
+
+  for (gsize i = 0; i < gdk_dmabuf_formats_builder_get_size (self); i++)
+    {
+      if (gdk_dmabuf_format_equal (gdk_dmabuf_formats_builder_get (self, i), &format))
+        return;
+    }
+
+  gdk_dmabuf_formats_builder_append (self, &format);
+}
+
+void
+gdk_dmabuf_formats_builder_next_priority (GdkDmabufFormatsBuilder *self)
+{
+  for (gsize i = gdk_dmabuf_formats_builder_get_size (self); i > 0; i--)
+    {
+      GdkDmabufFormat *format = gdk_dmabuf_formats_builder_get (self, i - 1);
+
+      if (format->next_priority != G_MAXSIZE)
+        break;
+
+       format->next_priority = gdk_dmabuf_formats_builder_get_size (self);
+    }
 }
 
 void
 gdk_dmabuf_formats_builder_add_formats (GdkDmabufFormatsBuilder *self,
                                         GdkDmabufFormats        *formats)
 {
-  gdk_dmabuf_formats_builder_splice (self,
+  for (gsize i = 0; i < gdk_dmabuf_formats_get_n_formats (formats); i++)
-                                     gdk_dmabuf_formats_builder_get_size (self),
+    {
-                                     0,
+      guint32 fourcc;
-                                     FALSE,
+      guint64 modifier;
-                                     gdk_dmabuf_formats_peek_formats (formats),
+
-                                     gdk_dmabuf_formats_get_n_formats (formats));
+      gdk_dmabuf_formats_get_format (formats, i, &fourcc, &modifier);
+      gdk_dmabuf_formats_builder_add_format (self, fourcc, modifier);
+    }
 }

gdk/gdkdmabufformatsbuilderprivate.h

@@ -10,5 +10,7 @@ GdkDmabufFormats *              gdk_dmabuf_formats_builder_free_to_formats
 void                            gdk_dmabuf_formats_builder_add_format           (GdkDmabufFormatsBuilder        *self,
                                                                                  guint32                         fourcc,
                                                                                  guint64                         modifier);
+
+void                            gdk_dmabuf_formats_builder_next_priority        (GdkDmabufFormatsBuilder        *self);
 void                            gdk_dmabuf_formats_builder_add_formats          (GdkDmabufFormatsBuilder        *self,
                                                                                  GdkDmabufFormats               *formats);

gdk/gdkdmabufformatsprivate.h

@@ -7,6 +7,7 @@ struct _GdkDmabufFormat
 {
   guint32 fourcc;
   guint64 modifier;
+  gsize next_priority;
 };
 
 GdkDmabufFormats *      gdk_dmabuf_formats_new                  (GdkDmabufFormat  *formats,

gdk/gdkdmabuftexture.c

@@ -138,6 +138,7 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder,
   GError *local_error = NULL;
   int width, height;
   gsize i;
+  const GdkDmabufDownloader *downloader = NULL;
 
   display = gdk_dmabuf_texture_builder_get_display (builder);
   width = gdk_dmabuf_texture_builder_get_width (builder);
@@ -152,6 +153,10 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder,
 
   gdk_display_init_dmabuf (display);
 
+  if (dmabuf.modifier == DRM_FORMAT_MOD_LINEAR)
+    downloader = gdk_dmabuf_get_direct_downloader ();
+  else
+    {
       for (i = 0; display->dmabuf_downloaders[i] != NULL; i++)
         {
           if (local_error && g_error_matches (local_error, GDK_DMABUF_ERROR, GDK_DMABUF_ERROR_UNSUPPORTED_FORMAT))
@@ -163,10 +168,14 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder,
                                                         gdk_dmabuf_texture_builder_get_premultiplied (builder),
                                                         &format,
                                                         local_error ? NULL : &local_error))
+            {
+              downloader = display->dmabuf_downloaders[i];
               break;
             }
+        }
+    }
 
-  if (display->dmabuf_downloaders[i] == NULL)
+  if (downloader == NULL)
     {
       g_propagate_error (error, local_error);
       return NULL;
@@ -178,7 +187,7 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder,
              gdk_dmabuf_texture_builder_get_premultiplied (builder) ? " premultiplied, " : "",
              dmabuf.n_planes,
              format,
-             display->dmabuf_downloaders[i]->name);
+             downloader->name);
 
   self = g_object_new (GDK_TYPE_DMABUF_TEXTURE,
                        "width", width,
@@ -187,7 +196,7 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder,
 
   GDK_TEXTURE (self)->format = format;
   g_set_object (&self->display, display);
-  self->downloader = display->dmabuf_downloaders[i];
+  self->downloader = downloader;
   self->dmabuf = dmabuf;
   self->destroy = destroy;
   self->data = data;

testsuite/gdk/dmabuftexture.c

@@ -4,6 +4,8 @@
 #include <gdk/gdkdisplayprivate.h>
 #include <gdk/gdkglcontextprivate.h>
 #include <gdk/gdkdmabuftextureprivate.h>
+#include <gdk/gdkdmabufformatsprivate.h>
+#include <gdk/gdkdmabufformatsbuilderprivate.h>
 
 #ifdef HAVE_DMABUF
 #include <drm_fourcc.h>
@@ -34,6 +36,56 @@ test_dmabuf_formats (void)
 #endif
 }
 
+#define AAAA fourcc_code ('A', 'A', 'A', 'A')
+#define BBBB fourcc_code ('B', 'B', 'B', 'B')
+#define CCCC fourcc_code ('C', 'C', 'C', 'C')
+#define DDDD fourcc_code ('D', 'D', 'D', 'D')
+
+static gboolean
+dmabuf_format_matches (const GdkDmabufFormat *f1, guint32 fourcc, guint64 modifier, gsize next_priority)
+{
+  return f1->fourcc == fourcc &&
+         f1->modifier == modifier &&
+         f1->next_priority == next_priority;
+}
+
+/* Test that sorting respects priorities, and the highest priority instance
+ * of duplicates is kept.
+ */
+static void
+test_priorities (void)
+{
+  GdkDmabufFormatsBuilder *builder;
+  GdkDmabufFormats *formats;
+  const GdkDmabufFormat *f;
+
+  builder = gdk_dmabuf_formats_builder_new ();
+
+  gdk_dmabuf_formats_builder_add_format (builder, AAAA, DRM_FORMAT_MOD_LINEAR);
+  gdk_dmabuf_formats_builder_add_format (builder, BBBB, DRM_FORMAT_MOD_LINEAR);
+  gdk_dmabuf_formats_builder_add_format (builder, AAAA, I915_FORMAT_MOD_X_TILED);
+  gdk_dmabuf_formats_builder_next_priority (builder);
+  gdk_dmabuf_formats_builder_add_format (builder, DDDD, I915_FORMAT_MOD_X_TILED);
+  gdk_dmabuf_formats_builder_add_format (builder, BBBB, I915_FORMAT_MOD_X_TILED);
+  gdk_dmabuf_formats_builder_add_format (builder, CCCC, DRM_FORMAT_MOD_LINEAR);
+  gdk_dmabuf_formats_builder_add_format (builder, BBBB, DRM_FORMAT_MOD_LINEAR); // a duplicate
+
+  formats = gdk_dmabuf_formats_builder_free_to_formats (builder);
+
+  g_assert_true (gdk_dmabuf_formats_get_n_formats (formats) == 6);
+
+  f = gdk_dmabuf_formats_peek_formats (formats);
+
+  g_assert_true (dmabuf_format_matches (&f[0], AAAA, DRM_FORMAT_MOD_LINEAR, 3));
+  g_assert_true (dmabuf_format_matches (&f[1], AAAA, I915_FORMAT_MOD_X_TILED, 3));
+  g_assert_true (dmabuf_format_matches (&f[2], BBBB, DRM_FORMAT_MOD_LINEAR, 3));
+  g_assert_true (dmabuf_format_matches (&f[3], BBBB, I915_FORMAT_MOD_X_TILED, 6));
+  g_assert_true (dmabuf_format_matches (&f[4], CCCC, DRM_FORMAT_MOD_LINEAR, 6));
+  g_assert_true (dmabuf_format_matches (&f[5], DDDD, I915_FORMAT_MOD_X_TILED, 6));
+
+  gdk_dmabuf_formats_unref (formats);
+}
+
 static cairo_surface_t *
 make_surface (int width,
               int height)
@@ -288,6 +340,7 @@ main (int argc, char *argv[])
   gtk_test_init (&argc, &argv, NULL);
 
   g_test_add_func ("/dmabuf/formats", test_dmabuf_formats);
+  g_test_add_func ("/dmabuf/priorities", test_priorities);
   g_test_add_func ("/dmabuf/export", test_dmabuf_export);
   g_test_add_func ("/dmabuf/import", test_dmabuf_import);