[Cogl] [PATCH] Add unit test to verify that modifying uniforms doesn't create chain

[Robert Bragg]

This looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
Robert

On Mon, Sep 2, 2013 at 3:30 PM, Neil Roberts <neil at linux.intel.com> wrote:
> The recommended usage model for rendering pipelines with minor changes
> is to make a copy of a base pipeline just before rendering and then
> modify that. The new pipeline can then be used as the base pipeline
> for the next paint. Currently this has a known problem when modifying
> uniform values in that Cogl won't prune the redundant ancestry and
> instead it will end up with an ever-growing chain of pipelines. This
> is particularly bad for something like CoglGST where it could also end
> up leaking textures for the video frames if the pipelines are used to
> render video.
>
> The patch adds a test case for that situation so that we won't forget
> about the problem. The test is maked as a known failure. Additionally
> the patch adds a similar test for setting the blend constant to
> constrast the test with some state that does work correctly.
> ---
>  cogl/cogl-pipeline-state.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>
> diff --git a/cogl/cogl-pipeline-state.c b/cogl/cogl-pipeline-state.c
> index b5e391b..131a09a 100644
> --- a/cogl/cogl-pipeline-state.c
> +++ b/cogl/cogl-pipeline-state.c
> @@ -38,6 +38,8 @@
>  #include "cogl-snippet-private.h"
>  #include "cogl-error-private.h"
>
> +#include <test-fixtures/test-unit.h>
> +
>  #include "string.h"
>
>  #ifndef GL_FUNC_ADD
> @@ -1614,3 +1616,72 @@ _cogl_pipeline_hash_fragment_snippets_state (CoglPipeline *authority,
>    _cogl_pipeline_snippet_list_hash (&authority->big_state->fragment_snippets,
>                                      &state->hash);
>  }
> +
> +UNIT_TEST (check_blend_constant_ancestry,
> +           0 /* no requirements */,
> +           0 /* no known failures */)
> +{
> +  CoglPipeline *pipeline = cogl_pipeline_new (test_ctx);
> +  CoglNode *node;
> +  int pipeline_length = 0;
> +  int i;
> +
> +  /* Repeatedly making a copy of a pipeline and changing the same
> +   * state (in this case the blend constant) shouldn't cause a long
> +   * chain of pipelines to be created because the redundant ancestry
> +   * should be pruned. */
> +
> +  for (i = 0; i < 20; i++)
> +    {
> +      CoglColor color = { i / 20.0f, 0.0f, 0.0f, 1.0f };
> +      CoglPipeline *tmp_pipeline;
> +
> +      tmp_pipeline = cogl_pipeline_copy (pipeline);
> +      cogl_object_unref (pipeline);
> +      pipeline = tmp_pipeline;
> +
> +      cogl_pipeline_set_blend_constant (pipeline, &color);
> +    }
> +
> +  for (node = (CoglNode *) pipeline; node; node = node->parent)
> +    pipeline_length++;
> +
> +  g_assert_cmpint (pipeline_length, <=, 2);
> +
> +  cogl_object_unref (pipeline);
> +}
> +
> +UNIT_TEST (check_uniform_ancestry,
> +           0 /* no requirements */,
> +           TEST_KNOWN_FAILURE)
> +{
> +  CoglPipeline *pipeline = cogl_pipeline_new (test_ctx);
> +  CoglNode *node;
> +  int pipeline_length = 0;
> +  int i;
> +
> +  /* Repeatedly making a copy of a pipeline and changing a uniform
> +   * shouldn't cause a long chain of pipelines to be created */
> +
> +  for (i = 0; i < 20; i++)
> +    {
> +      CoglPipeline *tmp_pipeline;
> +      int uniform_location;
> +
> +      tmp_pipeline = cogl_pipeline_copy (pipeline);
> +      cogl_object_unref (pipeline);
> +      pipeline = tmp_pipeline;
> +
> +      uniform_location =
> +        cogl_pipeline_get_uniform_location (pipeline, "a_uniform");
> +
> +      cogl_pipeline_set_uniform_1i (pipeline, uniform_location, i);
> +    }
> +
> +  for (node = (CoglNode *) pipeline; node; node = node->parent)
> +    pipeline_length++;
> +
> +  g_assert_cmpint (pipeline_length, <=, 2);
> +
> +  cogl_object_unref (pipeline);
> +}
> --
> 1.8.3.1
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl