[Cogl] [PATCH 1/2] Add test to verify replacing a layer doesn't leak the pipeline parent
Robert Bragg
robert at sixbynine.org
Tue Mar 19 10:47:18 PDT 2013
This looks good to land to me:
Reviewed-by: Robert Bragg <robert at linux.intel.com>
thanks,
- Robert
On Sat, Mar 9, 2013 at 9:05 PM, Neil Roberts <neil at linux.intel.com> wrote:
> The current recommendation for pipelines is that once they have been
> used for painting then they should be considered immutable. If you
> want to modify a pipeline you should instead make a copy and unref the
> original pipeline. Internally we try to check whether the modified
> copy replaces all of the properties of the parent and prune a
> redundant ancestor hierarchy. Pruning the hierarchy is particularly
> important if the pipelines contain textures because otherwise the
> textures may be leaked when the parent pipeline keeps a reference to
> it.
>
> This test verifies that usage pattern by creating a chain of pipeline
> copies each with their own replacement texture. Some user data is then
> set on the textures with a callback so that we can verify that once
> the original pipelines are destroyed then the textures are also
> destroyed.
>
> The test is currently failing because Cogl doesn't correctly prune
> ancestory for layer state authority.
> ---
> tests/conform/Makefile.am | 1 +
> tests/conform/test-conform-main.c | 2 +
> tests/conform/test-copy-replace-texture.c | 121 ++++++++++++++++++++++++++++++
> 3 files changed, 124 insertions(+)
> create mode 100644 tests/conform/test-copy-replace-texture.c
>
> diff --git a/tests/conform/Makefile.am b/tests/conform/Makefile.am
> index 7ccb028..6650766 100644
> --- a/tests/conform/Makefile.am
> +++ b/tests/conform/Makefile.am
> @@ -63,6 +63,7 @@ test_sources = \
> test-texture-mipmap-get-set.c \
> test-framebuffer-get-bits.c \
> test-primitive-and-journal.c \
> + test-copy-replace-texture.c \
> $(NULL)
>
> test_conformance_SOURCES = $(common_sources) $(test_sources)
> diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
> index 72cd750..8a38d27 100644
> --- a/tests/conform/test-conform-main.c
> +++ b/tests/conform/test-conform-main.c
> @@ -120,6 +120,8 @@ main (int argc, char **argv)
>
> ADD_TEST (test_primitive_and_journal, 0, 0);
>
> + ADD_TEST (test_copy_replace_texture, 0, TEST_KNOWN_FAILURE);
> +
> UNPORTED_TEST (test_viewport);
>
> ADD_TEST (test_gles2_context, TEST_REQUIREMENT_GLES2_CONTEXT, 0);
> diff --git a/tests/conform/test-copy-replace-texture.c b/tests/conform/test-copy-replace-texture.c
> new file mode 100644
> index 0000000..7d5ab48
> --- /dev/null
> +++ b/tests/conform/test-copy-replace-texture.c
> @@ -0,0 +1,121 @@
> +#include <cogl/cogl.h>
> +#include <string.h>
> +
> +#include "test-utils.h"
> +
> +/* Keep track of the number of textures that we've created and are
> + * still alive */
> +static int alive_texture_mask = 0;
> +
> +#define N_LAYERS 3
> +#define N_PIPELINES 4
> +
> +#define PIPELINE_LAYER_MASK(pipeline_num) \
> + (((1 << N_LAYERS) - 1) << (N_LAYERS * (pipeline_num) + 1))
> +#define LAST_PIPELINE_MASK PIPELINE_LAYER_MASK (N_PIPELINES - 1)
> +#define FIRST_PIPELINE_MASK PIPELINE_LAYER_MASK (0)
> +
> +static void
> +free_texture_cb (void *user_data)
> +{
> + int texture_num = GPOINTER_TO_INT (user_data);
> +
> + alive_texture_mask &= ~(1 << texture_num);
> +}
> +
> +static CoglTexture *
> +create_texture (void)
> +{
> + static const guint8 data[] =
> + { 0xff, 0xff, 0xff, 0xff };
> + static CoglUserDataKey texture_data_key;
> + CoglTexture2D *tex_2d;
> + static int texture_num = 1;
> +
> + alive_texture_mask |= (1 << texture_num);
> +
> + tex_2d = cogl_texture_2d_new_from_data (test_ctx,
> + 1, 1, /* width / height */
> + COGL_PIXEL_FORMAT_RGBA_8888_PRE,
> + COGL_PIXEL_FORMAT_ANY,
> + 4, /* rowstride */
> + data,
> + NULL);
> +
> + /* Set some user data on the texture so we can track when it has
> + * been destroyed */
> + cogl_object_set_user_data (COGL_OBJECT (tex_2d),
> + &texture_data_key,
> + GINT_TO_POINTER (texture_num),
> + free_texture_cb);
> +
> + texture_num++;
> +
> + return COGL_TEXTURE (tex_2d);
> +}
> +
> +void
> +test_copy_replace_texture (void)
> +{
> + CoglPipeline *pipelines[N_PIPELINES];
> + int pipeline_num;
> +
> + /* Create a set of pipeline copies each with three of their own
> + * replacement textures */
> + for (pipeline_num = 0; pipeline_num < N_PIPELINES; pipeline_num++)
> + {
> + int layer_num;
> +
> + if (pipeline_num == 0)
> + pipelines[pipeline_num] = cogl_pipeline_new (test_ctx);
> + else
> + pipelines[pipeline_num] =
> + cogl_pipeline_copy (pipelines[pipeline_num - 1]);
> +
> + for (layer_num = 0; layer_num < N_LAYERS; layer_num++)
> + {
> + CoglTexture *tex = create_texture ();
> + cogl_pipeline_set_layer_texture (pipelines[pipeline_num],
> + layer_num,
> + tex);
> + cogl_object_unref (tex);
> + }
> + }
> +
> + /* Unref everything but the last pipeline */
> + for (pipeline_num = 0; pipeline_num < N_PIPELINES - 1; pipeline_num++)
> + cogl_object_unref (pipelines[pipeline_num]);
> +
> + if (alive_texture_mask && cogl_test_verbose ())
> + {
> + int i;
> +
> + g_print ("Alive textures:");
> +
> + for (i = 0; i < N_PIPELINES * N_LAYERS; i++)
> + if ((alive_texture_mask & (1 << (i + 1))))
> + g_print (" %i", i);
> +
> + g_print ("\n");
> + }
> +
> + /* Ideally there should only be the textures from the last pipeline
> + * left alive. We also let Cogl keep the textures from the first
> + * texture alive because currently the child of the third layer in
> + * the first pipeline will retain its authority on the unit index
> + * state so that it can set it to 2. If there are more textures then
> + * it means the pipeline isn't correctly pruning redundant
> + * ancestors */
> + g_assert_cmpint (alive_texture_mask & ~FIRST_PIPELINE_MASK,
> + ==,
> + LAST_PIPELINE_MASK);
> +
> + /* Clean up the last pipeline */
> + cogl_object_unref (pipelines[N_PIPELINES - 1]);
> +
> + /* That should get rid of the last of the textures */
> + g_assert_cmpint (alive_texture_mask, ==, 0);
> +
> + if (cogl_test_verbose ())
> + g_print ("OK\n");
> +}
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list