[Mesa-dev] [PATCH 2/2] i965/cnl: Correctly sample from fast-cleared sRGB textures

[Nanley Chery]

On Mon, Nov 20, 2017 at 03:48:54PM -0800, Nanley Chery wrote:
> On Mon, Nov 20, 2017 at 02:55:28PM -0800, Jason Ekstrand wrote:
> > On Mon, Nov 20, 2017 at 2:10 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> > 
> > > On Mon, Nov 20, 2017 at 01:54:39PM -0800, Nanley Chery wrote:
> > > > When sampling from fast-cleared sRGB textures on gen10, the hardware
> > > > will not decode the clear color to linear. We must therefore do it in
> > > > software.
> > > >
> > >
> > > I should've included the tests that are now passing (added in locally):
> > >
> > > * spec at arb_framebuffer_srgb@arb_framebuffer_srgb-fast-clear-blend
> > > * spec at arb_framebuffer_srgb@fbo-fast-clear
> > > * spec at arb_framebuffer_srgb@msaa-fast-clear
> > > * spec at ext_texture_srgb@fbo-fast-clear
> > > * spec at ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb
> > >
> > > -Nanley
> > >
> > > > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_blorp.c            |  7 +++++
> > > >  src/mesa/drivers/dri/i965/brw_meta_util.c        | 38
> > > ++++++++++++++++++++++++
> > > >  src/mesa/drivers/dri/i965/brw_meta_util.h        |  5 ++++
> > > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  9 +++++-
> > > >  4 files changed, 58 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > index 38284d3b85..b10e9b95b7 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > > > @@ -328,6 +328,13 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
> > > >     struct blorp_surf src_surf, dst_surf;
> > > >     blorp_surf_for_miptree(brw, &src_surf, src_mt, src_aux_usage, false,
> > > >                            &src_level, src_layer, 1, &tmp_surfs[0]);
> > > > +   if (devinfo->gen >= 10 && isl_format_is_srgb(src_isl_format) &&
> > > > +       src_clear_supported) {
> > > > +      src_surf.clear_color =
> > > > +         gen10_convert_srgb_fast_clear_color(devinfo, src_isl_format,
> > > > +                                             src_mt->fast_clear_color);
> > > > +   }
> > > > +
> > > >     blorp_surf_for_miptree(brw, &dst_surf, dst_mt, dst_aux_usage, true,
> > > >                            &dst_level, dst_layer, 1, &tmp_surfs[1]);
> > > >
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > > index d292f5a8e2..97c6114f8a 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > > > @@ -28,6 +28,7 @@
> > > >  #include "brw_state.h"
> > > >  #include "main/blend.h"
> > > >  #include "main/fbobject.h"
> > > > +#include "main/format_utils.h"
> > > >  #include "util/format_srgb.h"
> > > >
> > > >  /**
> > > > @@ -315,6 +316,43 @@ brw_is_color_fast_clear_compatible(struct
> > > brw_context *brw,
> > > >     return true;
> > > >  }
> > > >
> > > > +/* When sampling from fast-cleared sRGB textures on gen10, the hardware
> > > > + * will not decode the clear color to linear. We must therefore do it in
> > > > + * software.
> > > > + */
> > > > +union isl_color_value
> > > > +gen10_convert_srgb_fast_clear_color(const struct gen_device_info
> > > *devinfo,
> > > > +                                    enum isl_format format,
> > > > +                                    const union isl_color_value color)
> > > > +{
> > > > +   assert(devinfo->gen >= 10);
> > > > +   assert(isl_format_is_srgb(format));
> > > > +   assert(isl_format_supports_ccs_d(devinfo, format));
> > > > +
> > > > +   /* All the CCS-supported sRGB textures in GL are 4-channel,
> > > 8-bit/channel,
> > > > +    * UNORM formats.
> > > > +    */
> > > > +   assert(isl_format_get_num_channels(format) == 4);
> > > > +   assert(isl_format_channels_have_size(format, 8));
> > > > +   assert(isl_format_has_unorm_channel(format));
> > >
> > 
> > There are exactly 4 ISL formats which match this.  You could just assert
> > that it's one of the four and drop the other 5 asserts.  It doesn't really
> > matter, but this is rather complicated.
> > 
> > 
> 
> I agree. I'll fix this locally and send out a v2 after getting more of
> your feedback.
> 

Just a note: after looking things over, I was surprised to find that
only 3 formats match the above (R8G8B8X8_UNORM_SRGB isn't included).

-Nanley