[Cogl] [PATCH] matrix-stack: Prune stack entries when loading a replacement matrix

[Robert Bragg]

Ah, yep this looks sensible.

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

regards,
- Robert

On Thu, May 17, 2012 at 11:41 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Previously if an application does not use the matrix stack as a stack
> but instead just loads its own matrices for each frame using
> cogl_framebuffer_set_modelview_matrix then it will continously push
> OP_LOAD entries on the stack and the stack will grow forever. This
> patch fixes that use case by resetting the top of the stack to the
> last save entry whenever something is pushed that replaces the
> previous matrix on the stack.
> ---
>  cogl/cogl-matrix-stack.c |   66 ++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 50 insertions(+), 16 deletions(-)
>
> diff --git a/cogl/cogl-matrix-stack.c b/cogl/cogl-matrix-stack.c
> index 14a9040..2d61a5e 100644
> --- a/cogl/cogl-matrix-stack.c
> +++ b/cogl/cogl-matrix-stack.c
> @@ -69,6 +69,34 @@ _cogl_matrix_stack_push_entry (CoglMatrixStack *stack,
>   return entry;
>  }
>
> +static void *
> +_cogl_matrix_stack_push_replacement_entry (CoglMatrixStack *stack,
> +                                           size_t size,
> +                                           CoglMatrixOp operation)
> +{
> +  CoglMatrixEntry *old_top = stack->last_entry;
> +  CoglMatrixEntry *new_top;
> +
> +  /* This would only be called for operations that completely replace
> +   * the matrix. In that case we don't need to keep a reference to
> +   * anything up to the last save entry. This optimisation could be
> +   * important for applications that aren't using the stack but
> +   * instead just perform their own matrix manipulations and load a
> +   * new stack every frame. If this optimisation isn't done then the
> +   * stack would just grow endlessly. See the comments
> +   * _cogl_matrix_stack_pop for a description of how popping works. */
> +  for (new_top = old_top;
> +       new_top->op != COGL_MATRIX_OP_SAVE && new_top->parent;
> +       new_top = new_top->parent)
> +    ;
> +
> +  _cogl_matrix_entry_ref (new_top);
> +  _cogl_matrix_entry_unref (old_top);
> +  stack->last_entry = new_top;
> +
> +  return _cogl_matrix_stack_push_entry (stack, size, operation);
> +}
> +
>  void
>  _cogl_matrix_entry_identity_init (CoglMatrixEntry *entry)
>  {
> @@ -81,9 +109,9 @@ _cogl_matrix_entry_identity_init (CoglMatrixEntry *entry)
>  void
>  _cogl_matrix_stack_load_identity (CoglMatrixStack *stack)
>  {
> -  _cogl_matrix_stack_push_entry (stack,
> -                                 sizeof (CoglMatrixEntry),
> -                                 COGL_MATRIX_OP_LOAD_IDENTITY);
> +  _cogl_matrix_stack_push_replacement_entry (stack,
> +                                             sizeof (CoglMatrixEntry),
> +                                             COGL_MATRIX_OP_LOAD_IDENTITY);
>  }
>
>  void
> @@ -192,9 +220,10 @@ _cogl_matrix_stack_set (CoglMatrixStack *stack,
>  {
>   CoglMatrixEntryLoad *entry;
>
> -  entry = _cogl_matrix_stack_push_entry (stack,
> -                                         sizeof (CoglMatrixEntryLoad),
> -                                         COGL_MATRIX_OP_LOAD);
> +  entry =
> +    _cogl_matrix_stack_push_replacement_entry (stack,
> +                                               sizeof (CoglMatrixEntryLoad),
> +                                               COGL_MATRIX_OP_LOAD);
>
>   entry->matrix =
>     _cogl_magazine_chunk_alloc (_cogl_matrix_stack_matrices_magazine);
> @@ -213,9 +242,10 @@ _cogl_matrix_stack_frustum (CoglMatrixStack *stack,
>  {
>   CoglMatrixEntryLoad *entry;
>
> -  entry = _cogl_matrix_stack_push_entry (stack,
> -                                         sizeof (CoglMatrixEntryLoad),
> -                                         COGL_MATRIX_OP_LOAD);
> +  entry =
> +    _cogl_matrix_stack_push_replacement_entry (stack,
> +                                               sizeof (CoglMatrixEntryLoad),
> +                                               COGL_MATRIX_OP_LOAD);
>
>   entry->matrix =
>     _cogl_magazine_chunk_alloc (_cogl_matrix_stack_matrices_magazine);
> @@ -235,9 +265,10 @@ _cogl_matrix_stack_perspective (CoglMatrixStack *stack,
>  {
>   CoglMatrixEntryLoad *entry;
>
> -  entry = _cogl_matrix_stack_push_entry (stack,
> -                                         sizeof (CoglMatrixEntryLoad),
> -                                         COGL_MATRIX_OP_LOAD);
> +  entry =
> +    _cogl_matrix_stack_push_replacement_entry (stack,
> +                                               sizeof (CoglMatrixEntryLoad),
> +                                               COGL_MATRIX_OP_LOAD);
>
>   entry->matrix =
>     _cogl_magazine_chunk_alloc (_cogl_matrix_stack_matrices_magazine);
> @@ -258,9 +289,10 @@ _cogl_matrix_stack_orthographic (CoglMatrixStack *stack,
>  {
>   CoglMatrixEntryLoad *entry;
>
> -  entry = _cogl_matrix_stack_push_entry (stack,
> -                                         sizeof (CoglMatrixEntryLoad),
> -                                         COGL_MATRIX_OP_LOAD);
> +  entry =
> +    _cogl_matrix_stack_push_replacement_entry (stack,
> +                                               sizeof (CoglMatrixEntryLoad),
> +                                               COGL_MATRIX_OP_LOAD);
>
>   entry->matrix =
>     _cogl_magazine_chunk_alloc (_cogl_matrix_stack_matrices_magazine);
> @@ -605,7 +637,9 @@ _cogl_matrix_stack_new (void)
>
>   stack->last_entry = NULL;
>
> -  _cogl_matrix_stack_load_identity (stack);
> +  _cogl_matrix_stack_push_entry (stack,
> +                                 sizeof (CoglMatrixEntry),
> +                                 COGL_MATRIX_OP_LOAD_IDENTITY);
>
>   return _cogl_matrix_stack_object_new (stack);
>  }
> --
> 1.7.3.16.g9464b
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl