Re: [Mesa-dev] [PATCH] intel/blorp: Fix blits to R8G8B8_UNORM_SRGB sRGB harder

[Lionel Landwerlin]

On 22/07/18 22:45, Jason Ekstrand wrote:

On Sun, Jul 22, 2018 at 1:23 PM Lionel Landwerlin <lionel.g.landwer...@intel.com <mailto:lionel.g.landwer...@intel.com>> wrote:

    On 21/07/18 07:12, Jason Ekstrand wrote:
    > The first fix attempt contained a nasty typo which somehow
    didn't get
    > caught in review.  It also didn't work as intended because the sRGB
    > conversion was happening but then throwing away all but the red
    channel
    > because it dind't know it was RGB.  Really, it's my fault for
    trying to
    > fix a bug without first writing tests.  I've now written tests
    and they
    > pass with this change. :)
    >
    > Fixes: 7adb1986808c "intel/blorp: Fix blits to R8G8B8_UNORM_SRGB"

    I couldn't find this hash. Are you referring to commit
    11712b9ca17e4e1a819dcb7d020e19c6da77bc90 ?


Yes, that one.  Fixed locally.

    Otherwise looks good.


Can I take that as a review?

Yes!

    -
    Lionel

    > ---
    >   src/intel/blorp/blorp_blit.c | 14 +++++++++++---
    >   1 file changed, 11 insertions(+), 3 deletions(-)
    >
    > diff --git a/src/intel/blorp/blorp_blit.c
    b/src/intel/blorp/blorp_blit.c
    > index 3b4f42a59b6..561897894c3 100644
    > --- a/src/intel/blorp/blorp_blit.c
    > +++ b/src/intel/blorp/blorp_blit.c
    > @@ -993,6 +993,8 @@ convert_color(struct nir_builder *b,
    nir_ssa_def *color,
    >         value = nir_fmul(b, nir_fsat(b, nir_channel(b, color, 0)),
    >                             nir_imm_float(b, factor));
    >      } else if (key->dst_format == ISL_FORMAT_L8_UNORM_SRGB) {
    > +      value = nir_format_linear_to_srgb(b, nir_channel(b,
    color, 0));
    > +   } else if (key->dst_format == ISL_FORMAT_R8G8B8_UNORM_SRGB) {
    >         value = nir_format_linear_to_srgb(b, color);
    >      } else if (key->dst_format == ISL_FORMAT_R9G9B9E5_SHAREDEXP) {
    >         value = nir_format_pack_r9g9b9e5(b, color);
    > @@ -1000,8 +1002,14 @@ convert_color(struct nir_builder *b,
    nir_ssa_def *color,
    >         unreachable("Unsupported format conversion");
    >      }
    >
    > -   nir_ssa_def *u = nir_ssa_undef(b, 1, 32);
    > -   return nir_vec4(b, value, u, u, u);
    > +   nir_ssa_def *out_comps[4];
    > +   for (unsigned i = 0; i < 4; i++) {
    > +      if (i < value->num_components)
    > +         out_comps[i] = nir_channel(b, value, i);
    > +      else
    > +         out_comps[i] = nir_ssa_undef(b, 1, 32);
    > +   }
    > +   return nir_vec(b, out_comps, 4);
    >   }
    >
    >   /**
    > @@ -1959,7 +1967,7 @@ try_blorp_blit(struct blorp_batch *batch,
    >
    >         /* If it happens to be sRGB, we need to force a
    conversion */
    >         if (params->dst.view.format == ISL_FORMAT_R8G8B8_UNORM_SRGB)
    > -         wm_prog_key->dst_format = ISL_FORMAT_R9G9B9E5_SHAREDEXP;
    > +         wm_prog_key->dst_format = ISL_FORMAT_R8G8B8_UNORM_SRGB;
    >
    >  surf_fake_rgb_with_red(batch->blorp->isl_dev, &params->dst);
    >

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev