[Mesa-dev] [PATCH 5/6] i965/draw: Do resolves properly for textures used by TXF

[Iago Toral]

On Thu, 2018-01-11 at 07:38 -0800, Jason Ekstrand wrote:
> On Thu, Jan 11, 2018 at 1:48 AM, Iago Toral <itoral at igalia.com>
> wrote:
> > On Wed, 2018-01-10 at 11:22 -0800, Jason Ekstrand wrote:
> > 
> > > ---
> > 
> > >  src/mesa/drivers/dri/i965/brw_draw.c | 41
> > 
> > > ++++++++++++++++++++++++++++++++++++
> > 
> > >  1 file changed, 41 insertions(+)
> > 
> > >
> > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> > 
> > > b/src/mesa/drivers/dri/i965/brw_draw.c
> > 
> > > index 4945dec..9fd44e4 100644
> > 
> > > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > 
> > > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > 
> > > @@ -40,6 +40,7 @@
> > 
> > >  #include "swrast_setup/swrast_setup.h"
> > 
> > >  #include "drivers/common/meta.h"
> > 
> > >  #include "util/bitscan.h"
> > 
> > > +#include "util/bitset.h"
> > 
> > >  
> > 
> > >  #include "brw_blorp.h"
> > 
> > >  #include "brw_draw.h"
> > 
> > > @@ -371,6 +372,20 @@ intel_disable_rb_aux_buffer(struct
> > brw_context
> > 
> > > *brw,
> > 
> > >     return found;
> > 
> > >  }
> > 
> > >  
> > 
> > > +static void
> > 
> > > +mark_textures_used_for_txf(BITSET_WORD *used_for_txf,
> > 
> > > +                           const struct gl_program *prog)
> > 
> > > +{
> > 
> > > +   if (!prog)
> > 
> > > +      return;
> > 
> > > +
> > 
> > > +   unsigned mask = prog->SamplersUsed & prog-
> > 
> > > >info.textures_used_by_txf;
> > 
> > > +   while (mask) {
> > 
> > > +      int s = u_bit_scan(&mask);
> > 
> > > +      BITSET_SET(used_for_txf, prog->SamplerUnits[s]);
> > 
> > > +   }
> > 
> > > +}
> > 
> > > +
> > 
> > >  /**
> > 
> > >   * \brief Resolve buffers before drawing.
> > 
> > >   *
> > 
> > > @@ -386,6 +401,18 @@ brw_predraw_resolve_inputs(struct
> > brw_context
> > 
> > > *brw, bool rendering)
> > 
> > >     memset(brw->draw_aux_buffer_disabled, 0,
> > 
> > >            sizeof(brw->draw_aux_buffer_disabled));
> > 
> > >  
> > 
> > > +   BITSET_DECLARE(used_for_txf,
> > MAX_COMBINED_TEXTURE_IMAGE_UNITS);
> > 
> > > +   memset(used_for_txf, 0, sizeof(used_for_txf));
> > 
> > > +   if (rendering) {
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >VertexProgram._Current);
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >TessCtrlProgram._Current);
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >TessEvalProgram._Current);
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >GeometryProgram._Current);
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >FragmentProgram._Current);
> > 
> > > +   } else {
> > 
> > > +      mark_textures_used_for_txf(used_for_txf, ctx-
> > 
> > > >ComputeProgram._Current);
> > 
> > > +   }
> > 
> > > +
> > 
> > >     /* Resolve depth buffer and render cache of each enabled
> > texture.
> > 
> > > */
> > 
> > >     int maxEnabledUnit = ctx->Texture._MaxEnabledTexImageUnit;
> > 
> > >     for (int i = 0; i <= maxEnabledUnit; i++) {
> > 
> > > @@ -422,6 +449,20 @@ brw_predraw_resolve_inputs(struct
> > brw_context
> > 
> > > *brw, bool rendering)
> > 
> > >                                      min_layer, num_layers,
> > 
> > >                                      disable_aux);
> > 
> > >  
> > 
> > > +      /* If any programs are using it with texelFetch, we may
> > need
> > 
> > > to also do
> > 
> > > +       * a prepare with an sRGB format to ensure texelFetch
> > works
> > 
> > > "properly".
> > 
> > > +       */
> > 
> > 
> > 
> > I am not sure I understand this. The only way that txf_format and
> > 
> > view_format can be different is if the texture format is sRGB and
> > the
> > 
> > user has selected GL_SKIP_DECODE_EXT, right? If the user selected
> > that,
> > 
> > it would mean that they do not want sRGB decoded data when
> > sampling,
> > 
> > but I understand that is what they would get, since we are
> > preparing
> > 
> > the MT with the sRGB format in this case. What am I missing?
> > 
> 
> The utterly insane rules around texelFetch in
> EXT_texture_sRGB_decode. :-)  I'd quote them here but it involves a
> table so that may not come out well in e-mail.  The short version is
> that, if you use texelFetch, GL_SKIP_DECODE_EXT is ignored because
> it's part of the sampler and texelFetch doesn't use a sampler.  It's
> utterly insane but somehow that's where "design by committee" got
> us.  The fallout is that if we see a texelFetch, we need to do
> intel_miptree_prepare_texture with both sRGB and UNORM formats.

Ugh, just read the table, yeah, that not very obvious... So, the second
call to intel_miptree_prepare_texture has basically two purposes:
1. Actually ignore GL_SKIP_DECODE_EXT for texelFetch operations2. And
trigger a resolve operation if needed
Ok, that makes sense now. Thanks for the explanation.
Patches 5-6 are:Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

> > > +      if (BITSET_TEST(used_for_txf, i)) {
> > 
> > > +         enum isl_format txf_format =
> > 
> > > +            translate_tex_format(brw, tex_obj->_Format,
> > 
> > > GL_DECODE_EXT);
> > 
> > > +         if (txf_format != view_format) {
> > 
> > > +            intel_miptree_prepare_texture(brw, tex_obj->mt,
> > 
> > > txf_format,
> > 
> > > +                                          min_level, num_levels,
> > 
> > > +                                          min_layer, num_layers,
> > 
> > > +                                          disable_aux);
> > 
> > > +         }
> > 
> > > +      }
> > 
> > > +
> > 
> > >        brw_cache_flush_for_read(brw, tex_obj->mt->bo);
> > 
> > >  
> > 
> > >        if (tex_obj->base.StencilSampling ||
> > 
> > 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180112/aaeb6e21/attachment-0001.html>