On 06/12/2017 10:55 AM, Marek Olšák wrote:
From: Marek Olšák <[email protected]>

This bypasses _mesa_update_state_locked.

Before:
    DrawElements ( 1 VBOs, 4 UBOs,  8 Tex) w/ stencil enable change:    3.99 
million
    DrawArrays   ( 1 VBOs, 4 UBOs,  8 Tex) w/ stencil enable change:    4.56 
million

After:
    DrawElements ( 1 VBOs, 4 UBOs,  8 Tex) w/ stencil enable change:    4.93 
million
    DrawArrays   ( 1 VBOs, 4 UBOs,  8 Tex) w/ stencil enable change:    5.84 
million

It's quite a difference in the draw call rate when ctx->NewState stays
equal to 0 the whole time.
---
  src/mesa/main/enable.c              |  6 ++++--
  src/mesa/main/mtypes.h              |  3 +++
  src/mesa/main/stencil.c             | 33 ++++++++++++++++++++++-----------
  src/mesa/state_tracker/st_context.c |  4 ++--
  4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
index 91c1f0d..128b5a9 100644
--- a/src/mesa/main/enable.c
+++ b/src/mesa/main/enable.c
@@ -672,21 +672,22 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
GLboolean state)
              if (newEnabled != ctx->Scissor.EnableFlags) {
                 FLUSH_VERTICES(ctx, _NEW_SCISSOR);
                 ctx->NewDriverState |= ctx->DriverFlags.NewScissorTest;
                 ctx->Scissor.EnableFlags = newEnabled;
              }
           }
           break;
        case GL_STENCIL_TEST:
           if (ctx->Stencil.Enabled == state)
              return;
-         FLUSH_VERTICES(ctx, _NEW_STENCIL);
+         FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+         ctx->NewDriverState |= ctx->DriverFlags.NewStencil;

This seems to be a very common pattern now. Maybe a new macro would be good. Something like:

#define STATE_CHANGE(ctx, newstate, driverFlag) \
do { \
   if (MESA_VERBOSE & VERBOSE_STATE) \
      _mesa_debug(ctx, "FLUSH_VERTICES in %s\n", MESA_FUNCTION); \
   if (ctx->Driver.NeedFlush & FLUSH_STORED_VERTICES) \
      vbo_exec_FlushVertices(ctx, FLUSH_STORED_VERTICES); \
   if (driverFlag) \
      ctx->NewDriverState |= driverFlag; \
   else \
      ctx->NewState |= newstate; \
} while (0)

I think that's a bit easier to understand, actually.

And if we don't care about the MESA_VERBOSE & VERBOSE_STATE debugging, this could be an inline function: _mesa_state_change().


I did a quick review of the series and it generally looks good to me.

Reviewed-by: Brian Paul <[email protected]>

-Brian


           ctx->Stencil.Enabled = state;
           break;
        case GL_TEXTURE_1D:
           if (ctx->API != API_OPENGL_COMPAT)
              goto invalid_enum_error;
           if (!enable_texture(ctx, state, TEXTURE_1D_BIT)) {
              return;
           }
           break;
        case GL_TEXTURE_2D:
@@ -898,21 +899,22 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, 
GLboolean state)
           }
           break;

        /* GL_EXT_stencil_two_side */
        case GL_STENCIL_TEST_TWO_SIDE_EXT:
           if (ctx->API != API_OPENGL_COMPAT)
              goto invalid_enum_error;
           CHECK_EXTENSION(EXT_stencil_two_side, cap);
           if (ctx->Stencil.TestTwoSide == state)
              return;
-         FLUSH_VERTICES(ctx, _NEW_STENCIL);
+         FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+         ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
           ctx->Stencil.TestTwoSide = state;
           if (state) {
              ctx->Stencil._BackFace = 2;
           } else {
              ctx->Stencil._BackFace = 1;
           }
           break;

        case GL_FRAGMENT_PROGRAM_ARB:
           if (ctx->API != API_OPENGL_COMPAT)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 1e02ff0..283215f 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4402,20 +4402,23 @@ struct gl_driver_flags
     uint64_t NewWindowRectangles;

     /** gl_context::Color::sRGBEnabled */
     uint64_t NewFramebufferSRGB;

     /** gl_context::Scissor::EnableFlags */
     uint64_t NewScissorTest;

     /** gl_context::Scissor::ScissorArray */
     uint64_t NewScissorRect;
