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

[Ilia Mirkin]

On Mon, Apr 6, 2015 at 11:33 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> 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.
>
> I'll look into this some more, but any suggestions are welcome.

Can you, at a high level, explain what's going on? Is stencil stored
separately from depth? If so, you *should* support
ARB_texture_stencil8, which will solve your issues. Alternatively, did
you mean to create a GL_DEPTH_STENCIL image with StencilSampling
turned on for the object?

  -ilia