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

Jason Ekstrand jason at jlekstrand.net
Mon Nov 27 19:45:24 UTC 2017


On Mon, Nov 27, 2017 at 11:37 AM, Roland Scheidegger <sroland at vmware.com>
wrote:

> 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...
>

I think we can end up hitting that problem.  However, this patch doesn't
really make things any worse.  I've considered a time or two exactly how we
can fix that problem properly and it's going to take a bit of work.


> 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,
> >
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171127/836eb019/attachment-0001.html>


More information about the mesa-dev mailing list