[Mesa-dev] [PATCH 1/2] mesa: reduce CPU overhead when updating the scissor

[Marek Olšák]

Hi Samuel,

FLUSH_VERTICES must be called *before* any of the states are changed, not after.

Marek

On Wed, Aug 2, 2017 at 9:42 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> Before:
> DrawElements (1 VBO, 8 UBO, 8 Tex) w/ scissor change: 3.67 million (44.4%)
>
> After:
> DrawElements (1 VBO, 8 UBO, 8 Tex) w/ scissor change: 5.02 million (62.1%)
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/mesa/main/scissor.c | 79 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
> index b38db06bc8..c2d85bae2e 100644
> --- a/src/mesa/main/scissor.c
> +++ b/src/mesa/main/scissor.c
> @@ -29,28 +29,20 @@
>  #include "main/mtypes.h"
>  #include "main/scissor.h"
>
> +static bool
> +is_scissor_equal(struct gl_context *ctx, unsigned idx,
> +                 GLint x, GLint y, GLsizei width, GLsizei height)
> +{
> +   return ctx->Scissor.ScissorArray[idx].X == x &&
> +          ctx->Scissor.ScissorArray[idx].Y == y &&
> +          ctx->Scissor.ScissorArray[idx].Width == width &&
> +          ctx->Scissor.ScissorArray[idx].Height == height;
> +}
>
> -/**
> - * Set scissor rectangle data directly in ScissorArray
> - *
> - * This is an internal function that performs no error checking on the
> - * supplied data.  It also does \b not call \c dd_function_table::Scissor.
> - *
> - * \sa _mesa_set_scissor
> - */
>  static void
> -set_scissor_no_notify(struct gl_context *ctx, unsigned idx,
> -                      GLint x, GLint y, GLsizei width, GLsizei height)
> +set_new_scissor(struct gl_context *ctx, unsigned idx,
> +                GLint x, GLint y, GLsizei width, GLsizei height)
>  {
> -   if (x == ctx->Scissor.ScissorArray[idx].X &&
> -       y == ctx->Scissor.ScissorArray[idx].Y &&
> -       width == ctx->Scissor.ScissorArray[idx].Width &&
> -       height == ctx->Scissor.ScissorArray[idx].Height)
> -      return;
> -
> -   FLUSH_VERTICES(ctx, ctx->DriverFlags.NewScissorRect ? 0 : _NEW_SCISSOR);
> -   ctx->NewDriverState |= ctx->DriverFlags.NewScissorRect;
> -
>     ctx->Scissor.ScissorArray[idx].X = x;
>     ctx->Scissor.ScissorArray[idx].Y = y;
>     ctx->Scissor.ScissorArray[idx].Width = width;
> @@ -58,8 +50,19 @@ set_scissor_no_notify(struct gl_context *ctx, unsigned idx,
>  }
>
>  static void
> +flush_scissor(struct gl_context *ctx)
> +{
> +   FLUSH_VERTICES(ctx, ctx->DriverFlags.NewScissorRect ? 0 : _NEW_SCISSOR);
> +   ctx->NewDriverState |= ctx->DriverFlags.NewScissorRect;
> +
> +   if (ctx->Driver.Scissor)
> +      ctx->Driver.Scissor(ctx);
> +}
> +
> +static void
>  scissor(struct gl_context *ctx, GLint x, GLint y, GLsizei width, GLsizei height)
>  {
> +   bool changed = false;
>     unsigned i;
>
>     /* The GL_ARB_viewport_array spec says:
> @@ -74,11 +77,16 @@ scissor(struct gl_context *ctx, GLint x, GLint y, GLsizei width, GLsizei height)
>      * Set the scissor rectangle for all of the viewports supported by the
>      * implementation, but only signal the driver once at the end.
>      */
> -   for (i = 0; i < ctx->Const.MaxViewports; i++)
> -      set_scissor_no_notify(ctx, i, x, y, width, height);
> +   for (i = 0; i < ctx->Const.MaxViewports; i++) {
> +      if (!changed && is_scissor_equal(ctx, i, x, y, width, height))
> +         continue;
>
> -   if (ctx->Driver.Scissor)
> -      ctx->Driver.Scissor(ctx);
> +      changed = true;
> +      set_new_scissor(ctx, i, x, y, width, height);
> +   }
> +
> +   if (changed)
> +      flush_scissor(ctx);
>  }
>
>  /**
> @@ -125,23 +133,31 @@ void
>  _mesa_set_scissor(struct gl_context *ctx, unsigned idx,
>                    GLint x, GLint y, GLsizei width, GLsizei height)
>  {
> -   set_scissor_no_notify(ctx, idx, x, y, width, height);
> +   if (is_scissor_equal(ctx, idx, x, y, width, height))
> +      return;
>
> -   if (ctx->Driver.Scissor)
> -      ctx->Driver.Scissor(ctx);
> +   set_new_scissor(ctx, idx, x, y, width, height);
> +   flush_scissor(ctx);
>  }
>
>  static void
>  scissor_array(struct gl_context *ctx, GLuint first, GLsizei count,
>                struct gl_scissor_rect *rect)
>  {
> +   bool changed = false;
> +
>     for (GLsizei i = 0; i < count; i++) {
> -      set_scissor_no_notify(ctx, i + first, rect[i].X, rect[i].Y,
> -                            rect[i].Width, rect[i].Height);
> +      if (!changed && is_scissor_equal(ctx, i + first, rect[i].X, rect[i].Y,
> +                                       rect[i].Width, rect[i].Height))
> +         continue;
> +
> +      changed = true;
> +      set_new_scissor(ctx, i + first, rect[i].X, rect[i].Y,
> +                      rect[i].Width, rect[i].Height);
>     }
>
> -   if (ctx->Driver.Scissor)
> -      ctx->Driver.Scissor(ctx);
> +   if (changed)
> +      flush_scissor(ctx);
>  }
>
>  /**
> @@ -328,5 +344,6 @@ _mesa_init_scissor(struct gl_context *ctx)
>      * so just initialize all of them.
>      */
>     for (i = 0; i < MAX_VIEWPORTS; i++)
> -      set_scissor_no_notify(ctx, i, 0, 0, 0, 0);
> +      set_new_scissor(ctx, i, 0, 0, 0, 0);
> +   flush_scissor(ctx);
>  }
> --
> 2.13.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev