From 24dc9dc6531392bd47d71edec88cf1d1f4f4492f Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 15 Sep 2024 11:26:20 +0200 Subject: [PATCH 1/3] Drop the dmabuf_downloaders array Just use two individual fields, so we can track if we've already created each one. This also matches the individual fields we have for the dmabuf formats. And change preference order of downloaders Previously, our order was mmap > vulkan > egl. But depending on the hw (discrete vs integrated gpu), mmap can be catastrophically slower (on the order of 20ms vs 1.5s). So, change the order to egl > vulkan > mmap. Note that this currently has less effect than we'd like to, since we don't let the downloaders claim linear formats. --- gdk/gdkdisplay.c | 41 ++++++++++------------------------------- gdk/gdkdisplayprivate.h | 3 ++- gdk/gdkdmabuftexture.c | 32 +++++++++++++++----------------- 3 files changed, 27 insertions(+), 49 deletions(-) diff --git a/gdk/gdkdisplay.c b/gdk/gdkdisplay.c index 7becc7bf3c..01241d56a1 100644 --- a/gdk/gdkdisplay.c +++ b/gdk/gdkdisplay.c @@ -418,15 +418,17 @@ gdk_display_dispose (GObject *object) { GdkDisplay *display = GDK_DISPLAY (object); GdkDisplayPrivate *priv = gdk_display_get_instance_private (display); - gsize i; - for (i = 0; i < G_N_ELEMENTS (display->dmabuf_downloaders); i++) + if (display->vk_downloader) { - if (display->dmabuf_downloaders[i] == NULL) - continue; + gdk_dmabuf_downloader_close (display->vk_downloader); + g_clear_object (&display->vk_downloader); + } - gdk_dmabuf_downloader_close (display->dmabuf_downloaders[i]); - g_clear_object (&display->dmabuf_downloaders[i]); + if (display->egl_downloader) + { + gdk_dmabuf_downloader_close (display->egl_downloader); + g_clear_object (&display->egl_downloader); } _gdk_display_manager_remove_display (gdk_display_manager_get (), display); @@ -1965,29 +1967,6 @@ gdk_display_get_egl_display (GdkDisplay *self) #endif } -#ifdef HAVE_DMABUF -static void -gdk_display_add_dmabuf_downloader (GdkDisplay *display, - GdkDmabufDownloader *downloader) -{ - gsize i; - - if (downloader == NULL) - return; - - /* dmabuf_downloaders is NULL-terminated */ - for (i = 0; i < G_N_ELEMENTS (display->dmabuf_downloaders) - 1; i++) - { - if (display->dmabuf_downloaders[i] == NULL) - break; - } - - g_assert (i < G_N_ELEMENTS (display->dmabuf_downloaders) - 1); - - display->dmabuf_downloaders[i] = downloader; -} -#endif - /* To support a drm format, we must be able to import it into GL * using the relevant EGL extensions, and download it into a memory * texture, possibly doing format conversion with shaders (in GSK). @@ -2009,11 +1988,11 @@ gdk_display_init_dmabuf (GdkDisplay *self) if (gdk_has_feature (GDK_FEATURE_DMABUF)) { #ifdef GDK_RENDERING_VULKAN - gdk_display_add_dmabuf_downloader (self, gdk_vulkan_get_dmabuf_downloader (self, builder)); + self->vk_downloader = gdk_vulkan_get_dmabuf_downloader (self, builder); #endif #ifdef HAVE_EGL - gdk_display_add_dmabuf_downloader (self, gdk_dmabuf_get_egl_downloader (self, builder)); + self->egl_downloader = gdk_dmabuf_get_egl_downloader (self, builder); #endif gdk_dmabuf_formats_builder_add_formats (builder, diff --git a/gdk/gdkdisplayprivate.h b/gdk/gdkdisplayprivate.h index 21848ae4db..9420b1e182 100644 --- a/gdk/gdkdisplayprivate.h +++ b/gdk/gdkdisplayprivate.h @@ -132,7 +132,8 @@ struct _GdkDisplay guint have_egl_gl_colorspace : 1; GdkDmabufFormats *dmabuf_formats; - GdkDmabufDownloader *dmabuf_downloaders[4]; + GdkDmabufDownloader *egl_downloader; + GdkDmabufDownloader *vk_downloader; /* Cached data the EGL dmabuf downloader */ GdkDmabufFormats *egl_dmabuf_formats; diff --git a/gdk/gdkdmabuftexture.c b/gdk/gdkdmabuftexture.c index 3f1960d56b..e68341db71 100644 --- a/gdk/gdkdmabuftexture.c +++ b/gdk/gdkdmabuftexture.c @@ -183,10 +183,8 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder, GdkDisplay *display; GdkDmabuf dmabuf; GdkColorState *color_state; - GError *local_error = NULL; int width, height; gboolean premultiplied; - gsize i; display = gdk_dmabuf_texture_builder_get_display (builder); width = gdk_dmabuf_texture_builder_get_width (builder); @@ -234,28 +232,28 @@ gdk_dmabuf_texture_new_from_builder (GdkDmabufTextureBuilder *builder, : GDK_MEMORY_R8G8B8A8; } - if (!gdk_dmabuf_formats_contains (gdk_dmabuf_get_mmap_formats (), dmabuf.fourcc, dmabuf.modifier)) + if (display->egl_downloader) { - 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)) - g_clear_error (&local_error); + if (gdk_dmabuf_downloader_supports (display->egl_downloader, self, error)) + self->downloader = g_object_ref (display->egl_downloader); + } - if (gdk_dmabuf_downloader_supports (display->dmabuf_downloaders[i], - self, - local_error ? NULL : &local_error)) - { - self->downloader = g_object_ref (display->dmabuf_downloaders[i]); - break; - } - } + if (!self->downloader && display->vk_downloader) + { + g_clear_error (error); + if (gdk_dmabuf_downloader_supports (display->vk_downloader, self, error)) + self->downloader = g_object_ref (display->vk_downloader); + } - if (self->downloader == NULL) + if (!self->downloader) + { + if (!gdk_dmabuf_formats_contains (gdk_dmabuf_get_mmap_formats (), dmabuf.fourcc, dmabuf.modifier)) { - g_propagate_error (error, local_error); g_object_unref (self); return NULL; } + + g_clear_error (error); } GDK_DISPLAY_DEBUG (display, DMABUF, From 6059eaf355f518b19d63fb23f25aa2344a3f1e53 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Sun, 15 Sep 2024 11:38:30 +0200 Subject: [PATCH 2/3] gdk: Reorganize dmabuf initialization Change the xxx_get_downloader() functions to init_dmabuf_xxx(), and make them initialize both xxx_dmabuf_formats and xxx_downloader. --- gdk/gdkdisplay.c | 15 ++-- gdk/gdkdmabufegl.c | 150 ++++++++++++++++++---------------- gdk/gdkdmabufeglprivate.h | 6 +- gdk/gdkvulkancontext.c | 37 ++++----- gdk/gdkvulkancontextprivate.h | 5 +- 5 files changed, 106 insertions(+), 107 deletions(-) diff --git a/gdk/gdkdisplay.c b/gdk/gdkdisplay.c index 01241d56a1..905b685a49 100644 --- a/gdk/gdkdisplay.c +++ b/gdk/gdkdisplay.c @@ -1967,10 +1967,6 @@ gdk_display_get_egl_display (GdkDisplay *self) #endif } -/* To support a drm format, we must be able to import it into GL - * using the relevant EGL extensions, and download it into a memory - * texture, possibly doing format conversion with shaders (in GSK). - */ void gdk_display_init_dmabuf (GdkDisplay *self) { @@ -1988,15 +1984,18 @@ gdk_display_init_dmabuf (GdkDisplay *self) if (gdk_has_feature (GDK_FEATURE_DMABUF)) { #ifdef GDK_RENDERING_VULKAN - self->vk_downloader = gdk_vulkan_get_dmabuf_downloader (self, builder); + gdk_vulkan_init_dmabuf (self); + if (self->vk_dmabuf_formats) + gdk_dmabuf_formats_builder_add_formats (builder, self->vk_dmabuf_formats); #endif #ifdef HAVE_EGL - self->egl_downloader = gdk_dmabuf_get_egl_downloader (self, builder); + gdk_dmabuf_egl_init (self); + if (self->egl_dmabuf_formats) + gdk_dmabuf_formats_builder_add_formats (builder, self->egl_dmabuf_formats); #endif - gdk_dmabuf_formats_builder_add_formats (builder, - gdk_dmabuf_get_mmap_formats ()); + gdk_dmabuf_formats_builder_add_formats (builder, gdk_dmabuf_get_mmap_formats ()); } #endif diff --git a/gdk/gdkdmabufegl.c b/gdk/gdkdmabufegl.c index 96fe6b951e..f6358c04cd 100644 --- a/gdk/gdkdmabufegl.c +++ b/gdk/gdkdmabufegl.c @@ -18,7 +18,6 @@ #include "config.h" -#if defined(HAVE_DMABUF) && defined (HAVE_EGL) #include "gdkdmabufeglprivate.h" #include "gdkdmabufformatsbuilderprivate.h" @@ -32,6 +31,8 @@ #include +#if defined(HAVE_DMABUF) && defined (HAVE_EGL) + /* A dmabuf downloader implementation that downloads buffers via * gsk_renderer_render_texture + GL texture download. */ @@ -133,77 +134,6 @@ gdk_dmabuf_egl_downloader_collect_formats (GdkDisplay *display, return TRUE; } -/* Hack. We don't include gsk/gsk.h here to avoid a build order problem - * with the generated header gskenumtypes.h, so we need to hack around - * a bit to access the gsk api we need. - */ - -typedef struct _GskRenderer GskRenderer; - -extern GskRenderer * gsk_gl_renderer_new (void); -extern gboolean gsk_renderer_realize_for_display (GskRenderer *renderer, - GdkDisplay *display, - GError **error); - -GdkDmabufDownloader * -gdk_dmabuf_get_egl_downloader (GdkDisplay *display, - GdkDmabufFormatsBuilder *builder) -{ - GdkDmabufFormatsBuilder *formats; - GdkDmabufFormatsBuilder *external; - gboolean retval = FALSE; - GError *error = NULL; - GskRenderer *renderer; - GdkGLContext *previous; - - g_assert (display->egl_dmabuf_formats == NULL); - g_assert (display->egl_external_formats == NULL); - - if (!gdk_display_prepare_gl (display, NULL)) - return NULL; - - previous = gdk_gl_context_get_current (); - if (previous) - g_object_ref (previous); - formats = gdk_dmabuf_formats_builder_new (); - external = gdk_dmabuf_formats_builder_new (); - - retval = gdk_dmabuf_egl_downloader_collect_formats (display, formats, external); - - display->egl_dmabuf_formats = gdk_dmabuf_formats_builder_free_to_formats (formats); - display->egl_external_formats = gdk_dmabuf_formats_builder_free_to_formats (external); - - gdk_dmabuf_formats_builder_add_formats (builder, display->egl_dmabuf_formats); - - if (!retval) - { - if (previous) - gdk_gl_context_make_current (previous); - return NULL; - } - - renderer = gsk_gl_renderer_new (); - - if (!gsk_renderer_realize_for_display (renderer, display, &error)) - { - g_warning ("Failed to realize GL renderer: %s", error->message); - g_error_free (error); - g_object_unref (renderer); - if (previous) - gdk_gl_context_make_current (previous); - - return NULL; - } - - if (previous) - { - gdk_gl_context_make_current (previous); - g_object_unref (previous); - } - - return GDK_DMABUF_DOWNLOADER (renderer); -} - EGLImage gdk_dmabuf_egl_create_image (GdkDisplay *display, int width, @@ -290,3 +220,79 @@ gdk_dmabuf_egl_create_image (GdkDisplay *display, } #endif /* HAVE_DMABUF && HAVE_EGL */ + +/* Hack. We don't include gsk/gsk.h here to avoid a build order problem + * with the generated header gskenumtypes.h, so we need to hack around + * a bit to access the gsk api we need. + */ + +typedef struct _GskRenderer GskRenderer; + +extern GskRenderer * gsk_gl_renderer_new (void); +extern gboolean gsk_renderer_realize_for_display (GskRenderer *renderer, + GdkDisplay *display, + GError **error); + +void +gdk_dmabuf_egl_init (GdkDisplay *display) +{ +#if defined (HAVE_DMABUF) && defined (HAVE_EGL) + GdkDmabufFormatsBuilder *formats; + GdkDmabufFormatsBuilder *external; + gboolean retval = FALSE; + GError *error = NULL; + GskRenderer *renderer; + GdkGLContext *previous; + + if (display->egl_dmabuf_formats != NULL) + return; + + if (!gdk_has_feature (GDK_FEATURE_DMABUF) || + !gdk_display_prepare_gl (display, NULL)) + { + return; + } + + formats = gdk_dmabuf_formats_builder_new (); + external = gdk_dmabuf_formats_builder_new (); + + previous = gdk_gl_context_get_current (); + if (previous) + g_object_ref (previous); + + retval = gdk_dmabuf_egl_downloader_collect_formats (display, formats, external); + + display->egl_dmabuf_formats = gdk_dmabuf_formats_builder_free_to_formats (formats); + display->egl_external_formats = gdk_dmabuf_formats_builder_free_to_formats (external); + + if (!retval) + { + if (previous) + gdk_gl_context_make_current (previous); + return; + } + + renderer = gsk_gl_renderer_new (); + + if (!gsk_renderer_realize_for_display (renderer, display, &error)) + { + g_warning ("Failed to realize GL renderer: %s", error->message); + g_error_free (error); + g_object_unref (renderer); + if (previous) + gdk_gl_context_make_current (previous); + + return; + } + + if (previous) + { + gdk_gl_context_make_current (previous); + g_object_unref (previous); + } + + display->egl_downloader = GDK_DMABUF_DOWNLOADER (renderer); + +#endif +} + diff --git a/gdk/gdkdmabufeglprivate.h b/gdk/gdkdmabufeglprivate.h index 4ff1a0f614..b860d0dae4 100644 --- a/gdk/gdkdmabufeglprivate.h +++ b/gdk/gdkdmabufeglprivate.h @@ -1,14 +1,12 @@ #pragma once +#include "gdkdisplayprivate.h" #if defined(HAVE_DMABUF) && defined (HAVE_EGL) #include "gdkdmabufprivate.h" -#include "gdkdmabufdownloaderprivate.h" #include -GdkDmabufDownloader * gdk_dmabuf_get_egl_downloader (GdkDisplay *display, - GdkDmabufFormatsBuilder *builder); EGLImage gdk_dmabuf_egl_create_image (GdkDisplay *display, int width, int height, @@ -16,3 +14,5 @@ EGLImage gdk_dmabuf_egl_create_image (GdkDisplay int target); #endif /* HAVE_DMABUF && HAVE_EGL */ + +void gdk_dmabuf_egl_init (GdkDisplay *display); diff --git a/gdk/gdkvulkancontext.c b/gdk/gdkvulkancontext.c index a424d36f17..45326195a4 100644 --- a/gdk/gdkvulkancontext.c +++ b/gdk/gdkvulkancontext.c @@ -1888,8 +1888,6 @@ gdk_display_unref_vulkan (GdkDisplay *display) display->vk_instance = VK_NULL_HANDLE; } -#ifdef HAVE_DMABUF - /* Hack. We don't include gsk/gsk.h here to avoid a build order problem * with the generated header gskenumtypes.h, so we need to hack around * a bit to access the gsk api we need. @@ -1902,10 +1900,10 @@ extern gboolean gsk_renderer_realize_for_display (GskRenderer *r GdkDisplay *display, GError **error); -GdkDmabufDownloader * -gdk_vulkan_get_dmabuf_downloader (GdkDisplay *display, - GdkDmabufFormatsBuilder *builder) +void +gdk_vulkan_init_dmabuf (GdkDisplay *display) { +#ifdef HAVE_DMABUF GdkDmabufFormatsBuilder *vulkan_builder; GskRenderer *renderer; VkDrmFormatModifierPropertiesEXT modifier_list[100]; @@ -1923,13 +1921,15 @@ gdk_vulkan_get_dmabuf_downloader (GdkDisplay *display, GError *error = NULL; gsize i, j; - g_assert (display->vk_dmabuf_formats == NULL); + if (display->vk_dmabuf_formats != NULL) + return; - if (!gdk_display_init_vulkan (display, NULL)) - return NULL; - - if ((display->vulkan_features & GDK_VULKAN_FEATURE_DMABUF) == 0) - return NULL; + if (!gdk_has_feature (GDK_FEATURE_DMABUF) || + !gdk_display_init_vulkan (display, NULL) || + ((display->vulkan_features & GDK_VULKAN_FEATURE_DMABUF) == 0)) + { + return; + } vulkan_builder = gdk_dmabuf_formats_builder_new (); @@ -1963,23 +1963,20 @@ gdk_vulkan_get_dmabuf_downloader (GdkDisplay *display, display->vk_dmabuf_formats = gdk_dmabuf_formats_builder_free_to_formats (vulkan_builder); - gdk_dmabuf_formats_builder_add_formats (builder, display->vk_dmabuf_formats); - renderer = gsk_vulkan_renderer_new (); if (!gsk_renderer_realize_for_display (renderer, display, &error)) { - g_warning ("Failed to realize GL renderer: %s", error->message); + g_warning ("Failed to realize Vulkan renderer: %s", error->message); g_error_free (error); g_object_unref (renderer); - - return NULL; } - - return GDK_DMABUF_DOWNLOADER (renderer); -} - + else + { + display->vk_downloader = GDK_DMABUF_DOWNLOADER (renderer); + } #endif +} VkShaderModule gdk_display_get_vk_shader_module (GdkDisplay *self, diff --git a/gdk/gdkvulkancontextprivate.h b/gdk/gdkvulkancontextprivate.h index eb85981ac4..f39e5c22e2 100644 --- a/gdk/gdkvulkancontextprivate.h +++ b/gdk/gdkvulkancontextprivate.h @@ -78,10 +78,7 @@ gboolean gdk_display_init_vulkan (GdkDisp void gdk_display_ref_vulkan (GdkDisplay *display); void gdk_display_unref_vulkan (GdkDisplay *display); -#ifdef HAVE_DMABUF -GdkDmabufDownloader * gdk_vulkan_get_dmabuf_downloader (GdkDisplay *display, - GdkDmabufFormatsBuilder *builder); -#endif +void gdk_vulkan_init_dmabuf (GdkDisplay *display); VkShaderModule gdk_display_get_vk_shader_module (GdkDisplay *display, const char *resource_name); From 7acc1c01253798210ee9f4ba85b075d874afbb91 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mon, 16 Sep 2024 09:54:19 +0200 Subject: [PATCH 3/3] Make dmabuf initialization lazier Only initialize the Vulkan or EGL parts where possible. When dmabufs or dmabuf formats are actually used, we still initialize fully by creating both a Vulkan and EGL downloader. This shortens the time to first commit from 149ms to 108ms. --- gdk/gdkglcontext.c | 12 +++++++----- gsk/gpu/gsknglrenderer.c | 3 +++ gsk/gpu/gskvulkanrenderer.c | 7 +++++++ gsk/gskrenderer.c | 4 +++- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/gdk/gdkglcontext.c b/gdk/gdkglcontext.c index f2a8be1325..7a43d8c840 100644 --- a/gdk/gdkglcontext.c +++ b/gdk/gdkglcontext.c @@ -2217,6 +2217,7 @@ gdk_gl_backend_use (GdkGLBackend backend_type) g_assert (the_gl_backend_type == backend_type); } +#if defined(HAVE_EGL) && defined(HAVE_DMABUF) static guint gdk_gl_context_import_dmabuf_for_target (GdkGLContext *self, int width, @@ -2224,7 +2225,6 @@ gdk_gl_context_import_dmabuf_for_target (GdkGLContext *self, const GdkDmabuf *dmabuf, int target) { -#if defined(HAVE_EGL) && defined(HAVE_DMABUF) GdkDisplay *display = gdk_gl_context_get_display (self); EGLImage image; guint texture_id; @@ -2246,10 +2246,8 @@ gdk_gl_context_import_dmabuf_for_target (GdkGLContext *self, eglDestroyImageKHR (gdk_display_get_egl_display (display), image); return texture_id; -#else - return 0; -#endif } +#endif guint gdk_gl_context_import_dmabuf (GdkGLContext *self, @@ -2258,10 +2256,11 @@ gdk_gl_context_import_dmabuf (GdkGLContext *self, const GdkDmabuf *dmabuf, gboolean *external) { +#if defined(HAVE_EGL) && defined(HAVE_DMABUF) GdkDisplay *display = gdk_gl_context_get_display (self); guint texture_id; - gdk_display_init_dmabuf (display); + gdk_dmabuf_egl_init (display); if (gdk_dmabuf_formats_contains (display->egl_dmabuf_formats, dmabuf->fourcc, dmabuf->modifier)) { @@ -2352,6 +2351,9 @@ gdk_gl_context_import_dmabuf (GdkGLContext *self, *external = target == GL_TEXTURE_EXTERNAL_OES; return texture_id; } +#else + return 0; +#endif } gboolean diff --git a/gsk/gpu/gsknglrenderer.c b/gsk/gpu/gsknglrenderer.c index 4d23a67cfa..511dcc2c52 100644 --- a/gsk/gpu/gsknglrenderer.c +++ b/gsk/gpu/gsknglrenderer.c @@ -9,6 +9,7 @@ #include "gskglimageprivate.h" #include "gdk/gdkdisplayprivate.h" +#include "gdk/gdkdmabufeglprivate.h" #include "gdk/gdkglcontextprivate.h" #include @@ -129,6 +130,8 @@ gsk_ngl_renderer_get_dmabuf_formats (GskGpuRenderer *renderer) { GdkDisplay *display = GDK_DISPLAY (gdk_draw_context_get_display (gsk_gpu_renderer_get_context (renderer))); + gdk_dmabuf_egl_init (display); + return display->egl_dmabuf_formats; } diff --git a/gsk/gpu/gskvulkanrenderer.c b/gsk/gpu/gskvulkanrenderer.c index e22ba2565d..8e61777eab 100644 --- a/gsk/gpu/gskvulkanrenderer.c +++ b/gsk/gpu/gskvulkanrenderer.c @@ -10,6 +10,7 @@ #include "gskvulkanframeprivate.h" #include "gskvulkanimageprivate.h" +#include "gdk/gdkvulkancontextprivate.h" #include "gdk/gdkdisplayprivate.h" #endif @@ -124,9 +125,15 @@ gsk_vulkan_renderer_get_backbuffer (GskGpuRenderer *renderer) static GdkDmabufFormats * gsk_vulkan_renderer_get_dmabuf_formats (GskGpuRenderer *renderer) { +#ifdef HAVE_DMABUF GdkDisplay *display = GDK_DISPLAY (gdk_draw_context_get_display (gsk_gpu_renderer_get_context (renderer))); + gdk_vulkan_init_dmabuf (display); + return display->vk_dmabuf_formats; +#else + return NULL; +#endif } static void diff --git a/gsk/gskrenderer.c b/gsk/gskrenderer.c index 8260727dd8..a9d5fdf7f8 100644 --- a/gsk/gskrenderer.c +++ b/gsk/gskrenderer.c @@ -683,7 +683,8 @@ vulkan_supported_platform (GdkSurface *surface, return FALSE; } - gdk_display_init_dmabuf (display); +#ifdef HAVE_DMABUF + gdk_vulkan_init_dmabuf (display); if (!display->vk_dmabuf_formats || gdk_dmabuf_formats_get_n_formats (display->vk_dmabuf_formats) == 0) { @@ -691,6 +692,7 @@ vulkan_supported_platform (GdkSurface *surface, GSK_DEBUG (RENDERER, "Not using '%s': no dmabuf support", g_type_name (renderer_type)); return FALSE; } +#endif if (as_fallback) return TRUE;