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

[Roland Scheidegger]

If you use a float input color, does it mean it isn't quantized?
I'm just wondering if you actually get (slightly) different values with
sampling from fast-cleared surfaces than you'd get when you'd have
cleared the surface normally.
In any case, patch 1/2 of this series looks alright (I was just
wondering why you needed it...), can't really review this one...

Roland


Am 27.11.2017 um 20:02 schrieb Nanley Chery:
> 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.
> 
> This makes the following piglit tests pass:
> * 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
> 
> v2 (Jason Ekstrand):
> - Update some comments.
> - Simplify assertions.
> - Use an sRGB-to-linear helper.
> 
> 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        | 26 ++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_meta_util.h        |  5 +++++
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  9 +++++++-
>  4 files changed, 46 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..d1ce9d46ea 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> @@ -315,6 +315,32 @@ 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)
> +{
> +   /* Gen10 supports fast-clearing three of GL's sRGB formats. */
> +   assert(devinfo->gen >= 10);
> +   assert(format == ISL_FORMAT_R8G8B8A8_UNORM_SRGB ||
> +          format == ISL_FORMAT_B8G8R8A8_UNORM_SRGB ||
> +          format == ISL_FORMAT_B8G8R8X8_UNORM_SRGB);
> +
> +   /* According to do_single_blorp_clear(), fast-clears for texture-views are
> +    * disabled. This means that we only have to do channel-to-channel format
> +    * conversions.
> +    */
> +   union isl_color_value override_color = color;
> +   for (unsigned i = 0; i < 3; i++)
> +      override_color.f32[i] = util_format_srgb_to_linear_float(color.f32[i]);
> +
> +   return override_color;
> +}
> +
>  /**
>   * Convert the given color to a bitfield suitable for ORing into DWORD 7 of
>   * SURFACE_STATE (DWORD 12-15 on SKL+).
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.h b/src/mesa/drivers/dri/i965/brw_meta_util.h
> index 4b3408df15..ee0b3bd3e1 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_util.h
> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.h
> @@ -42,6 +42,11 @@ brw_meta_mirror_clip_and_scissor(const struct gl_context *ctx,
>                                   GLfloat *dstX1, GLfloat *dstY1,
>                                   bool *mirror_x, bool *mirror_y);
>  
> +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);
> +
>  union isl_color_value
>  brw_meta_convert_fast_clear_color(const struct brw_context *brw,
>                                    const struct intel_mipmap_tree *mt,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index adf60a840b..315b96e9c4 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -51,6 +51,7 @@
>  #include "intel_buffer_objects.h"
>  
>  #include "brw_context.h"
> +#include "brw_meta_util.h"
>  #include "brw_state.h"
>  #include "brw_defines.h"
>  #include "brw_wm.h"
> @@ -174,7 +175,13 @@ brw_emit_surface_state(struct brw_context *brw,
>        /* We only really need a clear color if we also have an auxiliary
>         * surface.  Without one, it does nothing.
>         */
> -      clear_color = mt->fast_clear_color;
> +      if (devinfo->gen >= 10 && isl_format_is_srgb(view.format)) {
> +         clear_color =
> +            gen10_convert_srgb_fast_clear_color(devinfo, view.format,
> +                                                mt->fast_clear_color);
> +      } else {
> +         clear_color = mt->fast_clear_color;
> +      }
>     }
>  
>     void *state = brw_state_batch(brw,
>