[Mesa-dev] [PATCH 2/3] nir/lower-tex: add srgb->linear lowering

[Jason Ekstrand]

On Tue, Apr 19, 2016 at 10:15 AM, Rob Clark <robdclark at gmail.com> wrote:

> On Tue, Apr 19, 2016 at 1:06 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Tue, Apr 19, 2016 at 6:58 AM, Rob Clark <robdclark at gmail.com> wrote:
> >>
> >> From: Rob Clark <robclark at freedesktop.org>
> >>
> >> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> >> ---
> >>  src/compiler/nir/nir.h           |  7 ++++++
> >>  src/compiler/nir/nir_lower_tex.c | 46
> >> ++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 53 insertions(+)
> >>
> >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> index bbbc208..2c0f705 100644
> >> --- a/src/compiler/nir/nir.h
> >> +++ b/src/compiler/nir/nir.h
> >> @@ -2301,6 +2301,13 @@ typedef struct nir_lower_tex_options {
> >>      * while 4 and 5 represent 0 and 1 respectively.
> >>      */
> >>     uint8_t swizzles[32][4];
> >> +
> >> +   /**
> >> +    * Bitmap of textures that need srgb to linear conversion.  If
> >> +    * (srgb_to_linear & (1 << texture_index)) then the rgb (xyz)
> >> +    * components of the texture are lowered to linear.
> >> +    */
> >> +   unsigned lower_srgb;
> >
> >
> > You have lower_srgb here and srgb_to_linear in the comment.  Please pick
> > one.  I think I prefer srgb_to_linear as it's more explicit.
>
> oh, forgot to update the comment.. started as srgb_to_linear until I
> realized that lower_srgb was same # of chars as saturate_s/t/r and
> made a bunch of code line up neatly
>
> (ok, perhaps not the best reason.. but a reason.. :-P)
>

Wow, that's sad...  I guess I don't care too much beyond having it well
documented, but I do thing srgb_to_linear is more self-documenting.  Do
what you'd like.


> >>
> >>  } nir_lower_tex_options;
> >>
> >>  bool nir_lower_tex(nir_shader *shader,
> >> diff --git a/src/compiler/nir/nir_lower_tex.c
> >> b/src/compiler/nir/nir_lower_tex.c
> >> index 7740e58..6bae3b7 100644
> >> --- a/src/compiler/nir/nir_lower_tex.c
> >> +++ b/src/compiler/nir/nir_lower_tex.c
> >> @@ -275,6 +275,45 @@ swizzle_result(nir_builder *b, nir_tex_instr *tex,
> >> const uint8_t swizzle[4])
> >>                                    swizzled->parent_instr);
> >>  }
> >>
> >> +static void
> >> +linearize_srgb_result(nir_builder *b, nir_tex_instr *tex)
> >> +{
> >> +   assert(tex->dest.is_ssa);
> >> +   assert(nir_tex_instr_dest_size(tex) == 4);
> >> +   assert(nir_alu_type_get_base_type(tex->dest_type) ==
> nir_type_float);
> >> +
> >> +   b->cursor = nir_after_instr(&tex->instr);
> >> +
> >> +   nir_ssa_def *components[4];
> >> +
> >> +   /* first three channels are rgb: */
> >> +   for (unsigned i = 0; i < 3; i++) {
> >
> >
> > Um... NIR is a vector IR.  Why are we looping over channels?  Just do a
> > nir_swizzle to pick off the first three, do the sRGB conversion, and
> nir_vec
> > them back together with the alpha channel.
>
> oh, hmm, yeah.. used to thinking in scalar
>
> BR,
> -R
>
>
> >>
> >> +      /* Formula is:
> >> +       *    (comp <= 0.04045) ?
> >> +       *          (comp / 12.92) :
> >> +       *          pow((comp + 0.055) / 1.055, 2.4)
> >> +       */
> >> +      nir_ssa_def *comp = nir_channel(b, &tex->dest.ssa, i);
> >> +      nir_ssa_def *low  = nir_fmul(b, comp, nir_imm_float(b, 1.0 /
> >> 12.92));
> >>
> >> +      nir_ssa_def *high = nir_fpow(b,
> >> +                                   nir_fmul(b,
> >> +                                            nir_fadd(b,
> >> +                                                     comp,
> >> +                                                     nir_imm_float(b,
> >> 0.055)),
> >> +                                            nir_imm_float(b, 1.0 /
> >> 1.055)),
> >> +                                   nir_imm_float(b, 2.4));
> >> +      nir_ssa_def *cond = nir_fge(b, nir_imm_float(b, 0.04045), comp);
> >> +      components[i] = nir_bcsel(b, cond, low, high);
> >> +   }
> >> +
> >> +   /* alpha is untouched: */
> >> +   components[3] = nir_channel(b, &tex->dest.ssa, 3);
> >> +
> >> +   nir_ssa_def *linearized = nir_vec(b, components, 4);
> >> +   nir_ssa_def_rewrite_uses_after(&tex->dest.ssa,
> >> nir_src_for_ssa(linearized),
> >> +                                  linearized->parent_instr);
> >> +}
> >> +
> >>  static bool
> >>  nir_lower_tex_block(nir_block *block, void *void_state)
> >>  {
> >> @@ -323,6 +362,13 @@ nir_lower_tex_block(nir_block *block, void
> >> *void_state)
> >>           swizzle_result(b, tex, options->swizzles[tex->texture_index]);
> >>           state->progress = true;
> >>        }
> >> +
> >> +      /* should be after swizzle so we know which channels are rgb: */
> >> +      if (((1 << tex->texture_index) & options->lower_srgb) &&
> >> +          !nir_tex_instr_is_query(tex) && !tex->is_shadow) {
> >> +         linearize_srgb_result(b, tex);
> >> +         state->progress = true;
> >> +      }
> >>     }
> >>
> >>     return true;
> >> --
> >> 2.5.5
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160419/778006ff/attachment-0001.html>