[Mesa-dev] [PATCH 2/2] mesa: Dynamically allocate the matrix stack.

[Brian Paul]

Just two minor things below.

On 08/03/2016 11:27 AM, Eric Anholt wrote:
> By allocating and initializing the matrices at context creation, the OS
> can't even overcommit the pages.  This saves about 63k (out of 946k) of
> maximum memory size according to massif on simulated vc4
> glsl-algebraic-add-add-1.  It also means we could potentially relax the
> maximum stack sizes, but that should be a separate commit.
> ---
>   src/mesa/main/matrix.c | 27 ++++++++++++++++++++++++---
>   src/mesa/main/mtypes.h |  1 +
>   2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/matrix.c b/src/mesa/main/matrix.c
> index 293d50c33595..a7cca799bcca 100644
> --- a/src/mesa/main/matrix.c
> +++ b/src/mesa/main/matrix.c
> @@ -243,6 +243,25 @@ _mesa_PushMatrix( void )
>         }
>         return;
>      }
> +   if (stack->Depth + 1 >= stack->StackSize) {
> +      unsigned new_stack_size = stack->StackSize * 2;
> +      unsigned i;
> +      GLmatrix *new_stack = realloc(stack->Stack,
> +                                    sizeof(*new_stack) * new_stack_size);
> +
> +      if (!new_stack) {
> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushMatrix()");
> +         return;
> +      }
> +
> +      for (i = stack->StackSize; i < new_stack_size; i++)
> +         _math_matrix_ctr(&new_stack[i]);
> +
> +      stack->Top = new_stack + (stack->Top - stack->Stack);

It looks like stack->Top is assigned later in the function so I don't 
think this assignment is needed.

> +      stack->Stack = new_stack;
> +      stack->StackSize = new_stack_size;
> +   }
> +
>      _math_matrix_copy( &stack->Stack[stack->Depth + 1],
>                         &stack->Stack[stack->Depth] );
>      stack->Depth++;
> @@ -646,8 +665,9 @@ init_matrix_stack( struct gl_matrix_stack *stack,
>      stack->MaxDepth = maxDepth;
>      stack->DirtyFlag = dirtyFlag;
>      /* The stack */
> -   stack->Stack = calloc(maxDepth, sizeof(GLmatrix));
> -   for (i = 0; i < maxDepth; i++) {

Maybe put a comment here saying that the allocation for the stack will 
grow as needed in _mesa_PushMatrix().


> +   stack->Stack = calloc(1, sizeof(GLmatrix));
> +   stack->StackSize = 1;
> +   for (i = 0; i < stack->StackSize; i++) {
>         _math_matrix_ctr(&stack->Stack[i]);
>      }
>      stack->Top = stack->Stack;
> @@ -665,11 +685,12 @@ static void
>   free_matrix_stack( struct gl_matrix_stack *stack )
>   {
>      GLuint i;
> -   for (i = 0; i < stack->MaxDepth; i++) {
> +   for (i = 0; i < stack->StackSize; i++) {
>         _math_matrix_dtr(&stack->Stack[i]);
>      }
>      free(stack->Stack);
>      stack->Stack = stack->Top = NULL;
> +   stack->StackSize = 0;
>   }
>
>   /*@}*/
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 2647f8fab841..5b02fadf3cd8 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3990,6 +3990,7 @@ struct gl_matrix_stack
>   {
>      GLmatrix *Top;      /**< points into Stack */
>      GLmatrix *Stack;    /**< array [MaxDepth] of GLmatrix */
> +   unsigned StackSize; /**< Number of elements in Stack */
>      GLuint Depth;       /**< 0 <= Depth < MaxDepth */
>      GLuint MaxDepth;    /**< size of Stack[] array */
>      GLuint DirtyFlag;   /**< _NEW_MODELVIEW or _NEW_PROJECTION, for example */
>