dmabuf: Properly propagate damage area

The current approach makes use of
- A tiled rendering to work around gdk_texture_diff only doing pointer
comparaison
- Assumes that we would only recieve a scanout cmd followed by multiple
flush ones

In reality, with virtio-gpu at least, the scanout cmd is always
submitted followed by a flush one containing the damaged region.

With the assumption currently made, we end up creating a new paintable
for every scanout cmd causing a full redraw instead
of only redrawing the damaged areas.
Isntead we create the paintable once and call import whenever
we receive a flush cmd (UpdateDMABUF) so we can properly
pass the damage area when creating a GdkGLTexture making
the tiled rendering no longer needed.

lib/mks-dmabuf-paintable-private.h

@@ -1,6 +1,7 @@
 /* mks-dmabuf-paintable-private.h
  *
  * Copyright 2023 Christian Hergert <chergert@redhat.com>
+ * Copyright 2023 Bilal Elmoussaoui <belmouss@redhat.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -24,23 +25,25 @@
 
 G_BEGIN_DECLS
 
+typedef struct _MksDmabufScanoutData
+{
+  guint                  width;
+  guint                  height;
+  guint                  stride;
+  guint                  fourcc;
+  guint64                modifier;
+  int                    dmabuf_fd;
+} MksDmabufScanoutData;
+
 #define MKS_TYPE_DMABUF_PAINTABLE (mks_dmabuf_paintable_get_type())
 
 G_DECLARE_FINAL_TYPE (MksDmabufPaintable, mks_dmabuf_paintable, MKS, DMABUF_PAINTABLE, GObject)
 
-MksDmabufPaintable *mks_dmabuf_paintable_new        (GdkGLContext        *gl_context,
+MksDmabufPaintable *mks_dmabuf_paintable_new        (void);
-                                                     int                  dmabuf_fd,
+gboolean            mks_dmabuf_paintable_import     (MksDmabufPaintable   *self,
-                                                     guint                width,
+                                                     GdkGLContext         *gl_context,
-                                                     guint                height,
+                                                     MksDmabufScanoutData *data,
-                                                     guint                stride,
+                                                     cairo_region_t       *region,
-                                                     guint                fourcc,
+                                                     GError              **error);
-                                                     guint64              modifier,
-                                                     gboolean             y0_top,
-                                                     GError             **error);
-void                mks_dmabuf_paintable_invalidate (MksDmabufPaintable  *self,
-                                                     guint                x,
-                                                     guint                y,
-                                                     guint                width,
-                                                     guint                height);
 
 G_END_DECLS

lib/mks-dmabuf-paintable.c

@@ -1,6 +1,7 @@
 /* mks-dmabuf-paintable.c
  *
  * Copyright 2023 Christian Hergert <chergert@redhat.com>
+ * Copyright 2023 Bilal Elmoussaoui <belmouss@redhat.com>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -28,25 +29,17 @@
 #include "mks-gl-context-private.h"
 
 /*
- * MksDmabufPaintable is a GdkPaintable for a single dmabuf_fd which is then
+ * MksDmabufPaintable is a GdkPaintable that gets created the first time
- * imported into OpenGL. This has an advantage over just using a single
+ * `ScanoutDMABUF` is called.
- * GdkGLTexture for such a situation in that we can take advantage of how
+ * 
- * GskGLRenderer and GdkTexture work.
+ * The scanout data is then stored until we receive a `UpdateDMABUF` call
- *
+ * so we can pass the damage region to `GdkGLTextureBuilder`.
- * First, by swapping between 2 GdkGLTextures, gsk_render_node_diff()
- * will see a pointer difference and ensure the tile region is damaged.
- * Since it is a dmabuf, we can already assume the contents are available
- * to the GPU by layers beneath us.
  */
 
-#define TILE_WIDTH  128
-#define TILE_HEIGHT 128
-
 struct _MksDmabufPaintable
 {
   GObject parent_instance;
-  GdkTexture *textures[2];
+  GdkTexture *texture;
-  GArray *tiles;
   guint width;
   guint height;
 };
@@ -57,12 +50,6 @@ typedef struct _MksDmabufTextureData
   GLuint texture_id;
 } MksDmabufTextureData;
 
