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

[Rob Clark]

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)

>>
>>  } 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
>>
>