[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:43:48 UTC 2017
On Mon, Nov 27, 2017 at 11:02 AM, Nanley Chery <nanleychery at gmail.com>
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.
>
> 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);
> + }
>
Hrm... I don't think this is quite correct. There are other places
(dst_surf here, partial slow clears, etc.) where we need a proper clear
color. I think we want this code to go in blorp_surf_for_miptree so that
it gets properly handled in all the call sites. Unfortunately, this
probably means that blorp_surf_for_miptree will have to take a view
format... and that will break blorp_copy...
Ugh. This makes me want to resurrect my entire "store it as a pixel value"
series.
We may be able to get around this issue by defining a convention (probably
the gen9 one for now) and shoving it all into ISL.
> +
> 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,
> --
> 2.15.0
>
> _______________________________________________
> 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/f0c3fa9c/attachment-0001.html>
More information about the mesa-dev
mailing list