+
+   /** gl_context::Stencil */
+   uint64_t NewStencil;
  };

  struct gl_uniform_buffer_binding
  {
     struct gl_buffer_object *BufferObject;
     /** Start of uniform block data in the buffer */
     GLintptr Offset;
     /** Size of data allowed to be referenced from the buffer (in bytes) */
     GLsizeiptr Size;
     /**
diff --git a/src/mesa/main/stencil.c b/src/mesa/main/stencil.c
index 2547f45..ea04f2e 100644
--- a/src/mesa/main/stencil.c
+++ b/src/mesa/main/stencil.c
@@ -150,21 +150,22 @@ _mesa_StencilFuncSeparateATI( GLenum frontfunc, GLenum 
backfunc, GLint ref, GLui
     }

     /* set both front and back state */
     if (ctx->Stencil.Function[0] == frontfunc &&
         ctx->Stencil.Function[1] == backfunc &&
         ctx->Stencil.ValueMask[0] == mask &&
         ctx->Stencil.ValueMask[1] == mask &&
         ctx->Stencil.Ref[0] == ref &&
         ctx->Stencil.Ref[1] == ref)
        return;
-   FLUSH_VERTICES(ctx, _NEW_STENCIL);
+   FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+   ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
     ctx->Stencil.Function[0]  = frontfunc;
     ctx->Stencil.Function[1]  = backfunc;
     ctx->Stencil.Ref[0]       = ctx->Stencil.Ref[1]       = ref;
     ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask;
     if (ctx->Driver.StencilFuncSeparate) {
        ctx->Driver.StencilFuncSeparate(ctx, GL_FRONT,
                                        frontfunc, ref, mask);
        ctx->Driver.StencilFuncSeparate(ctx, GL_BACK,
                                        backfunc, ref, mask);
     }
@@ -196,42 +197,44 @@ _mesa_StencilFunc( GLenum func, GLint ref, GLuint mask )
     if (!validate_stencil_func(ctx, func)) {
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilFunc(func)");
        return;
     }

     if (face != 0) {
        if (ctx->Stencil.Function[face] == func &&
            ctx->Stencil.ValueMask[face] == mask &&
            ctx->Stencil.Ref[face] == ref)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.Function[face] = func;
        ctx->Stencil.Ref[face] = ref;
        ctx->Stencil.ValueMask[face] = mask;

        /* Only propagate the change to the driver if EXT_stencil_two_side
         * is enabled.
         */
        if (ctx->Driver.StencilFuncSeparate && ctx->Stencil.TestTwoSide) {
           ctx->Driver.StencilFuncSeparate(ctx, GL_BACK, func, ref, mask);
        }
     }
     else {
        /* set both front and back state */
        if (ctx->Stencil.Function[0] == func &&
            ctx->Stencil.Function[1] == func &&
            ctx->Stencil.ValueMask[0] == mask &&
            ctx->Stencil.ValueMask[1] == mask &&
            ctx->Stencil.Ref[0] == ref &&
            ctx->Stencil.Ref[1] == ref)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.Function[0]  = ctx->Stencil.Function[1]  = func;
        ctx->Stencil.Ref[0]       = ctx->Stencil.Ref[1]       = ref;
        ctx->Stencil.ValueMask[0] = ctx->Stencil.ValueMask[1] = mask;
        if (ctx->Driver.StencilFuncSeparate) {
           ctx->Driver.StencilFuncSeparate(ctx,
                                         ((ctx->Stencil.TestTwoSide)
                                          ? GL_FRONT : GL_FRONT_AND_BACK),
                                           func, ref, mask);
        }
     }
@@ -256,36 +259,38 @@ _mesa_StencilMask( GLuint mask )
     const GLint face = ctx->Stencil.ActiveFace;

     if (MESA_VERBOSE & VERBOSE_API)
        _mesa_debug(ctx, "glStencilMask()\n");

     if (face != 0) {
        /* Only modify the EXT_stencil_two_side back-face state.
         */
        if (ctx->Stencil.WriteMask[face] == mask)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.WriteMask[face] = mask;

        /* Only propagate the change to the driver if EXT_stencil_two_side
         * is enabled.
         */
        if (ctx->Driver.StencilMaskSeparate && ctx->Stencil.TestTwoSide) {
           ctx->Driver.StencilMaskSeparate(ctx, GL_BACK, mask);
        }
     }
     else {
        /* set both front and back state */
        if (ctx->Stencil.WriteMask[0] == mask &&
            ctx->Stencil.WriteMask[1] == mask)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.WriteMask[0] = ctx->Stencil.WriteMask[1] = mask;
        if (ctx->Driver.StencilMaskSeparate) {
           ctx->Driver.StencilMaskSeparate(ctx,
                                         ((ctx->Stencil.TestTwoSide)
                                          ? GL_FRONT : GL_FRONT_AND_BACK),
                                          mask);
        }
     }
  }

