[Mesa-dev] [PATCH 02/31] mesa: stop using _NEW_STENCIL with st/mesa, use DriverFlags.NewStencil instead
Brian Paul
brianp at vmware.com
Tue Jun 13 13:49:31 UTC 2017
On 06/12/2017 10:55 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> 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 <brianp at vmware.com>
-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;
>
More information about the mesa-dev
mailing list