[Mesa-dev] [PATCH 01/26] i965/meta: Split conversion of color and setting it

Ben Widawsky benjamin.widawsky at intel.com
Wed Oct 19 21:29:17 UTC 2016


On 16-10-11 22:26:33, Topi Pohjolainen wrote:
>And fix a mangled comment while at it.
>
>Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>CC: Ben Widawsky <benjamin.widawsky at intel.com>
>CC: Jason Ekstrand <jason.ekstrand at intel.com>
>---
> src/mesa/drivers/dri/i965/brw_blorp.c     |  7 +++-
> src/mesa/drivers/dri/i965/brw_meta_util.c | 56 +++++++++++++++++--------------
> src/mesa/drivers/dri/i965/brw_meta_util.h | 10 ++++--
> 3 files changed, 45 insertions(+), 28 deletions(-)
>
>diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
>index b6c27982..f301192 100644
>--- a/src/mesa/drivers/dri/i965/brw_blorp.c
>+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>@@ -809,12 +809,17 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb,
>                                           brw, irb->mt);
>
>    if (can_fast_clear) {
>+      union gl_color_union override_color;
>+      brw_meta_convert_fast_clear_color(brw, irb->mt, &ctx->Color.ClearColor,
>+                                        &override_color);
>+
>       /* Record the clear color in the miptree so that it will be
>        * programmed in SURFACE_STATE by later rendering and resolve
>        * operations.
>        */
>       const bool color_updated = brw_meta_set_fast_clear_color(
>-                                    brw, irb->mt, &ctx->Color.ClearColor);
>+                                    brw, &irb->mt->gen9_fast_clear_color,
>+                                    &override_color);
>
>       /* If the buffer is already in INTEL_FAST_CLEAR_STATE_CLEAR, and the
>        * buffer isn't compressed, the clear is redundant and can be skipped.
>diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c b/src/mesa/drivers/dri/i965/brw_meta_util.c
>index 499b6ea..1badea1 100644
>--- a/src/mesa/drivers/dri/i965/brw_meta_util.c
>+++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
>@@ -372,15 +372,14 @@ brw_is_color_fast_clear_compatible(struct brw_context *brw,
> /**
>  * Convert the given color to a bitfield suitable for ORing into DWORD 7 of
>  * SURFACE_STATE (DWORD 12-15 on SKL+).
>- *
>- * Returned boolean tells if the given color differs from the stored.
>  */
>-bool
>-brw_meta_set_fast_clear_color(struct brw_context *brw,
>-                              struct intel_mipmap_tree *mt,
>-                              const union gl_color_union *color)
>+void
>+brw_meta_convert_fast_clear_color(const struct brw_context *brw,
>+                                  const struct intel_mipmap_tree *mt,
>+                                  const union gl_color_union *color,
>+                                  union gl_color_union *override_color)

I think it makes a lot more sense to return the color, my suggestion would be
(but whatever you like)...
union gl_color_union
brw_meta_convert_fast_clear_color(const struct brw_context *brw,
				  uint32_t format
				  const union gl_color_union *color)

> {
>-   union gl_color_union override_color = *color;
>+   *override_color = *color;
>
>    /* The sampler doesn't look at the format of the surface when the fast
>     * clear color is used so we need to implement luminance, intensity and
>@@ -388,55 +387,62 @@ brw_meta_set_fast_clear_color(struct brw_context *brw,
>     */
>    switch (_mesa_get_format_base_format(mt->format)) {
>    case GL_INTENSITY:
>-      override_color.ui[3] = override_color.ui[0];
>+      override_color->ui[3] = override_color->ui[0];
>       /* flow through */
>    case GL_LUMINANCE:
>    case GL_LUMINANCE_ALPHA:
>-      override_color.ui[1] = override_color.ui[0];
>-      override_color.ui[2] = override_color.ui[0];
>+      override_color->ui[1] = override_color->ui[0];
>+      override_color->ui[2] = override_color->ui[0];
>       break;
>    default:
>       for (int i = 0; i < 3; i++) {
>          if (!_mesa_format_has_color_component(mt->format, i))
>-            override_color.ui[i] = 0;
>+            override_color->ui[i] = 0;
>       }
>       break;
>    }
>
>    if (!_mesa_format_has_color_component(mt->format, 3)) {
>       if (_mesa_is_format_integer_color(mt->format))
>-         override_color.ui[3] = 1;
>+         override_color->ui[3] = 1;
>       else
>-         override_color.f[3] = 1.0f;
>+         override_color->f[3] = 1.0f;
>    }
>
>-   /* Handle linear→SRGB conversion */
>+   /* Handle linear to SRGB conversion */
>    if (brw->ctx.Color.sRGBEnabled &&
>        _mesa_get_srgb_format_linear(mt->format) != mt->format) {
>       for (int i = 0; i < 3; i++) {
>-         override_color.f[i] =
>-            util_format_linear_to_srgb_float(override_color.f[i]);
>+         override_color->f[i] =
>+            util_format_linear_to_srgb_float(override_color->f[i]);
>       }
>    }
>+}
>
>+/* Returned boolean tells if the given color differs from the current. */
>+bool
>+brw_meta_set_fast_clear_color(struct brw_context *brw,
>+                              union gl_color_union *curr_color,
>+                              const union gl_color_union *new_color)
>+{
>    bool updated;
>+
>    if (brw->gen >= 9) {
>-      updated = memcmp(&mt->gen9_fast_clear_color, &override_color,
>-                       sizeof(mt->gen9_fast_clear_color));
>-      mt->gen9_fast_clear_color = override_color;
>+      updated = memcmp(curr_color, new_color, sizeof(*curr_color));
>+      *curr_color = *new_color;
>    } else {
>-      const uint32_t old_color_value = mt->fast_clear_color_value;
>+      const uint32_t old_color_value = *(uint32_t *)curr_color;
>+      uint32_t adjusted = 0;
>
>-      mt->fast_clear_color_value = 0;
>       for (int i = 0; i < 4; i++) {
>          /* Testing for non-0 works for integer and float colors */
>-         if (override_color.f[i] != 0.0f) {
>-             mt->fast_clear_color_value |=
>-                1 << (GEN7_SURFACE_CLEAR_COLOR_SHIFT + (3 - i));
>+         if (new_color->f[i] != 0.0f) {
>+            adjusted |= 1 << (GEN7_SURFACE_CLEAR_COLOR_SHIFT + (3 - i));
>          }
>       }
>
>-      updated = (old_color_value != mt->fast_clear_color_value);
>+      updated = (old_color_value != adjusted);
>+      *(uint32_t *)curr_color = adjusted;
>    }

Pretty sure you can remove the mt->fast_clear_color_value field after this
change.

>
>    return updated;
>diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.h b/src/mesa/drivers/dri/i965/brw_meta_util.h
>index b9c4eac..c0e3100 100644
>--- a/src/mesa/drivers/dri/i965/brw_meta_util.h
>+++ b/src/mesa/drivers/dri/i965/brw_meta_util.h
>@@ -42,10 +42,16 @@ brw_meta_mirror_clip_and_scissor(const struct gl_context *ctx,
>                                  GLfloat *dstX1, GLfloat *dstY1,
>                                  bool *mirror_x, bool *mirror_y);
>
>+void
>+brw_meta_convert_fast_clear_color(const struct brw_context *brw,
>+                                  const struct intel_mipmap_tree *mt,
>+                                  const union gl_color_union *color,
>+                                  union gl_color_union *override_color);
>+
> bool
> brw_meta_set_fast_clear_color(struct brw_context *brw,
>-                              struct intel_mipmap_tree *mt,
>-                              const union gl_color_union *color);
>+                              union gl_color_union *curr_color,
>+                              const union gl_color_union *new_color);
>
> bool
> brw_is_color_fast_clear_compatible(struct brw_context *brw,

I don't see any other problems

Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

-- 
Ben Widawsky, Intel Open Source Technology Center


More information about the mesa-dev mailing list