@@ -325,42 +330,44 @@ _mesa_StencilOp(GLenum fail, GLenum zfail, GLenum zpass)
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilOp(zpass)");
        return;
     }

     if (face != 0) {
        /* only set active face state */
        if (ctx->Stencil.ZFailFunc[face] == zfail &&
            ctx->Stencil.ZPassFunc[face] == zpass &&
            ctx->Stencil.FailFunc[face] == fail)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.ZFailFunc[face] = zfail;
        ctx->Stencil.ZPassFunc[face] = zpass;
        ctx->Stencil.FailFunc[face] = fail;

        /* Only propagate the change to the driver if EXT_stencil_two_side
         * is enabled.
         */
        if (ctx->Driver.StencilOpSeparate && ctx->Stencil.TestTwoSide) {
           ctx->Driver.StencilOpSeparate(ctx, GL_BACK, fail, zfail, zpass);
        }
     }
     else {
        /* set both front and back state */
        if (ctx->Stencil.ZFailFunc[0] == zfail &&
            ctx->Stencil.ZFailFunc[1] == zfail &&
            ctx->Stencil.ZPassFunc[0] == zpass &&
            ctx->Stencil.ZPassFunc[1] == zpass &&
            ctx->Stencil.FailFunc[0] == fail &&
            ctx->Stencil.FailFunc[1] == fail)
           return;
-      FLUSH_VERTICES(ctx, _NEW_STENCIL);
+      FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+      ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
        ctx->Stencil.ZFailFunc[0] = ctx->Stencil.ZFailFunc[1] = zfail;
        ctx->Stencil.ZPassFunc[0] = ctx->Stencil.ZPassFunc[1] = zpass;
        ctx->Stencil.FailFunc[0]  = ctx->Stencil.FailFunc[1]  = fail;
        if (ctx->Driver.StencilOpSeparate) {
           ctx->Driver.StencilOpSeparate(ctx,
                                       ((ctx->Stencil.TestTwoSide)
                                        ? GL_FRONT : GL_FRONT_AND_BACK),
                                         fail, zfail, zpass);
        }
     }
@@ -416,33 +423,35 @@ _mesa_StencilOpSeparate(GLenum face, GLenum sfail, GLenum 
zfail, GLenum zpass)
     if (face != GL_FRONT && face != GL_BACK && face != GL_FRONT_AND_BACK) {
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilOpSeparate(face)");
        return;
     }

     if (face != GL_BACK) {
        /* set front */
        if (ctx->Stencil.ZFailFunc[0] != zfail ||
            ctx->Stencil.ZPassFunc[0] != zpass ||
            ctx->Stencil.FailFunc[0] != sfail){
-         FLUSH_VERTICES(ctx, _NEW_STENCIL);
+         FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+         ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
           ctx->Stencil.ZFailFunc[0] = zfail;
           ctx->Stencil.ZPassFunc[0] = zpass;
           ctx->Stencil.FailFunc[0] = sfail;
           set = GL_TRUE;
        }
     }
     if (face != GL_FRONT) {
        /* set back */
        if (ctx->Stencil.ZFailFunc[1] != zfail ||
            ctx->Stencil.ZPassFunc[1] != zpass ||
            ctx->Stencil.FailFunc[1] != sfail) {
-         FLUSH_VERTICES(ctx, _NEW_STENCIL);
+         FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+         ctx->NewDriverState |= ctx->DriverFlags.NewStencil;
           ctx->Stencil.ZFailFunc[1] = zfail;
           ctx->Stencil.ZPassFunc[1] = zpass;
           ctx->Stencil.FailFunc[1] = sfail;
           set = GL_TRUE;
        }
     }
     if (set && ctx->Driver.StencilOpSeparate) {
        ctx->Driver.StencilOpSeparate(ctx, face, sfail, zfail, zpass);
     }
  }
@@ -459,21 +468,22 @@ _mesa_StencilFuncSeparate(GLenum face, GLenum func, GLint 
ref, GLuint mask)

     if (face != GL_FRONT && face != GL_BACK && face != GL_FRONT_AND_BACK) {
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilFuncSeparate(face)");
        return;
     }
     if (!validate_stencil_func(ctx, func)) {
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilFuncSeparate(func)");
        return;
     }

-   FLUSH_VERTICES(ctx, _NEW_STENCIL);
+   FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+   ctx->NewDriverState |= ctx->DriverFlags.NewStencil;

     if (face != GL_BACK) {
        /* set front */
        ctx->Stencil.Function[0] = func;
        ctx->Stencil.Ref[0] = ref;
        ctx->Stencil.ValueMask[0] = mask;
     }
     if (face != GL_FRONT) {
        /* set back */
        ctx->Stencil.Function[1] = func;
@@ -493,21 +503,22 @@ _mesa_StencilMaskSeparate(GLenum face, GLuint mask)
     GET_CURRENT_CONTEXT(ctx);

     if (MESA_VERBOSE & VERBOSE_API)
        _mesa_debug(ctx, "glStencilMaskSeparate()\n");

     if (face != GL_FRONT && face != GL_BACK && face != GL_FRONT_AND_BACK) {
        _mesa_error(ctx, GL_INVALID_ENUM, "glStencilaMaskSeparate(face)");
        return;
     }

-   FLUSH_VERTICES(ctx, _NEW_STENCIL);
+   FLUSH_VERTICES(ctx, ctx->DriverFlags.NewStencil ? 0 : _NEW_STENCIL);
+   ctx->NewDriverState |= ctx->DriverFlags.NewStencil;

     if (face != GL_BACK) {
        ctx->Stencil.WriteMask[0] = mask;
     }
     if (face != GL_FRONT) {
        ctx->Stencil.WriteMask[1] = mask;
     }
     if (ctx->Driver.StencilMaskSeparate) {
        ctx->Driver.StencilMaskSeparate(ctx, face, mask);
     }
diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index b99a53b..b9ebc38 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -186,22 +186,21 @@ st_invalidate_state(struct gl_context * ctx)
  {
     GLbitfield new_state = ctx->NewState;
     struct st_context *st = st_context(ctx);

     if (new_state & _NEW_BUFFERS) {
        st_invalidate_buffers(st);
     } else {
        /* These set a subset of flags set by _NEW_BUFFERS, so we only have to
         * check them when _NEW_BUFFERS isn't set.
         */
-      if (new_state & (_NEW_DEPTH |
-                       _NEW_STENCIL))
+      if (new_state & _NEW_DEPTH)
           st->dirty |= ST_NEW_DSA;

        if (new_state & _NEW_PROGRAM)
           st->dirty |= ST_NEW_RASTERIZER;

        if (new_state & _NEW_FOG)
           st->dirty |= ST_NEW_FS_STATE;

        if (new_state & _NEW_POLYGONSTIPPLE)
           st->dirty |= ST_NEW_POLY_STIPPLE;
@@ -514,20 +513,21 @@ static void st_init_driver_flags(struct st_context *st)
     f->NewUniformBuffer = ST_NEW_UNIFORM_BUFFER;
     f->NewDefaultTessLevels = ST_NEW_TESS_STATE;
     f->NewTextureBuffer = ST_NEW_SAMPLER_VIEWS;
     f->NewAtomicBuffer = ST_NEW_ATOMIC_BUFFER;
     f->NewShaderStorageBuffer = ST_NEW_STORAGE_BUFFER;
     f->NewImageUnits = ST_NEW_IMAGE_UNITS;
     f->NewWindowRectangles = ST_NEW_WINDOW_RECTANGLES;
     f->NewFramebufferSRGB = ST_NEW_FRAMEBUFFER;
     f->NewScissorRect = ST_NEW_SCISSOR;
     f->NewScissorTest = ST_NEW_SCISSOR | ST_NEW_RASTERIZER;
+   f->NewStencil = ST_NEW_DSA;
  }

  struct st_context *st_create_context(gl_api api, struct pipe_context *pipe,
                                       const struct gl_config *visual,
                                       struct st_context *share,
                                       const struct st_config_options *options)
  {
     struct gl_context *ctx;
     struct gl_context *shareCtx = share ? share->ctx : NULL;
     struct dd_function_table funcs;


_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to