-typedef struct _MksDmabufTile
-{
-  graphene_rect_t area;
-  guint16 texture : 1;
-} MksDmabufTile;
-
 static int
 mks_dmabuf_paintable_get_intrinsic_width (GdkPaintable *paintable)
 {
@@ -95,24 +82,8 @@ mks_dmabuf_paintable_snapshot (GdkPaintable *paintable,
   g_assert (MKS_IS_DMABUF_PAINTABLE (self));
   g_assert (GDK_IS_SNAPSHOT (snapshot));
 
-  gtk_snapshot_save (snapshot);
+  area = GRAPHENE_RECT_INIT (0, 0, width, height);
-  gtk_snapshot_scale (snapshot,
+  gtk_snapshot_append_texture (snapshot, self->texture, &area);
-                      width / (double)self->width,
-                      height / (double)self->height);
-
-  area = GRAPHENE_RECT_INIT (0, 0, self->width, self->height);
-
-  for (guint i = 0; i < self->tiles->len; i++)
-    {
-      MksDmabufTile *tile = &g_array_index (self->tiles, MksDmabufTile, i);
-      GdkTexture *texture = self->textures[tile->texture];
-
-      gtk_snapshot_push_clip (snapshot, &tile->area);
-      gtk_snapshot_append_texture (snapshot, texture, &area);
-      gtk_snapshot_pop (snapshot);
-    }
-
-  gtk_snapshot_restore (snapshot);
 }
 
 static void
@@ -174,9 +145,7 @@ mks_dmabuf_paintable_dispose (GObject *object)
 {
   MksDmabufPaintable *self = (MksDmabufPaintable *)object;
 
-  g_clear_pointer (&self->tiles, g_array_unref);
+  g_clear_object (&self->texture);
-  g_clear_object (&self->textures[0]);
-  g_clear_object (&self->textures[1]);
 
   G_OBJECT_CLASS (mks_dmabuf_paintable_parent_class)->dispose (object);
 }
@@ -192,112 +161,93 @@ mks_dmabuf_paintable_class_init (MksDmabufPaintableClass *klass)
 static void
 mks_dmabuf_paintable_init (MksDmabufPaintable *self)
 {
-  self->tiles = g_array_new (FALSE, FALSE, sizeof (MksDmabufTile));
 }
 
-MksDmabufPaintable *
+gboolean
-mks_dmabuf_paintable_new (GdkGLContext  *gl_context,
+mks_dmabuf_paintable_import (MksDmabufPaintable   *self,
-                          int            dmabuf_fd,
+                             GdkGLContext         *gl_context,
-                          guint          width,
+                             MksDmabufScanoutData *data,
-                          guint          height,
+                             cairo_region_t       *region,
-                          guint          stride,
+                             GError              **error)
-                          guint          fourcc,
-                          guint64        modifier,
-                          gboolean       y0_top,
-                          GError       **error)
 {
   g_autoptr(MksDmabufTextureData) texture_data = NULL;
-  g_autoptr(MksDmabufPaintable) self = NULL;
   GLuint texture_id;
+  g_autoptr(GdkGLTextureBuilder) builder = NULL;
   guint zero = 0;
 
-  if (dmabuf_fd < 0)
+  g_return_val_if_fail (MKS_IS_DMABUF_PAINTABLE (self), FALSE);
+
+  if (data->dmabuf_fd < 0)
     {
       g_set_error  (error,
                     G_IO_ERROR,
                     G_IO_ERROR_INVALID_ARGUMENT,
                     "invalid dmabuf_fd (%d)",
-                    dmabuf_fd);
+                    data->dmabuf_fd);
-      return NULL;
+      return FALSE;
     }
 
-  if (width == 0 || height == 0 || stride == 0)
+  if (data->width == 0 || data->height == 0 || data->stride == 0)
     {
       g_set_error  (error,
                     G_IO_ERROR,
                     G_IO_ERROR_INVALID_ARGUMENT,
                     "invalid width/height/stride (%u/%u/%u)",
-                    width, height, stride);
+                    data->width, data->height, data->stride);
-      return NULL;
+      return FALSE;
+    }
+
+  if (self->width != data->width || self->height != data->height)
+    {
+      self->width = data->width;
+      self->height = data->height;
+      gdk_paintable_invalidate_size (GDK_PAINTABLE (self));
     }
 
-  self = g_object_new (MKS_TYPE_DMABUF_PAINTABLE, NULL);
-  self->width = width;
-  self->height = height;
 
   if (!(texture_id = mks_gl_context_import_dmabuf (gl_context,
-                                                   fourcc, width, height,
+                                                   data->fourcc, data->width, data->height,
-                                                   1, &dmabuf_fd, &stride, &zero, &modifier)))
+                                                   1, &data->dmabuf_fd, &data->stride, &zero, 
+                                                   &data->modifier)))
     {
       g_set_error (error,
                    G_IO_ERROR,
                    G_IO_ERROR_FAILED,
                    "Failed to import dmabuf into GL texture");
-      return NULL;
+      return FALSE;
+    }
+
+  builder = gdk_gl_texture_builder_new ();
+  gdk_gl_texture_builder_set_id (builder, texture_id);
+  gdk_gl_texture_builder_set_width (builder, self->width);
+  gdk_gl_texture_builder_set_height (builder, self->height);
+  gdk_gl_texture_builder_set_context (builder, gl_context);
+  
+  if (region)
+    {
+      gdk_gl_texture_builder_set_update_region (builder, region);
+      gdk_gl_texture_builder_set_update_texture (builder, self->texture);
     }
 
   texture_data = mks_dmabuf_texture_data_new (gl_context, texture_id);
-
+  g_clear_object (&self->texture); 
-  self->textures[0] = gdk_gl_texture_new (gl_context, texture_id, width, height,
+  self->texture = gdk_gl_texture_builder_build (builder, 
-                                          (GDestroyNotify) mks_dmabuf_texture_data_unref,
+                                                (GDestroyNotify) mks_dmabuf_texture_data_unref,
-                                          mks_dmabuf_texture_data_ref (texture_data));
+                                                mks_dmabuf_texture_data_ref (texture_data)
-  self->textures[1] = gdk_gl_texture_new (gl_context, texture_id, width, height,
+                                               );
-                                          (GDestroyNotify) mks_dmabuf_texture_data_unref,
-                                          mks_dmabuf_texture_data_ref (texture_data));
-
-  for (guint y = 0; y < height; y += TILE_HEIGHT)
-    {
-      guint tile_height = MIN (TILE_HEIGHT, height - y);
-
-      for (guint x = 0; x < width; x += TILE_WIDTH)
-        {
-          MksDmabufTile tile;
-          guint tile_width = MIN (TILE_WIDTH, width - x);
-
-          tile.area = GRAPHENE_RECT_INIT (x, y, tile_width, tile_height);
-          tile.texture = 0;
-
-          g_array_append_val (self->tiles, tile);
-        }
-    }
-
-  return g_steal_pointer (&self);
-}
-
-void
-mks_dmabuf_paintable_invalidate (MksDmabufPaintable *self,
-                                 guint               x,
-                                 guint               y,
-                                 guint               width,
-                                 guint               height)
-{
-  graphene_rect_t area;
-
-  g_return_if_fail (MKS_IS_DMABUF_PAINTABLE (self));
-
-  if (width == 0 || height == 0)
-    return;
-
-  area = GRAPHENE_RECT_INIT (x, y, width, height);
-
-  for (guint i = 0; i < self->tiles->len; i++)
-    {
-      MksDmabufTile *tile = &g_array_index (self->tiles, MksDmabufTile, i);
-      G_GNUC_UNUSED graphene_rect_t res;
-
-      if (graphene_rect_intersection (&area, &tile->area, &res))
-        tile->texture = !tile->texture;
-    }
 
   gdk_paintable_invalidate_contents (GDK_PAINTABLE (self));
+  return TRUE;
+}
+
+
+MksDmabufPaintable *
+mks_dmabuf_paintable_new (void)
+{
+  g_autoptr(MksDmabufPaintable) self = NULL;
+
+  self = g_object_new (MKS_TYPE_DMABUF_PAINTABLE, NULL);
+  self->width = 0;
+  self->height = 0;
+  return g_steal_pointer (&self);
 }

lib/mks-paintable.c

