[Mesa-dev] [PATCH 0.5/5] i965/gen9/fast-clear: Handle linear???SRGB conversion

[Pohjolainen, Topi]

On Wed, Nov 25, 2015 at 06:23:01PM +0100, Neil Roberts wrote:
> If GL_FRAMEBUFFER_SRGB is enabled when writing to an SRGB-capable
> framebuffer then the color will be converted from linear to SRGB
> before being written. There is no chance for the hardware to do this
> itself because it can't modify the clear color that is programmed in
> the surface state so it seems pretty clear that the driver should be
> handling this itself.
> 
> Note that this wasn't a problem before Gen9 because previously we were
> only able to do fast clears to 0 or 1 and those values are the same in
> linear and SRGB space.
> ---
> 
> I noticed this after posting the patch series but I think it belongs
> in this series because it only causes a problem once we enable fast
> clears for MSRTs. I made a test case to demonstrate this here:
> 
> http://patchwork.freedesktop.org/patch/66222/
> 
> I tried to think what other similar problems there could be such as
> whether we need to clamp the values to [0,1] for normalised formats.
> However this seems to just work so I guess the sampler hardware does
> look at the surface format enough to determine it needs to clamp the
> clear color. It's a bit odd what the sampler does and doesn't do with
> the fast clear color. I made a test case for this as well:
> 
> http://patchwork.freedesktop.org/patch/66223/
> 
> Annoyingly it doesn't clamp the value correctly for
> GL_R11F_G11F_B10F_EXT. That format is floating-point but it has no
> signed bits so I think it should clamp to a minimum of 0.
> 
> I have a github branch with all of my SKL fast clear patches here:
> 
> https://github.com/bpeel/mesa/commits/skl-fast-clear
> 
>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> index 1b2ea42..f1920b2 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> @@ -41,6 +41,8 @@
>  #include "main/api_validate.h"
>  #include "main/state.h"
>  
> +#include "util/format_srgb.h"
> +
>  #include "vbo/vbo_context.h"
>  
>  #include "drivers/common/meta.h"
> @@ -424,6 +426,15 @@ set_fast_clear_color(struct brw_context *brw,
>           override_color.f[3] = 1.0f;
>     }
>  
> +   /* Handle linear???SRGB conversion */
> +   if (brw->ctx.Color.sRGBEnabled &&
> +       _mesa_get_srgb_format_linear(mt->format) != mt->format) {

Patch five disables fast clear for single-sampled if
brw->ctx.Color.sRGBEnabled is set. How about something like this:

         /* Fast clears are disabled for single-sampled buffers when
          * sRGBEnabled is set.
          */
         assert(mt->num_samples > 1);

> +      for (int i = 0; i < 3; i++) {
> +         override_color.f[i] =
> +            util_format_linear_to_srgb_float(override_color.f[i]);
> +      }
> +   }
> +
>     if (brw->gen >= 9) {
>        mt->gen9_fast_clear_color = override_color;
>     } else {
> -- 
> 1.9.3
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev