From c846f8d74565c2639dbebe1b223962f6e59fd228 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Tue, 26 Sep 2023 13:08:45 -0700 Subject: [PATCH 1/2] gsk/gl: use GdkArrayImpl for Clip tracking We can end up spending a lot of time in g_array_maybe_expand() through the use of g_array_set_size() for clip tracking. That is somewhat due to the simple nature of GArray being size-dynamic. Instead, we can use GdkArrayImpl and let the compiler do what it does best to elide some work and hoist other work into the calling function. This also fixes a potential UAF in gsk_gl_render_job_push_contained_clip(). --- gsk/gl/gskglrenderjob.c | 42 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/gsk/gl/gskglrenderjob.c b/gsk/gl/gskglrenderjob.c index 664b9a0738..1111bfb699 100644 --- a/gsk/gl/gskglrenderjob.c +++ b/gsk/gl/gskglrenderjob.c @@ -65,6 +65,14 @@ typedef struct _GskGLRenderClip guint is_fully_contained : 1; } GskGLRenderClip; +#define GDK_ARRAY_NAME clips +#define GDK_ARRAY_TYPE_NAME Clips +#define GDK_ARRAY_ELEMENT_TYPE GskGLRenderClip +#define GDK_ARRAY_BY_VALUE 1 +#define GDK_ARRAY_PREALLOC 16 +#define GDK_ARRAY_NO_MEMSET +#include "gdk/gdkarrayimpl.c" + typedef struct _GskGLRenderModelview { GskTransform *transform; @@ -122,7 +130,7 @@ struct _GskGLRenderJob /* An array of GskGLRenderClip updated as nodes are processed. The * current clip is the last element. */ - GArray *clip; + Clips clip; /* Our current alpha state as we process nodes */ float alpha; @@ -193,6 +201,14 @@ static gboolean gsk_gl_render_job_visit_node_with_offscreen (GskGLRenderJob const GskRenderNode *node, GskGLRenderOffscreen *offscreen); +static inline GskGLRenderClip * +clips_grow_one (Clips *clips) +{ + guint len = clips_get_size (clips); + clips_set_size (clips, len + 1); + return clips_get (clips, len); +} + static inline int get_target_format (GskGLRenderJob *job, const GskRenderNode *node) @@ -588,14 +604,12 @@ gsk_gl_render_job_push_clip (GskGLRenderJob *job, GskGLRenderClip *clip; g_assert (job != NULL); - g_assert (job->clip != NULL); g_assert (rect != NULL); job->driver->stamps[UNIFORM_SHARED_CLIP_RECT]++; - g_array_set_size (job->clip, job->clip->len + 1); + clip = clips_grow_one (&job->clip); - clip = &g_array_index (job->clip, GskGLRenderClip, job->clip->len - 1); memcpy (&clip->rect, rect, sizeof *rect); clip->is_rectilinear = gsk_rounded_rect_is_rectilinear (rect); clip->is_fully_contained = FALSE; @@ -610,16 +624,13 @@ gsk_gl_render_job_push_contained_clip (GskGLRenderJob *job) GskGLRenderClip *old_clip; g_assert (job != NULL); - g_assert (job->clip != NULL); - g_assert (job->clip->len > 0); + g_assert (clips_get_size (&job->clip) > 0); job->driver->stamps[UNIFORM_SHARED_CLIP_RECT]++; - old_clip = &g_array_index (job->clip, GskGLRenderClip, job->clip->len - 1); + clip = clips_grow_one (&job->clip); + old_clip = clips_get (&job->clip, clips_get_size (&job->clip) - 2); - g_array_set_size (job->clip, job->clip->len + 1); - - clip = &g_array_index (job->clip, GskGLRenderClip, job->clip->len - 1); memcpy (&clip->rect.bounds, &old_clip->rect.bounds, sizeof (graphene_rect_t)); memset (clip->rect.corner, 0, sizeof clip->rect.corner); clip->is_rectilinear = TRUE; @@ -632,12 +643,11 @@ static void gsk_gl_render_job_pop_clip (GskGLRenderJob *job) { g_assert (job != NULL); - g_assert (job->clip != NULL); - g_assert (job->clip->len > 0); + g_assert (clips_get_size (&job->clip) > 0); job->driver->stamps[UNIFORM_SHARED_CLIP_RECT]++; job->current_clip--; - job->clip->len--; + job->clip.end--; } static inline void @@ -1730,7 +1740,7 @@ gsk_gl_render_job_visit_rounded_clip_node (GskGLRenderJob *job, * which both have rounded corners. */ - if (job->clip->len <= 1) + if (clips_get_size (&job->clip) <= 1) need_offscreen = FALSE; else if (gsk_rounded_rect_contains_rect (&job->current_clip->rect, &transformed_clip.bounds)) need_offscreen = FALSE; @@ -4531,7 +4541,7 @@ gsk_gl_render_job_new (GskGLDriver *driver, job = g_new0 (GskGLRenderJob, 1); job->driver = g_object_ref (driver); job->command_queue = job->driver->command_queue; - job->clip = g_array_sized_new (FALSE, FALSE, sizeof (GskGLRenderClip), 16); + clips_init (&job->clip); job->modelview = g_array_sized_new (FALSE, FALSE, sizeof (GskGLRenderModelview), 16); job->framebuffer = framebuffer; job->clear_framebuffer = !!clear_framebuffer; @@ -4592,6 +4602,6 @@ gsk_gl_render_job_free (GskGLRenderJob *job) g_clear_object (&job->driver); g_clear_pointer (&job->region, cairo_region_destroy); g_clear_pointer (&job->modelview, g_array_unref); - g_clear_pointer (&job->clip, g_array_unref); + clips_clear (&job->clip); g_free (job); } From 089c34fa03e12df104b9418a781c064e9888e47c Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Tue, 26 Sep 2023 13:21:06 -0700 Subject: [PATCH 2/2] gsk/gl: use GdkArrayImpl for tracking modelview Like the previous change, this uses GdkArrayImpl instead of GArray for tracking modelview changes. This is less important than clip tracking simple due to being used less, but it keeps the implementation synchronous with the Clip tracking code. --- gsk/gl/gskglrenderjob.c | 61 +++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/gsk/gl/gskglrenderjob.c b/gsk/gl/gskglrenderjob.c index 1111bfb699..57d7976f5e 100644 --- a/gsk/gl/gskglrenderjob.c +++ b/gsk/gl/gskglrenderjob.c @@ -85,6 +85,14 @@ typedef struct _GskGLRenderModelview graphene_matrix_t matrix; } GskGLRenderModelview; +#define GDK_ARRAY_NAME modelviews +#define GDK_ARRAY_TYPE_NAME Modelviews +#define GDK_ARRAY_ELEMENT_TYPE GskGLRenderModelview +#define GDK_ARRAY_BY_VALUE 1 +#define GDK_ARRAY_PREALLOC 16 +#define GDK_ARRAY_NO_MEMSET +#include "gdk/gdkarrayimpl.c" + struct _GskGLRenderJob { /* The context containing the framebuffer we are drawing to. Generally this @@ -125,7 +133,7 @@ struct _GskGLRenderJob /* An array of GskGLRenderModelview updated as nodes are processed. The * current modelview is the last element. */ - GArray *modelview; + Modelviews modelview; /* An array of GskGLRenderClip updated as nodes are processed. The * current clip is the last element. @@ -209,6 +217,14 @@ clips_grow_one (Clips *clips) return clips_get (clips, len); } +static inline GskGLRenderModelview * +modelviews_grow_one (Modelviews *modelviews) +{ + guint len = modelviews_get_size (modelviews); + modelviews_set_size (modelviews, len + 1); + return modelviews_get (modelviews, len); +} + static inline int get_target_format (GskGLRenderJob *job, const GskRenderNode *node) @@ -482,15 +498,10 @@ gsk_gl_render_job_set_modelview (GskGLRenderJob *job, GskGLRenderModelview *modelview; g_assert (job != NULL); - g_assert (job->modelview != NULL); job->driver->stamps[UNIFORM_SHARED_MODELVIEW]++; - g_array_set_size (job->modelview, job->modelview->len + 1); - - modelview = &g_array_index (job->modelview, - GskGLRenderModelview, - job->modelview->len - 1); + modelview = modelviews_grow_one (&job->modelview); modelview->transform = transform; @@ -515,26 +526,17 @@ gsk_gl_render_job_push_modelview (GskGLRenderJob *job, GskGLRenderModelview *modelview; g_assert (job != NULL); - g_assert (job->modelview != NULL); g_assert (transform != NULL); job->driver->stamps[UNIFORM_SHARED_MODELVIEW]++; - g_array_set_size (job->modelview, job->modelview->len + 1); + modelview = modelviews_grow_one (&job->modelview); - modelview = &g_array_index (job->modelview, - GskGLRenderModelview, - job->modelview->len - 1); - - if G_LIKELY (job->modelview->len > 1) + if G_LIKELY (modelviews_get_size (&job->modelview) > 1) { - GskGLRenderModelview *last; + GskGLRenderModelview *last = job->modelview.end - 2; GskTransform *t = NULL; - last = &g_array_index (job->modelview, - GskGLRenderModelview, - job->modelview->len - 2); - /* Multiply given matrix with our previous modelview */ t = gsk_transform_translate (gsk_transform_ref (last->transform), &(graphene_point_t) { @@ -568,8 +570,7 @@ gsk_gl_render_job_pop_modelview (GskGLRenderJob *job) const GskGLRenderModelview *head; g_assert (job != NULL); - g_assert (job->modelview); - g_assert (job->modelview->len > 0); + g_assert (modelviews_get_size (&job->modelview) > 0); job->driver->stamps[UNIFORM_SHARED_MODELVIEW]++; @@ -580,11 +581,11 @@ gsk_gl_render_job_pop_modelview (GskGLRenderJob *job) gsk_transform_unref (head->transform); - job->modelview->len--; + job->modelview.end--; - if (job->modelview->len >= 1) + if (modelviews_get_size (&job->modelview) >= 1) { - head = &g_array_index (job->modelview, GskGLRenderModelview, job->modelview->len - 1); + head = job->modelview.end - 1; job->scale_x = head->scale_x; job->scale_y = head->scale_y; @@ -729,7 +730,7 @@ gsk_gl_render_job_transform_bounds (GskGLRenderJob *job, GskTransformCategory category; g_assert (job != NULL); - g_assert (job->modelview->len > 0); + g_assert (modelviews_get_size (&job->modelview) > 0); g_assert (rect != NULL); g_assert (out_rect != NULL); @@ -4542,7 +4543,7 @@ gsk_gl_render_job_new (GskGLDriver *driver, job->driver = g_object_ref (driver); job->command_queue = job->driver->command_queue; clips_init (&job->clip); - job->modelview = g_array_sized_new (FALSE, FALSE, sizeof (GskGLRenderModelview), 16); + modelviews_init (&job->modelview); job->framebuffer = framebuffer; job->clear_framebuffer = !!clear_framebuffer; job->default_framebuffer = default_framebuffer; @@ -4592,16 +4593,16 @@ gsk_gl_render_job_free (GskGLRenderJob *job) job->current_modelview = NULL; job->current_clip = NULL; - while (job->modelview->len > 0) + while (job->modelview.end > job->modelview.start) { - GskGLRenderModelview *modelview = &g_array_index (job->modelview, GskGLRenderModelview, job->modelview->len-1); + GskGLRenderModelview *modelview = job->modelview.end-1; g_clear_pointer (&modelview->transform, gsk_transform_unref); - job->modelview->len--; + job->modelview.end--; } g_clear_object (&job->driver); g_clear_pointer (&job->region, cairo_region_destroy); - g_clear_pointer (&job->modelview, g_array_unref); + modelviews_clear (&job->modelview); clips_clear (&job->clip); g_free (job); }