Merge branch 'wip/otte/texture-threads' into 'master'

gltexture: Make sure downloading textures works in a different thread

See merge request GNOME/gtk!3942

gdk/gdkgltexture.c

@@ -20,6 +20,7 @@
 
 #include "gdkgltextureprivate.h"
 
+#include "gdkdisplayprivate.h"
 #include "gdkmemorytextureprivate.h"
 #include "gdktextureprivate.h"
 
@@ -69,26 +70,56 @@ gdk_gl_texture_dispose (GObject *object)
   G_OBJECT_CLASS (gdk_gl_texture_parent_class)->dispose (object);
 }
 
-static GdkTexture *
-gdk_gl_texture_download_texture (GdkTexture *texture)
+typedef struct _InvokeData
 {
-  GdkGLTexture *self = GDK_GL_TEXTURE (texture);
-  GdkTexture *result;
-  int active_texture;
+  GdkGLTexture *self;
+  volatile int spinlock;
+  GFunc func;
+  gpointer data;
+} InvokeData;
+
+static gboolean
+gdk_gl_texture_invoke_callback (gpointer data)
+{
+  InvokeData *invoke = data;
+  GdkGLContext *context;
+
+  context = gdk_display_get_gl_context (gdk_gl_context_get_display (invoke->self->context));
+
+  gdk_gl_context_make_current (context);
+  glBindTexture (GL_TEXTURE_2D, invoke->self->id);
+
+  invoke->func (invoke->self, invoke->data);
+
+  g_atomic_int_set (&invoke->spinlock, 1);
+
+  return FALSE;
+}
+
+static void
+gdk_gl_texture_run (GdkGLTexture *self,
+                    GFunc         func,
+                    gpointer      data)
+{
+  InvokeData invoke = { self, 0, func, data };
+
+  g_main_context_invoke (NULL, gdk_gl_texture_invoke_callback, &invoke);
+
+  while (g_atomic_int_get (&invoke.spinlock) == 0);
+}
+
+static void
+gdk_gl_texture_do_download_texture (gpointer texture_,
+                                    gpointer result_)
+{
+  GdkTexture *texture = texture_;
+  GdkTexture **result = result_;
   GdkMemoryFormat format;
   GLint internal_format, gl_format, gl_type;
   guchar *data;
   gsize stride;
   GBytes *bytes;
 
-  if (self->saved)
-    return g_object_ref (self->saved);
-
-  gdk_gl_context_make_current (self->context);
-
-  glGetIntegerv (GL_TEXTURE_BINDING_2D, &active_texture);
-  glBindTexture (GL_TEXTURE_2D, self->id);
-
   glGetTexLevelParameteriv (GL_TEXTURE_2D, 0, GL_TEXTURE_INTERNAL_FORMAT, &internal_format);
 
   switch (internal_format)
@@ -161,26 +192,53 @@ gdk_gl_texture_download_texture (GdkTexture *texture)
                  data);
 
   bytes = g_bytes_new_take (data, stride * texture->height);
-  result = gdk_memory_texture_new (texture->width,
-                                   texture->height,
-                                   format,
-                                   bytes,
-                                   stride);
+  *result = gdk_memory_texture_new (texture->width,
+                                    texture->height,
+                                    format,
+                                    bytes,
+                                    stride);
 
   g_bytes_unref (bytes);
+}
 
-  glBindTexture (GL_TEXTURE_2D, active_texture);
+static GdkTexture *
+gdk_gl_texture_download_texture (GdkTexture *texture)
+{
+  GdkGLTexture *self = GDK_GL_TEXTURE (texture);
+  GdkTexture *result;
+
+  if (self->saved)
+    return g_object_ref (self->saved);
+
+  gdk_gl_texture_run (self, gdk_gl_texture_do_download_texture, &result);
 
   return result;
 }
 
