[Mesa-dev] [PATCH 1/4] mesa/teximage: use correct extension for accept stencil texture.

[Kenneth Graunke]

On Monday, April 06, 2015 09:44:07 PM Pohjolainen, Topi wrote:
> On Mon, Apr 06, 2015 at 11:37:08AM -0700, Ian Romanick wrote:
> > On 04/06/2015 08:33 AM, Pohjolainen, Topi wrote:
> > > On Sun, Apr 05, 2015 at 08:22:13PM +0300, Pohjolainen, Topi wrote:
> > >> On Sun, Apr 05, 2015 at 08:06:50PM +0300, Pohjolainen, Topi wrote:
> > >>> On Sun, Apr 05, 2015 at 08:46:16AM -0400, Ilia Mirkin wrote:
> > >>>> While this change is correct, the Intel guys will yell at you, because
> > >>>> they're somehow misusing this in meta for Broadwell, s.t. this will
> > >>>> cause crashes when blitting stencil. IMHO that's a problem that should
> > >>>> be fixed in their driver and this can go on, but... it's also not my
> > >>>> driver that's crashing -- they might feel differently :)
> > >>>
> > >>> As far as I can tell we only do:
> > >>>
> > >>>    _mesa_TexParameteri(target, GL_DEPTH_STENCIL_TEXTURE_MODE,
> > >>>                        GL_STENCIL_INDEX);
> > >>>
> > >>> which suppose to be the right thing to do - we select the stencil to be
> > >>> sampled instead of depth. And this won't hit the path below. I made the
> > >>> change locally and I'm now running piglit on broadwell.
> > >>>
> > >>> I noticed that _mesa_base_tex_format() is in turn used in
> > >>>
> > >>> src/mesa/drivers/common/meta_blit.c
> > >>>
> > >>> but we shouldn't go there with intel driver ever. On hardware older than
> > >>> broadwell we don't use meta and the one used on broadwell and newer
> > >>> is found in:
> > >>>
> > >>> src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> > >>>
> > >>> But lets see what piglit says.
> > >>
> > >> Right you are. This is more subtle, we will hit it when we actually create
> > >> a temporary texture out of the given read renderbuffer. It seems that this
> > >> was hit first time when formats where adjusted and then Jason added the
> > >> conditional using ARB_stencil_texturing (which is not right either).
> > >>
> > >> Really sorry that this is hindering your work now. I'll try to take a look
> > >> at this tomorrow.
> > > 
> > > So far I can't come up with other things than pure hacks. I'll explain
> > > a little what happens in the "intel stencil meta blit". Like I said, the
> > > driver creates a temporary texture out of the stencil attachment:
> > > 
> > >    const struct gl_renderbuffer_attachment *att =
> > >       &ctx->ReadBuffer->Attachment[BUFFER_STENCIL];
> > >    struct gl_renderbuffer *rb = att->Renderbuffer;
> > >    struct gl_texture_object *tex_obj;
> > > 
> > >    ...
> > >       if (!_mesa_meta_bind_rb_as_tex_image(ctx, rb, &blit->tempTex, &tex_obj,
> > >                                           target)) {
> > > 
> > > 
> > > This gets wound back to the driver, a call to
> > > intel_bind_renderbuffer_tex_image() which in turn calls the core again.
> > > 
> > >    _mesa_init_teximage_fields(ctx, image,
> > >                               rb->Width, rb->Height, 1,
> > >                               0, rb->InternalFormat, rb->Format);
> > > 
> > > Here "rb->InternalFormat" is GL_STENCIL_INDEX that won't be accepted by
> > > _mesa_base_tex_format() anymore without ARB_texture_stencil8. As most of
> > > the texture image setting up logic takes place in the core, the boolean state
> > > flag (brw_context::meta_in_progress) we have in intel driver is not much
> > > help. It looks that we would need additional driver driven overriding.
> > > But I don't like that at all.
> > 
> > On the platforms that use this path, don't we fake DEPTH_STENCIL
> > textures by having separate depth and stencil surfaces?  The implication
> > being that all of the mechanism that does stencil texturing from
> > DEPTH_STENCIL surfaces is the same as we would need to texture from
> > STENCIL_INDEX8 surfaces.
> > 
> > Wouldn't it be easier to just enable ARB_texture_stencil8 on those
> > platforms?
> 
> I'm sure you would know better than me :)

Actually, you're the expert here :)

I think that we can just turn on ARB_texture_stencil8 - I just hadn't
done the core Mesa plumbing.  Why don't we try and do that?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150407/bb838bcc/attachment.sig>