<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sun, Jul 22, 2018 at 1:23 PM Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 21/07/18 07:12, Jason Ekstrand wrote:<br>
> The first fix attempt contained a nasty typo which somehow didn't get<br>
> caught in review.  It also didn't work as intended because the sRGB<br>
> conversion was happening but then throwing away all but the red channel<br>
> because it dind't know it was RGB.  Really, it's my fault for trying to<br>
> fix a bug without first writing tests.  I've now written tests and they<br>
> pass with this change. :)<br>
><br>
> Fixes: 7adb1986808c "intel/blorp: Fix blits to R8G8B8_UNORM_SRGB"<br>
<br>
I couldn't find this hash. Are you referring to commit <br>
11712b9ca17e4e1a819dcb7d020e19c6da77bc90 ?<br></blockquote><div><br></div><div>Yes, that one.  Fixed locally.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Otherwise looks good.<br></blockquote><div><br></div><div>Can I take that as a review?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-<br>
Lionel<br>
<br>
> ---<br>
>   src/intel/blorp/blorp_blit.c | 14 +++++++++++---<br>
>   1 file changed, 11 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c<br>
> index 3b4f42a59b6..561897894c3 100644<br>
> --- a/src/intel/blorp/blorp_blit.c<br>
> +++ b/src/intel/blorp/blorp_blit.c<br>
> @@ -993,6 +993,8 @@ convert_color(struct nir_builder *b, nir_ssa_def *color,<br>
>         value = nir_fmul(b, nir_fsat(b, nir_channel(b, color, 0)),<br>
>                             nir_imm_float(b, factor));<br>
>      } else if (key->dst_format == ISL_FORMAT_L8_UNORM_SRGB) {<br>
> +      value = nir_format_linear_to_srgb(b, nir_channel(b, color, 0));<br>
> +   } else if (key->dst_format == ISL_FORMAT_R8G8B8_UNORM_SRGB) {<br>
>         value = nir_format_linear_to_srgb(b, color);<br>
>      } else if (key->dst_format == ISL_FORMAT_R9G9B9E5_SHAREDEXP) {<br>
>         value = nir_format_pack_r9g9b9e5(b, color);<br>
> @@ -1000,8 +1002,14 @@ convert_color(struct nir_builder *b, nir_ssa_def *color,<br>
>         unreachable("Unsupported format conversion");<br>
>      }<br>
>   <br>
> -   nir_ssa_def *u = nir_ssa_undef(b, 1, 32);<br>
> -   return nir_vec4(b, value, u, u, u);<br>
> +   nir_ssa_def *out_comps[4];<br>
> +   for (unsigned i = 0; i < 4; i++) {<br>
> +      if (i < value->num_components)<br>
> +         out_comps[i] = nir_channel(b, value, i);<br>
> +      else<br>
> +         out_comps[i] = nir_ssa_undef(b, 1, 32);<br>
> +   }<br>
> +   return nir_vec(b, out_comps, 4);<br>
>   }<br>
>   <br>
>   /**<br>
> @@ -1959,7 +1967,7 @@ try_blorp_blit(struct blorp_batch *batch,<br>
>   <br>
>         /* If it happens to be sRGB, we need to force a conversion */<br>
>         if (params->dst.view.format == ISL_FORMAT_R8G8B8_UNORM_SRGB)<br>
> -         wm_prog_key->dst_format = ISL_FORMAT_R9G9B9E5_SHAREDEXP;<br>
> +         wm_prog_key->dst_format = ISL_FORMAT_R8G8B8_UNORM_SRGB;<br>
>   <br>
>         surf_fake_rgb_with_red(batch->blorp->isl_dev, &params->dst);<br>
>   <br>
<br>
<br>
</blockquote></div></div>