@@ -38,15 +38,16 @@
 
 struct _MksPaintable
 {
-  GObject          parent_instance;
+  GObject               parent_instance;
-  GdkGLContext    *gl_context;
+  GdkGLContext         *gl_context;
-  MksQemuListener *listener;
+  MksQemuListener      *listener;
-  GDBusConnection *connection;
+  GDBusConnection      *connection;
-  GdkPaintable    *child;
+  GdkPaintable         *child;
-  GdkCursor       *cursor;
+  GdkCursor            *cursor;
-  int              mouse_x;
+  MksDmabufScanoutData *scanout_data;
-  int              mouse_y;
+  int                   mouse_x;
-  guint            y_inverted : 1;
+  int                   mouse_y;
+  guint                 y_inverted : 1;
 };
 
 enum {
@@ -332,8 +333,8 @@ mks_paintable_listener_scanout_dmabuf (MksPaintable          *self,
 {
   g_autoptr(MksDmabufPaintable) child = NULL;
   g_autoptr(GError) error = NULL;
-  g_autofd int dmabuf_fd = -1;
+  int dmabuf_fd = -1;
-  GdkGLContext *gl_context;
+  MksDmabufScanoutData *scanout_data;
   guint handle;
 
   g_assert (MKS_IS_PAINTABLE (self));
@@ -352,22 +353,29 @@ mks_paintable_listener_scanout_dmabuf (MksPaintable          *self,
       return TRUE;
     }
 
-  if (-1 == (dmabuf_fd = g_unix_fd_list_get (unix_fd_list, handle, &error)) ||
+  if (!MKS_IS_DMABUF_PAINTABLE (self->child))
-      !(gl_context = mks_paintable_get_gl_context (self, &error)) ||
+    {
-      !(child = mks_dmabuf_paintable_new (gl_context,
+      child = mks_dmabuf_paintable_new ();
-                                          dmabuf_fd,
+      mks_paintable_set_child (self, GDK_PAINTABLE (child));
-                                          width, height,
+    }
-                                          stride, fourcc,
+
-                                          modifier, y0_top,
+  if (-1 == (dmabuf_fd = g_unix_fd_list_get (unix_fd_list, handle, &error)))
-                                          &error)))
     {
       g_dbus_method_invocation_return_gerror (invocation, error);
       return TRUE;
     }
 
   self->y_inverted = !y0_top;
-
+  scanout_data = g_new0 (MksDmabufScanoutData, 1);
-  mks_paintable_set_child (self, GDK_PAINTABLE (child));
+  
+  scanout_data->dmabuf_fd = dmabuf_fd;
+  scanout_data->width = width;
+  scanout_data->height = height;
+  scanout_data->stride = stride;
+  scanout_data->fourcc = fourcc;
+  scanout_data->modifier = modifier;
+  g_clear_pointer (&self->scanout_data, g_free);
+  self->scanout_data = scanout_data;
 
   mks_qemu_listener_complete_scanout_dmabuf (listener, invocation, NULL);
 
@@ -383,14 +391,36 @@ mks_paintable_listener_update_dmabuf (MksPaintable          *self,
                                       int                    height,
                                       MksQemuListener       *listener)
 {
+  cairo_region_t *region = NULL;
+  g_autoptr(GError) error = NULL;
+  GdkGLContext *gl_context;
+
   g_assert (MKS_IS_PAINTABLE (self));
   g_assert (G_IS_DBUS_METHOD_INVOCATION (invocation));
   g_assert (MKS_QEMU_IS_LISTENER (listener));
 
   if (MKS_IS_DMABUF_PAINTABLE (self->child))
-    mks_dmabuf_paintable_invalidate (MKS_DMABUF_PAINTABLE (self->child), x, y, width, height);
+    { 
+      g_assert (self->scanout_data != NULL);
+      if (!self->y_inverted)
+        y = self->scanout_data->height - y - height;
 
+      region = cairo_region_create_rectangle (&(cairo_rectangle_int_t) { x, y, width, height });
+      if (!(gl_context = mks_paintable_get_gl_context (self, &error)) ||
+          !mks_dmabuf_paintable_import (MKS_DMABUF_PAINTABLE (self->child),
+                                        gl_context,
+                                        self->scanout_data,
+                                        region,
+                                        &error))
+        {
+          g_dbus_method_invocation_return_gerror (invocation, error);
+          goto cleanup;
+        }
+    }
+  
   mks_qemu_listener_complete_update_dmabuf (listener, invocation);
+cleanup:
+  g_clear_pointer (&region, cairo_region_destroy);
 
   return TRUE;
 }