When programming the fast clear color there was previously a chunk of
code to try to make the color match the constraints of the surface
format such as by filling in missing components and handling luminance
formats. These cases are not handled by the hardware. There are some
additional possible restrictions that the hardware does seem to
handle, such as clamping to [0,1] for normalised formats. However for
whatever reason it doesn't clamp to [0,∞] for the special float
formats that don't have a sign bit.

Rather than having special cases for all of these this patch makes it
instead convert the color to the actual surface format and back again
so that we can be sure it will have all of the possible restrictions.
Additionally this would avoid some other potential surprises such as
getting more precision for the clear color when fast clears are used.

Originally this patch was created to fix the following bug, but it is
no longer neccessary since f1fa4be871e13c68b50685aaf64. However I
think this approach is still worthwhile because it has the advantage
of reducing code redundancy.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93338
v2: Rebase and add support for converting int formats
---

This is a rebase of this patch from 2 years ago:

https://patchwork.freedesktop.org/patch/67912/

 src/mesa/drivers/dri/i965/brw_meta_util.c | 107 +++++++++++-------------------
 1 file changed, 37 insertions(+), 70 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c 
b/src/mesa/drivers/dri/i965/brw_meta_util.c
index 54dc6a5..ec727eb 100644
--- a/src/mesa/drivers/dri/i965/brw_meta_util.c
+++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
@@ -29,6 +29,8 @@
 #include "main/blend.h"
 #include "main/fbobject.h"
 #include "util/format_srgb.h"
+#include "main/format_pack.h"
+#include "main/format_unpack.h"
 
 /**
  * Helper function for handling mirror image blits.
@@ -345,6 +347,7 @@ brw_meta_convert_fast_clear_color(const struct brw_context 
*brw,
                                   const struct intel_mipmap_tree *mt,
                                   const union gl_color_union *color)
 {
+   mesa_format linear_format = _mesa_get_srgb_format_linear(mt->format);
    union isl_color_value override_color = {
       .u32 = {
          color->ui[0],
@@ -354,82 +357,46 @@ brw_meta_convert_fast_clear_color(const struct 
brw_context *brw,
       },
    };
 
-   /* 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
-    * missing components manually.
-    */
-   switch (_mesa_get_format_base_format(mt->format)) {
-   case GL_INTENSITY:
-      override_color.u32[3] = override_color.u32[0];
-      /* flow through */
-   case GL_LUMINANCE:
-   case GL_LUMINANCE_ALPHA:
-      override_color.u32[1] = override_color.u32[0];
-      override_color.u32[2] = override_color.u32[0];
-      break;
-   default:
-      for (int i = 0; i < 3; i++) {
-         if (!_mesa_format_has_color_component(mt->format, i))
-            override_color.u32[i] = 0;
-      }
-      break;
-   }
-
-   switch (_mesa_get_format_datatype(mt->format)) {
-   case GL_UNSIGNED_NORMALIZED:
-      for (int i = 0; i < 4; i++)
-         override_color.f32[i] = CLAMP(override_color.f32[i], 0.0f, 1.0f);
-      break;
-
-   case GL_SIGNED_NORMALIZED:
-      for (int i = 0; i < 4; i++)
-         override_color.f32[i] = CLAMP(override_color.f32[i], -1.0f, 1.0f);
-      break;
-
-   case GL_UNSIGNED_INT:
-      for (int i = 0; i < 4; i++) {
-         unsigned bits = _mesa_get_format_bits(mt->format, GL_RED_BITS + i);
-         if (bits < 32) {
-            uint32_t max = (1u << bits) - 1;
-            override_color.u32[i] = MIN2(override_color.u32[i], max);
-         }
-      }
-      break;
-
-   case GL_INT:
-      for (int i = 0; i < 4; i++) {
-         unsigned bits = _mesa_get_format_bits(mt->format, GL_RED_BITS + i);
-         if (bits < 32) {
-            int32_t max = (1 << (bits - 1)) - 1;
-            int32_t min = -(1 << (bits - 1));
-            override_color.i32[i] = CLAMP(override_color.i32[i], min, max);
-         }
-      }
-      break;
-
-   case GL_FLOAT:
-      if (!_mesa_is_format_signed(mt->format)) {
-         for (int i = 0; i < 4; i++)
-            override_color.f32[i] = MAX2(override_color.f32[i], 0.0f);
-      }
-      break;
-   }
-
-   if (!_mesa_format_has_color_component(mt->format, 3)) {
-      if (_mesa_is_format_integer_color(mt->format))
-         override_color.u32[3] = 1;
-      else
-         override_color.f32[3] = 1.0f;
-   }
-
    /* Handle linear to SRGB conversion */
-   if (brw->ctx.Color.sRGBEnabled &&
-       _mesa_get_srgb_format_linear(mt->format) != mt->format) {
+   if (brw->ctx.Color.sRGBEnabled && linear_format != mt->format) {
       for (int i = 0; i < 3; i++) {
          override_color.f32[i] =
             util_format_linear_to_srgb_float(override_color.f32[i]);
       }
    }
 
+   union gl_color_union tmp_color;
+
+   /* Convert the clear color to the surface format and back so that the color
+    * returned when sampling is guaranteed to be a value that could be stored
+    * in the surface. For example if the surface is a luminance format and we
+    * clear to 0.5,0.75,0.1,0.2 we want the color to come back as
+    * 0.5,0.5,0.5,1.0. In general the hardware doesn't seem to look at the
+    * surface format when returning the clear color so we need to do this to
+    * implement luminance, intensity and missing components. However it does
+    * seem to look at it in some cases such as to clamp to the range [0,1] for
+    * unorm formats. Suprisingly however it doesn't clamp to [0,∞] for the
+    * special float formats that don't have a sign bit.
+    */
+   if (_mesa_is_format_integer_color(linear_format)) {
+      _mesa_pack_uint_rgba_row(linear_format,
+                               1, /* n_pixels */
+                               (const GLuint (*)[4]) override_color.u32,
+                               &tmp_color);
+      _mesa_unpack_uint_rgba_row(linear_format,
+                                 1, /* n_pixels */
+                                 &tmp_color,
+                                 (GLuint (*)[4]) override_color.u32);
+   } else {
+      _mesa_pack_float_rgba_row(linear_format,
+                                1, /* n_pixels */
+                                (const GLfloat (*)[4]) override_color.f32,
+                                &tmp_color);
+      _mesa_unpack_rgba_row(linear_format,
+                            1, /* n_pixels */
+                            &tmp_color,
+                            (GLfloat (*)[4]) override_color.f32);
+   }
+
    return override_color;
 }
-- 
2.9.5

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to