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

[Eric Anholt]

"Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:

> On Wed, May 21, 2014 at 12:51:07PM -0700, Eric Anholt wrote:
>> "Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:
>> 
>> > 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?
>> 
>> Sounds like it's time to track down why the original value was invalid,
>> and fix it.
>
> So I take it that any normal RGBA texture should have either GL_DEPTH_COMPONENT
> or GL_STENCIL_INDEX as the value for the stencil texturing mode even if
> neither is really applicable?

The ARB_stencil_texturing.txt says the default value is
GL_DEPTH_COMPONENT -- we should probably have a test for
glGetTexParameteri on the parameter on a normal RGBA texture.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20140521/18284af5/attachment.sig>