+static void
+gdk_gl_texture_do_download (gpointer texture,
+                            gpointer data)
+{
+  glGetTexImage (GL_TEXTURE_2D,
+                 0,
+                 GL_BGRA,
+#if G_BYTE_ORDER == G_LITTLE_ENDIAN
+                 GL_UNSIGNED_INT_8_8_8_8_REV,
+#elif G_BYTE_ORDER == G_BIG_ENDIAN
+                 GL_UNSIGNED_BYTE,
+#else
+#error "Unknown byte order for gdk_gl_texture_download()"
+#endif
+                 data);
+
+}
+
 static void
 gdk_gl_texture_download (GdkTexture *texture,
                          guchar     *data,
                          gsize       stride)
 {
   GdkGLTexture *self = GDK_GL_TEXTURE (texture);
-  GLint active_texture;
 
   if (self->saved)
     {
@@ -195,45 +253,18 @@ gdk_gl_texture_download (GdkTexture *texture,
       return;
     }
 
-  gdk_gl_context_make_current (self->context);
-
-  glGetIntegerv (GL_TEXTURE_BINDING_2D, &active_texture);
-  glBindTexture (GL_TEXTURE_2D, self->id);
-
-  glGetTexImage (GL_TEXTURE_2D,
-                 0,
-                 GL_BGRA,
-#if G_BYTE_ORDER == G_LITTLE_ENDIAN
-                 GL_UNSIGNED_INT_8_8_8_8_REV,
-#elif G_BYTE_ORDER == G_BIG_ENDIAN
-                 GL_UNSIGNED_BYTE,
-#else
-#error "Unknown byte order for gdk_gl_texture_download()"
-#endif
-                 data);
-
-  glBindTexture (GL_TEXTURE_2D, active_texture);
+  gdk_gl_texture_run (self, gdk_gl_texture_do_download, data);
 }
 
 static void
-gdk_gl_texture_do_download_float (GdkTexture *texture,
-                                  float      *data)
+gdk_gl_texture_do_download_float (gpointer texture,
+                                  gpointer data)
 {
-  GdkGLTexture *self = GDK_GL_TEXTURE (texture);
-  int active_texture;
-
-  gdk_gl_context_make_current (self->context);
-
-  glGetIntegerv (GL_TEXTURE_BINDING_2D, &active_texture);
-  glBindTexture (GL_TEXTURE_2D, self->id);
-
   glGetTexImage (GL_TEXTURE_2D,
                  0,
                  GL_RGBA,
                  GL_FLOAT,
                  data);
-
-  glBindTexture (GL_TEXTURE_2D, active_texture);
 }
 
 static void
@@ -256,13 +287,13 @@ gdk_gl_texture_download_float (GdkTexture *texture,
 
   if (stride == width * 4)
     {
-      gdk_gl_texture_do_download_float (texture, data);
+      gdk_gl_texture_run (self, gdk_gl_texture_do_download_float, data);
       return;
     }
 
   copy = g_new (float, width * height * 4);
 
-  gdk_gl_texture_do_download_float (texture, copy);
+  gdk_gl_texture_run (self, gdk_gl_texture_do_download_float, copy);
   for (y = 0; y < height; y++)
     memcpy (data + y * stride, copy + y * 4 * width, 4 * width);
 

gsk/ngl/gskngldriver.c

@@ -735,8 +735,7 @@ gsk_ngl_driver_load_texture (GskNglDriver *self,
                              int           mag_filter)
 {
   GdkGLContext *context;
-  GdkTexture *downloaded_texture = NULL;
-  GdkTexture *source_texture;
+  GdkTexture *downloaded_texture;
   GskNglTexture *t;
   guint texture_id;
   int height;
@@ -760,21 +759,7 @@ gsk_ngl_driver_load_texture (GskNglDriver *self,
         }
       else
         {
-          cairo_surface_t *surface;
-
-          /* In this case, we have to temporarily make the texture's
-           * context the current one, download its data into our context
-           * and then create a texture from it. */
-          if (texture_context != NULL)
-            gdk_gl_context_make_current (texture_context);
-
-          surface = gdk_texture_download_surface (texture);
-          downloaded_texture = gdk_texture_new_for_surface (surface);
-          cairo_surface_destroy (surface);
-
-          gdk_gl_context_make_current (context);
-
-          source_texture = downloaded_texture;
+          downloaded_texture = gdk_texture_download_texture (texture);
         }
     }
   else
@@ -785,13 +770,17 @@ gsk_ngl_driver_load_texture (GskNglDriver *self,
             return t->texture_id;
         }
 
-      source_texture = texture;
+      downloaded_texture = gdk_texture_download_texture (texture);
     }
 
+  /* The download_texture() call may have switched the GL context. Make sure
+   * the right context is at work again. */
+  gdk_gl_context_make_current (context);
+
   width = gdk_texture_get_width (texture);
   height = gdk_texture_get_height (texture);
   texture_id = gsk_ngl_command_queue_upload_texture (self->command_queue,
-                                                     source_texture,
+                                                     downloaded_texture,
                                                      0,
                                                      0,
                                                      width,

testsuite/gdk/meson.build

@@ -26,6 +26,7 @@ tests = [
   'rgba',
   'seat',
   'texture',
+  'texture-threads',
 ]
 
 foreach t : tests

testsuite/gdk/texture-threads.c

@@ -0,0 +1,103 @@
+#include <gtk/gtk.h>
+
+#include "gsk/ngl/gsknglrenderer.h"
+
+/* This function will be called from a thread and/or the main loop.
+ * Textures are threadsafe after all. */
+static void
+ensure_texture_access (GdkTexture *texture)
+{
+  /* Make sure to initialize the pixel to anything but red */
+  guint32 pixel = 0;
+  float float_pixel[4] = { INFINITY, INFINITY, INFINITY, INFINITY };
+
+  /* Just to be sure */
+  g_assert_cmpint (gdk_texture_get_width (texture), ==, 1);
+  g_assert_cmpint (gdk_texture_get_height (texture), ==, 1);
+
+  /* download the pixel */
+  gdk_texture_download (texture, (guchar *) &pixel, 4);
+  gdk_texture_download_float (texture, float_pixel, 4);
+
+  /* check the pixel is now red */
+  g_assert_cmphex (pixel, ==, 0xFFFF0000);
+  g_assert_cmpfloat (float_pixel[0], ==, 1.0);
+  g_assert_cmpfloat (float_pixel[1], ==, 0.0);
+  g_assert_cmpfloat (float_pixel[2], ==, 0.0);
+  g_assert_cmpfloat (float_pixel[3], ==, 1.0);
+}
+
+static void
+texture_download_done (GObject      *texture,
+                       GAsyncResult *res,
+                       gpointer      loop)
+{
+  ensure_texture_access (GDK_TEXTURE (texture));
+
+  g_main_loop_quit (loop);
+}
+
+static void
+texture_download_thread (GTask        *task,
+                         gpointer      texture,
+                         gpointer      unused,
+                         GCancellable *cancellable)
+{
+  ensure_texture_access (GDK_TEXTURE (texture));
+
+  g_task_return_boolean (task, TRUE);
+}
+
+static void
+texture_threads (void)
+{
+  GdkSurface *surface;
+  GskRenderer *gl_renderer;
+  GskRenderNode *node;
+  GMainLoop *loop;
+  GdkTexture *texture;
+  GTask *task;
+
+  /* 1. Get a GL renderer */
+  surface = gdk_surface_new_toplevel (gdk_display_get_default());
+  gl_renderer = gsk_ngl_renderer_new ();
+  g_assert_true (gsk_renderer_realize (gl_renderer, surface, NULL));
+
+  /* 2. Get a GL texture */
+  node = gsk_color_node_new (&(GdkRGBA) { 1, 0, 0, 1 }, &GRAPHENE_RECT_INIT(0, 0, 1, 1));
+  texture = gsk_renderer_render_texture (gl_renderer, node, &GRAPHENE_RECT_INIT(0, 0, 1, 1));
+  gsk_render_node_unref (node);
+
+  /* 3. This is a bit fishy, but we want to make sure that
+   * the texture's GL context is current in the main thread.
+   *
+   * If we had access to the context, we'd make_current() here.
+   */
+  ensure_texture_access (texture);
+  g_assert_nonnull (gdk_gl_context_get_current ());
+
+  /* 4. Run a thread trying to download the texture */
+  loop = g_main_loop_new (NULL, TRUE);
+  task = g_task_new (texture, NULL, texture_download_done, loop);
+  g_task_run_in_thread (task, texture_download_thread);
+  g_clear_object (&task);
+
+  /* 5. Run the main loop waiting for the thread to return */
+  g_main_loop_run (loop);
+
+  /* 6. All good */
+  gsk_renderer_unrealize (gl_renderer);
+  g_clear_pointer (&loop, g_main_loop_unref);
+  g_clear_object (&gl_renderer);
+  g_clear_object (&surface);
+}
+
+int
+main (int argc, char *argv[])
+{
+  gtk_test_init (&argc, &argv, NULL);
+
+  g_test_add_func ("/texture-threads", texture_threads);
+
+  return g_test_run ();
+}