Bug 1703402 - Clamp RGB components after YUV conversion for SWGL blen…

…ding. r=jrmuizel This expands on an earlier fix from bug 1698009. It turns out we can occasionally find YUV values which can still produce negative RGB values if only Y is clamped. The final solution to this is just to clamp the output RGB values rather than input YUV values. Since this is only used when we fall off the SWGL fast-paths (which properly handle this clamping already), the performance impact of the extra clamping should be negligible. Differential Revision: https://phabricator.services.mozilla.com/D111032 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/577145ad8319474cb2d34f4048364df1efe5e804
servo · Apr 7, 2021 · 1352307 · 1352307
1 parent 4276d4e
commit 1352307
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 6 deletions.
diff --git a/glsl-to-cxx/src/hir.rs b/glsl-to-cxx/src/hir.rs
@@ -3264,13 +3264,27 @@ pub fn ast_to_hir(state: &mut State, tu: &syntax::TranslationUnit) -> Translatio
         Type::new(Vec2),
         vec![Type::new(Vec2), Type::new(Vec2)],
     );
+    declare_function(
+        state,
+        "min",
+        None,
+        Type::new(Vec2),
+        vec![Type::new(Vec2), Type::new(Float)],
+    );
     declare_function(
         state,
         "min",
         None,
         Type::new(Vec3),
         vec![Type::new(Vec3), Type::new(Vec3)],
     );
+    declare_function(
+        state,
+        "min",
+        None,
+        Type::new(Vec3),
+        vec![Type::new(Vec3), Type::new(Float)],
+    );
 
     declare_function(
         state,
@@ -3300,6 +3314,13 @@ pub fn ast_to_hir(state: &mut State, tu: &syntax::TranslationUnit) -> Translatio
         Type::new(Vec3),
         vec![Type::new(Vec3), Type::new(Vec3)],
     );
+    declare_function(
+        state,
+        "max",
+        None,
+        Type::new(Vec3),
+        vec![Type::new(Vec3), Type::new(Float)],
+    );
 
     declare_function(
         state,

diff --git a/swgl/src/glsl.h b/swgl/src/glsl.h
@@ -1612,10 +1612,19 @@ vec3_scalar step(vec3_scalar edge, vec3_scalar x) {
 SI vec3 min(vec3 a, vec3 b) {
   return vec3(min(a.x, b.x), min(a.y, b.y), min(a.z, b.z));
 }
+SI vec3 min(vec3 a, Float b) {
+  return vec3(min(a.x, b), min(a.y, b), min(a.z, b));
+}
+SI vec3_scalar min(vec3_scalar a, vec3_scalar b) {
+  return vec3_scalar{min(a.x, b.x), min(a.y, b.y), min(a.z, b.z)};
+}
+
 SI vec3 max(vec3 a, vec3 b) {
   return vec3(max(a.x, b.x), max(a.y, b.y), max(a.z, b.z));
 }
-
+SI vec3 max(vec3 a, Float b) {
+  return vec3(max(a.x, b), max(a.y, b), max(a.z, b));
+}
 SI vec3_scalar max(vec3_scalar a, vec3_scalar b) {
   return vec3_scalar{max(a.x, b.x), max(a.y, b.y), max(a.z, b.z)};
 }

diff --git a/webrender/res/yuv.glsl b/webrender/res/yuv.glsl
@@ -167,12 +167,12 @@ vec4 sample_yuv(
 
     // See the YuvColorMatrix definition for an explanation of where the constants come from.
     vec3 yuv = yuv_value * coefficient - yuv_offset_vector;
-    #ifdef WR_FEATURE_ALPHA_PASS
-        // Avoid negative Y values that can mess with blending. These occur due to invalid Y
-        // values outside the mappable space that never the less can be generated.
-        yuv.x = max(yuv.x, 0.0);
-    #endif
     vec3 rgb = yuv_color_matrix * yuv;
+    #if defined(WR_FEATURE_ALPHA_PASS) && defined(SWGL_CLIP_MASK)
+        // Avoid negative RGB values that can mess with blending. These occur due to invalid
+        // YUV values outside the mappable space that never the less can be generated.
+        rgb = max(rgb, 0.0);
+    #endif
     vec4 color = vec4(rgb, 1.0);
 
     return color;