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

[Samuel Pitoiset]

On 08/04/2017 12:33 AM, Marek Olšák wrote:
> Hi Samuel,
> 
> FLUSH_VERTICES must be called *before* any of the states are changed, not after.

My mistake, please ignore this series...

Samuel.

> 
> 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