[Mesa-stable] [Mesa-dev] [PATCH 1/2] meta/blit: Add stencil texturing mode save and restore

[Pohjolainen, Topi]

On Wed, May 21, 2014 at 09:41:39PM +0300, Pohjolainen, Topi wrote:
> On Wed, May 21, 2014 at 09:37:30PM +0300, Pohjolainen, Topi wrote:
> > On Wed, May 21, 2014 at 11:29:02AM -0700, Eric Anholt wrote:
> > > Topi Pohjolainen <topi.pohjolainen at intel.com> writes:
> > > 
> > > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > > Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> > > > ---
> > > >  src/mesa/drivers/common/meta.h      |  4 ++--
> > > >  src/mesa/drivers/common/meta_blit.c | 15 ++++++++++++++-
> > > >  2 files changed, 16 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/src/mesa/drivers/common/meta.h b/src/mesa/drivers/common/meta.h
> > > > index 007f104..765f8df 100644
> > > > --- a/src/mesa/drivers/common/meta.h
> > > > +++ b/src/mesa/drivers/common/meta.h
> > > > @@ -270,7 +270,7 @@ struct blit_state
> > > >  struct fb_tex_blit_state
> > > >  {
> > > >     GLint baseLevelSave, maxLevelSave;
> > > > -   GLuint sampler, samplerSave;
> > > > +   GLuint sampler, samplerSave, stencilSamplingSave;
> > > >     GLuint tempTex;
> > > >  };
> > > >  
> > > > @@ -407,7 +407,7 @@ _mesa_meta_fb_tex_blit_begin(const struct gl_context *ctx,
> > > >                               struct fb_tex_blit_state *blit);
> > > >  
> > > >  extern void
> > > > -_mesa_meta_fb_tex_blit_end(const struct gl_context *ctx, GLenum target,
> > > > +_mesa_meta_fb_tex_blit_end(struct gl_context *ctx, GLenum target,
> > > >                             struct fb_tex_blit_state *blit);
> > > >  
> > > >  extern GLboolean
> > > > diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> > > > index e10a181..59f7e70 100644
> > > > --- a/src/mesa/drivers/common/meta_blit.c
> > > > +++ b/src/mesa/drivers/common/meta_blit.c
> > > > @@ -441,6 +441,7 @@ blitframebuffer_texture(struct gl_context *ctx,
> > > >  
> > > >     fb_tex_blit.baseLevelSave = texObj->BaseLevel;
> > > >     fb_tex_blit.maxLevelSave = texObj->MaxLevel;
> > > > +   fb_tex_blit.stencilSamplingSave = texObj->StencilSampling;
> > > >  
> > > >     if (glsl_version) {
> > > >        setup_glsl_blit_framebuffer(ctx, blit, rb, target);
> > > > @@ -567,7 +568,7 @@ _mesa_meta_fb_tex_blit_begin(const struct gl_context *ctx,
> > > >  }
> > > >  
> > > >  void
> > > > -_mesa_meta_fb_tex_blit_end(const struct gl_context *ctx, GLenum target,
> > > > +_mesa_meta_fb_tex_blit_end(struct gl_context *ctx, GLenum target,
> > > >                             struct fb_tex_blit_state *blit)
> > > >  {
> > > >     /* Restore texture object state, the texture binding will
> > > > @@ -576,6 +577,18 @@ _mesa_meta_fb_tex_blit_end(const struct gl_context *ctx, GLenum target,
> > > >     if (target != GL_TEXTURE_RECTANGLE_ARB) {
> > > >        _mesa_TexParameteri(target, GL_TEXTURE_BASE_LEVEL, blit->baseLevelSave);
> > > >        _mesa_TexParameteri(target, GL_TEXTURE_MAX_LEVEL, blit->maxLevelSave);
> > > > +
> > > > +      if (ctx->Extensions.ARB_stencil_texturing) {
> > > > +         struct gl_texture_object *texObj =
> > > > +            _mesa_get_current_tex_object(ctx, target);
> > > > +
> > > > +         /* One cannot use _mesa_TexParameteri() in case the stencil indexing
> > > > +          * wasn't modified during the blit. Hence write the saved value
> > > > +          * directly.
> > > > +          */
> > > 
> > > Can you explain that some more?  I'm uncomfortable with direct
> > > modification of the values instead of _mesa_TexParameteri, becuase it
> > > means that any other state update handling (like making sure that the
> > > call to Driver.TexParameter() has happened, or any necessary flushes)
> > > isn't getting done.
> > 
> > Glad to, if the mode was 'GL_DEPTH_COMPONENT' originally and the blit doesn't
> > modify it than the following issues an error:
> 
> That doesn't make sense afterall, let me check again which error I hit.

Right, the value can originally be invalid, I have simply zero in one of the
runs. So it looks that proper solution is to check in addition to the
stencil extension being supported that the original value was valid. And only
then try to restore. Does that sound right?

> 
> > 
> > 
> > static GLboolean
> > set_tex_parameteri(struct gl_context *ctx,
> >                    struct gl_texture_object *texObj,
> >                    GLenum pname, const GLint *params)
> > {
> > ...
> >    case GL_DEPTH_STENCIL_TEXTURE_MODE:
> >       if (_mesa_is_desktop_gl(ctx) && ctx->Extensions.ARB_stencil_texturing) {
> >          bool stencil = params[0] == GL_STENCIL_INDEX;
> >          if (!stencil && params[0] != GL_DEPTH_COMPONENT)
> >             goto invalid_param;
> > 
> >          if (texObj->StencilSampling == stencil)
> >             return GL_FALSE;
> > 
> >          texObj->StencilSampling = stencil;
> >          return GL_TRUE;
> >       }
> >       goto invalid_pname;
> > 
> > > 
> > > > +         if (texObj)
> > > > +            texObj->StencilSampling = blit->stencilSamplingSave;
> > 
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev