From 1223f8df6869c0b3b4429595095fd4d1c1804c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Mon, 15 Jul 2024 16:09:12 +0200 Subject: [PATCH] clutter/pipeline-cache: Return a copy of the pipeline This means the pipeline can be manipulated after retrieving. This also fixes a leak when adding pipelines to the cache, as we the pipeline would take a ref, but when adding, we wouldn't clean up our own ref. Part-of: --- clutter/clutter/clutter-pipeline-cache.c | 8 +- src/compositor/meta-shaped-texture.c | 4 +- src/tests/clutter/conform/meson.build | 1 + src/tests/clutter/conform/pipeline-cache.c | 126 ++++++++++++++------- 4 files changed, 93 insertions(+), 46 deletions(-) diff --git a/clutter/clutter/clutter-pipeline-cache.c b/clutter/clutter/clutter-pipeline-cache.c index 74dc70c58..a5a6a67d5 100644 --- a/clutter/clutter/clutter-pipeline-cache.c +++ b/clutter/clutter/clutter-pipeline-cache.c @@ -122,6 +122,7 @@ clutter_pipeline_cache_get_pipeline (ClutterPipelineCache *pipeline_cache, { PipelineGroupEntry *group_entry; uint64_t key; + CoglPipeline *pipeline; group_entry = g_hash_table_lookup (pipeline_cache->groups, group); if (!group_entry) @@ -134,7 +135,12 @@ clutter_pipeline_cache_get_pipeline (ClutterPipelineCache *pipeline_cache, return NULL; key = calculate_key (source_color_state, target_color_state); - return g_hash_table_lookup (group_entry->slots[slot], &key); + pipeline = g_hash_table_lookup (group_entry->slots[slot], &key); + + if (pipeline) + return cogl_pipeline_copy (pipeline); + else + return NULL; } /** diff --git a/src/compositor/meta-shaped-texture.c b/src/compositor/meta-shaped-texture.c index 110cafcdd..854fc6e52 100644 --- a/src/compositor/meta-shaped-texture.c +++ b/src/compositor/meta-shaped-texture.c @@ -961,7 +961,7 @@ do_paint_content (MetaShapedTexture *stex, if (!mtk_region_is_empty (region)) { - CoglPipeline *opaque_pipeline; + g_autoptr (CoglPipeline) opaque_pipeline = NULL; opaque_pipeline = get_unblended_pipeline (stex, paint_context, paint_tex); @@ -1008,7 +1008,7 @@ do_paint_content (MetaShapedTexture *stex, */ if (!blended_tex_region || !mtk_region_is_empty (blended_tex_region)) { - CoglPipeline *blended_pipeline; + g_autoptr (CoglPipeline) blended_pipeline = NULL; CoglColor color; int i; diff --git a/src/tests/clutter/conform/meson.build b/src/tests/clutter/conform/meson.build index adcd3b095..2cfe074b4 100644 --- a/src/tests/clutter/conform/meson.build +++ b/src/tests/clutter/conform/meson.build @@ -1,5 +1,6 @@ clutter_tests_conform_c_args = [ '-DG_LOG_DOMAIN="Clutter-Conform"', + '-DCOGL_ENABLE_MUTTER_API', '-DGETTEXT_PACKAGE="@0@"'.format(meson.project_name()), ] clutter_tests_conform_c_args += clutter_debug_c_args diff --git a/src/tests/clutter/conform/pipeline-cache.c b/src/tests/clutter/conform/pipeline-cache.c index 19492bb3a..44680e648 100644 --- a/src/tests/clutter/conform/pipeline-cache.c +++ b/src/tests/clutter/conform/pipeline-cache.c @@ -4,6 +4,28 @@ #include #include "tests/clutter-test-utils.h" +#include "cogl/cogl-mutter.h" + +static void +assert_match_and_unref (CoglPipeline *pipeline, + CoglPipeline *expected_pipeline) +{ + g_assert_cmpstr (cogl_pipeline_get_name (pipeline), + ==, + cogl_pipeline_get_name (expected_pipeline)); + g_object_unref (pipeline); +} + +static CoglPipeline * +create_test_pipeline (CoglContext *context, + const char *name) +{ + CoglPipeline *pipeline; + + pipeline = cogl_pipeline_new (context); + cogl_pipeline_set_static_name (pipeline, name); + return pipeline; +} static void pipeline_cache_group_pipelines (void) @@ -24,8 +46,8 @@ pipeline_cache_group_pipelines (void) /* HDR content with HDR output */ CoglPipeline *bt2020_pq_to_bt2020_linear; CoglPipeline *srgb_linear_to_srgb_srgb; - /* Copy for group2 */ - CoglPipeline *srgb_srgb_to_bt2020_linear_copy; + /* Second pipeline for group2 */ + CoglPipeline *srgb_srgb_to_bt2020_linear_2; srgb_srgb = clutter_color_state_new (context, CLUTTER_COLORSPACE_SRGB, @@ -40,10 +62,14 @@ pipeline_cache_group_pipelines (void) CLUTTER_COLORSPACE_BT2020, CLUTTER_TRANSFER_FUNCTION_LINEAR); - srgb_srgb_to_bt2020_linear = cogl_pipeline_new (cogl_context); - bt2020_linear_to_bt2020_pq = cogl_pipeline_new (cogl_context); - bt2020_pq_to_bt2020_linear = cogl_pipeline_new (cogl_context); - srgb_linear_to_srgb_srgb = cogl_pipeline_new (cogl_context); + srgb_srgb_to_bt2020_linear = create_test_pipeline (cogl_context, + "srgb_srgb_to_bt2020_linear"); + bt2020_linear_to_bt2020_pq = create_test_pipeline (cogl_context, + "bt2020_linear_to_bt2020_pq"); + bt2020_pq_to_bt2020_linear = create_test_pipeline (cogl_context, + "bt2020_pq_to_bt2020_linear"); + srgb_linear_to_srgb_srgb = create_test_pipeline (cogl_context, + "srgb_linear_to_srgb_srgb"); clutter_color_state_add_pipeline_transform (srgb_srgb, bt2020_linear, @@ -76,31 +102,37 @@ pipeline_cache_group_pipelines (void) bt2020_linear, bt2020_pq, bt2020_linear_to_bt2020_pq); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group1, 0, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group1, 0, - bt2020_linear, bt2020_pq) == - bt2020_linear_to_bt2020_pq); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group1, 0, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group1, 0, + bt2020_linear, + bt2020_pq), + bt2020_linear_to_bt2020_pq); g_assert_null (clutter_pipeline_cache_get_pipeline (pipeline_cache, group2, 0, srgb_srgb, bt2020_linear)); g_assert_null (clutter_pipeline_cache_get_pipeline (pipeline_cache, group2, 0, bt2020_linear, bt2020_pq)); - srgb_srgb_to_bt2020_linear_copy = - cogl_pipeline_copy (srgb_srgb_to_bt2020_linear); - g_assert_true (srgb_srgb_to_bt2020_linear_copy != - srgb_srgb_to_bt2020_linear); + srgb_srgb_to_bt2020_linear_2 = + create_test_pipeline (cogl_context, "srgb_srgb_to_bt2020_linear_2"); clutter_pipeline_cache_set_pipeline (pipeline_cache, group2, 0, srgb_srgb, bt2020_linear, - srgb_srgb_to_bt2020_linear_copy); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group1, 0, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group2, 0, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear_copy); + srgb_srgb_to_bt2020_linear_2); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group1, 0, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group2, 0, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear_2); } static void @@ -114,7 +146,7 @@ pipeline_cache_replace_pipeline (void) ClutterColorState *srgb_srgb; ClutterColorState *bt2020_linear; CoglPipeline *srgb_srgb_to_bt2020_linear; - CoglPipeline *srgb_srgb_to_bt2020_linear_copy; + CoglPipeline *srgb_srgb_to_bt2020_linear_2; srgb_srgb = clutter_color_state_new (context, CLUTTER_COLORSPACE_SRGB, @@ -123,9 +155,10 @@ pipeline_cache_replace_pipeline (void) CLUTTER_COLORSPACE_BT2020, CLUTTER_TRANSFER_FUNCTION_PQ); - srgb_srgb_to_bt2020_linear = cogl_pipeline_new (cogl_context); - srgb_srgb_to_bt2020_linear_copy = - cogl_pipeline_copy (srgb_srgb_to_bt2020_linear); + srgb_srgb_to_bt2020_linear = create_test_pipeline (cogl_context, + "srgb_srgb_to_bt2020_linear"); + srgb_srgb_to_bt2020_linear_2 = + create_test_pipeline (cogl_context, "srgb_srgb_to_bt2020_linear_2"); g_object_add_weak_pointer (G_OBJECT (srgb_srgb_to_bt2020_linear), (gpointer *) &srgb_srgb_to_bt2020_linear); @@ -143,15 +176,17 @@ pipeline_cache_replace_pipeline (void) clutter_color_state_add_pipeline_transform (srgb_srgb, bt2020_linear, - srgb_srgb_to_bt2020_linear_copy); + srgb_srgb_to_bt2020_linear_2); clutter_pipeline_cache_set_pipeline (pipeline_cache, group, 0, srgb_srgb, bt2020_linear, - srgb_srgb_to_bt2020_linear_copy); + srgb_srgb_to_bt2020_linear_2); g_assert_null (srgb_srgb_to_bt2020_linear); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group, 0, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear_copy); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group, 0, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear_2); } static void @@ -165,7 +200,7 @@ pipeline_slots (void) ClutterColorState *srgb_srgb; ClutterColorState *bt2020_linear; CoglPipeline *srgb_srgb_to_bt2020_linear; - CoglPipeline *srgb_srgb_to_bt2020_linear_copy; + CoglPipeline *srgb_srgb_to_bt2020_linear_2; srgb_srgb = clutter_color_state_new (context, CLUTTER_COLORSPACE_SRGB, @@ -174,23 +209,28 @@ pipeline_slots (void) CLUTTER_COLORSPACE_BT2020, CLUTTER_TRANSFER_FUNCTION_PQ); - srgb_srgb_to_bt2020_linear = cogl_pipeline_new (cogl_context); - srgb_srgb_to_bt2020_linear_copy = - cogl_pipeline_copy (srgb_srgb_to_bt2020_linear); + srgb_srgb_to_bt2020_linear = create_test_pipeline (cogl_context, + "srgb_srgb_to_bt2020_linear"); + srgb_srgb_to_bt2020_linear_2 = + create_test_pipeline (cogl_context, "srgb_srgb_to_bt2020_linear_2 "); clutter_pipeline_cache_set_pipeline (pipeline_cache, group, 0, srgb_srgb, bt2020_linear, srgb_srgb_to_bt2020_linear); clutter_pipeline_cache_set_pipeline (pipeline_cache, group, 1, srgb_srgb, bt2020_linear, - srgb_srgb_to_bt2020_linear_copy); + srgb_srgb_to_bt2020_linear_2); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group, 0, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear); - g_assert_true (clutter_pipeline_cache_get_pipeline (pipeline_cache, group, 1, - srgb_srgb, bt2020_linear) == - srgb_srgb_to_bt2020_linear_copy); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group, 0, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear); + assert_match_and_unref (clutter_pipeline_cache_get_pipeline (pipeline_cache, + group, 1, + srgb_srgb, + bt2020_linear), + srgb_srgb_to_bt2020_linear_2); } CLUTTER_TEST_SUITE (