On Mon, Apr 06, 2015 at 12:05:56PM -0400, Ilia Mirkin wrote: > On Mon, Apr 6, 2015 at 11:33 AM, Pohjolainen, Topi > <topi.pohjolai...@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?
Stencil is stored separately and I agree that supporting ARB_texture_stencil8 is the right solution of course. I was in the impression that we are missing things in our driver preventing it simply to be turned on. And therfore I was looking for something simple we could use in the meantime. I could be wrong though, I just don't know all the implications turning on ARB_texture_stencil8 has. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev