<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 19, 2016 at 10:15 AM, Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Apr 19, 2016 at 1:06 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
><br>
> On Tue, Apr 19, 2016 at 6:58 AM, Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> wrote:<br>
>><br>
>> From: Rob Clark <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>><br>
>><br>
>> Signed-off-by: Rob Clark <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>><br>
>> ---<br>
>>  src/compiler/nir/nir.h           |  7 ++++++<br>
>>  src/compiler/nir/nir_lower_tex.c | 46<br>
>> ++++++++++++++++++++++++++++++++++++++++<br>
>>  2 files changed, 53 insertions(+)<br>
>><br>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h<br>
>> index bbbc208..2c0f705 100644<br>
>> --- a/src/compiler/nir/nir.h<br>
>> +++ b/src/compiler/nir/nir.h<br>
>> @@ -2301,6 +2301,13 @@ typedef struct nir_lower_tex_options {<br>
>>      * while 4 and 5 represent 0 and 1 respectively.<br>
>>      */<br>
>>     uint8_t swizzles[32][4];<br>
>> +<br>
>> +   /**<br>
>> +    * Bitmap of textures that need srgb to linear conversion.  If<br>
>> +    * (srgb_to_linear & (1 << texture_index)) then the rgb (xyz)<br>
>> +    * components of the texture are lowered to linear.<br>
>> +    */<br>
>> +   unsigned lower_srgb;<br>
><br>
><br>
> You have lower_srgb here and srgb_to_linear in the comment.  Please pick<br>
> one.  I think I prefer srgb_to_linear as it's more explicit.<br>
<br>
</div></div>oh, forgot to update the comment.. started as srgb_to_linear until I<br>
realized that lower_srgb was same # of chars as saturate_s/t/r and<br>
made a bunch of code line up neatly<br>
<br>
(ok, perhaps not the best reason.. but a reason.. :-P)<br><div><div class="h5"></div></div></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
>><br>
>>  } nir_lower_tex_options;<br>
>><br>
>>  bool nir_lower_tex(nir_shader *shader,<br>
>> diff --git a/src/compiler/nir/nir_lower_tex.c<br>
>> b/src/compiler/nir/nir_lower_tex.c<br>
>> index 7740e58..6bae3b7 100644<br>
>> --- a/src/compiler/nir/nir_lower_tex.c<br>
>> +++ b/src/compiler/nir/nir_lower_tex.c<br>
>> @@ -275,6 +275,45 @@ swizzle_result(nir_builder *b, nir_tex_instr *tex,<br>
>> const uint8_t swizzle[4])<br>
>>                                    swizzled->parent_instr);<br>
>>  }<br>
>><br>
>> +static void<br>
>> +linearize_srgb_result(nir_builder *b, nir_tex_instr *tex)<br>
>> +{<br>
>> +   assert(tex->dest.is_ssa);<br>
>> +   assert(nir_tex_instr_dest_size(tex) == 4);<br>
>> +   assert(nir_alu_type_get_base_type(tex->dest_type) == nir_type_float);<br>
>> +<br>
>> +   b->cursor = nir_after_instr(&tex->instr);<br>
>> +<br>
>> +   nir_ssa_def *components[4];<br>
>> +<br>
>> +   /* first three channels are rgb: */<br>
>> +   for (unsigned i = 0; i < 3; i++) {<br>
><br>
><br>
> Um... NIR is a vector IR.  Why are we looping over channels?  Just do a<br>
> nir_swizzle to pick off the first three, do the sRGB conversion, and nir_vec<br>
> them back together with the alpha channel.<br>
<br>
</div></div>oh, hmm, yeah.. used to thinking in scalar<br>
<br>
BR,<br>
-R<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
>><br>
>> +      /* Formula is:<br>
>> +       *    (comp <= 0.04045) ?<br>
>> +       *          (comp / 12.92) :<br>
>> +       *          pow((comp + 0.055) / 1.055, 2.4)<br>
>> +       */<br>
>> +      nir_ssa_def *comp = nir_channel(b, &tex->dest.ssa, i);<br>
>> +      nir_ssa_def *low  = nir_fmul(b, comp, nir_imm_float(b, 1.0 /<br>
>> 12.92));<br>
>><br>
>> +      nir_ssa_def *high = nir_fpow(b,<br>
>> +                                   nir_fmul(b,<br>
>> +                                            nir_fadd(b,<br>
>> +                                                     comp,<br>
>> +                                                     nir_imm_float(b,<br>
>> 0.055)),<br>
>> +                                            nir_imm_float(b, 1.0 /<br>
>> 1.055)),<br>
>> +                                   nir_imm_float(b, 2.4));<br>
>> +      nir_ssa_def *cond = nir_fge(b, nir_imm_float(b, 0.04045), comp);<br>
>> +      components[i] = nir_bcsel(b, cond, low, high);<br>
>> +   }<br>
>> +<br>
>> +   /* alpha is untouched: */<br>
>> +   components[3] = nir_channel(b, &tex->dest.ssa, 3);<br>
>> +<br>
>> +   nir_ssa_def *linearized = nir_vec(b, components, 4);<br>
>> +   nir_ssa_def_rewrite_uses_after(&tex->dest.ssa,<br>
>> nir_src_for_ssa(linearized),<br>
>> +                                  linearized->parent_instr);<br>
>> +}<br>
>> +<br>
>>  static bool<br>
>>  nir_lower_tex_block(nir_block *block, void *void_state)<br>
>>  {<br>
>> @@ -323,6 +362,13 @@ nir_lower_tex_block(nir_block *block, void<br>
>> *void_state)<br>
>>           swizzle_result(b, tex, options->swizzles[tex->texture_index]);<br>
>>           state->progress = true;<br>
>>        }<br>
>> +<br>
>> +      /* should be after swizzle so we know which channels are rgb: */<br>
>> +      if (((1 << tex->texture_index) & options->lower_srgb) &&<br>
>> +          !nir_tex_instr_is_query(tex) && !tex->is_shadow) {<br>
>> +         linearize_srgb_result(b, tex);<br>
>> +         state->progress = true;<br>
>> +      }<br>
>>     }<br>
>><br>
>>     return true;<br>
>> --<br>
>> 2.5.5<br>
>><br>
><br>
</div></div></blockquote></div><br></div></div>