Bug 1607746 - Part 2: Clean up passing filter mode to shader r=nical
authorConnor Brewster <connorbrewster@yahoo.com>
Mon, 13 Jan 2020 16:05:59 +0000
changeset 509997 5cd63d35010f9bf9538a37fb330127d459b7ccc0
parent 509996 416ad9bef3910a000af9fb1ccfbe92258d5521b9
child 509998 d01d4ff45ce8604f97146811310d80113af99d92
push id37012
push userapavel@mozilla.com
push dateTue, 14 Jan 2020 03:45:02 +0000
treeherdermozilla-central@d1406439c461 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1607746
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1607746 - Part 2: Clean up passing filter mode to shader r=nical I removed the old opacity filter path in the brush_blend shader and cleaned up the filter mode constants in the shader so there are less magic numbers. This should help if/when we move more filters to their own shaders. Depends on D59610 Differential Revision: https://phabricator.services.mozilla.com/D59611
gfx/wr/webrender/res/brush_blend.glsl
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/internal_types.rs
--- a/gfx/wr/webrender/res/brush_blend.glsl
+++ b/gfx/wr/webrender/res/brush_blend.glsl
@@ -9,16 +9,31 @@
 #define WR_BRUSH_FS_FUNCTION blend_brush_fs
 
 #define COMPONENT_TRANSFER_IDENTITY 0
 #define COMPONENT_TRANSFER_TABLE 1
 #define COMPONENT_TRANSFER_DISCRETE 2
 #define COMPONENT_TRANSFER_LINEAR 3
 #define COMPONENT_TRANSFER_GAMMA 4
 
+// Must be kept in sync with `Filter::as_int` in internal_types.rs
+// Not all filters are defined here because some filter use different shaders.
+#define FILTER_CONTRAST            0
+#define FILTER_GRAYSCALE           1
+#define FILTER_HUE_ROTATE          2
+#define FILTER_INVERT              3
+#define FILTER_SATURATE            4
+#define FILTER_SEPIA               5
+#define FILTER_BRIGHTNESS          6
+#define FILTER_COLOR_MATRIX        7
+#define FILTER_SRGB_TO_LINEAR      8
+#define FILTER_LINEAR_TO_SRGB      9
+#define FILTER_FLOOD               10
+#define FILTER_COMPONENT_TRANSFER  11
+
 #include shared,prim_shared,brush
 
 // Interpolated UV coordinates to sample.
 #define V_UV                varying_vec4_0.zw
 #define V_LOCAL_POS         varying_vec4_0.xy
 
 #define V_FLOOD_COLOR       flat_varying_vec4_1
 
@@ -95,77 +110,70 @@ void blend_brush_vs(
     // default: just to satisfy angle_shader_validation.rs which needs one
     // default: for every switch, even in comments.
     vFuncs[0] = (prim_user_data.y >> 28) & 0xf; // R
     vFuncs[1] = (prim_user_data.y >> 24) & 0xf; // G
     vFuncs[2] = (prim_user_data.y >> 20) & 0xf; // B
     vFuncs[3] = (prim_user_data.y >> 16) & 0xf; // A
 
     switch (V_OP) {
-        case 2: {
-            // Grayscale
+        case FILTER_GRAYSCALE: {
             vColorMat = mat4(
                 vec4(lumR + oneMinusLumR * invAmount, lumR - lumR * invAmount, lumR - lumR * invAmount, 0.0),
                 vec4(lumG - lumG * invAmount, lumG + oneMinusLumG * invAmount, lumG - lumG * invAmount, 0.0),
                 vec4(lumB - lumB * invAmount, lumB - lumB * invAmount, lumB + oneMinusLumB * invAmount, 0.0),
                 vec4(0.0, 0.0, 0.0, 1.0)
             );
             V_COLOR_OFFSET = vec4(0.0);
             break;
         }
-        case 3: {
-            // HueRotate
+        case FILTER_HUE_ROTATE: {
             float c = cos(amount);
             float s = sin(amount);
             vColorMat = mat4(
                 vec4(lumR + oneMinusLumR * c - lumR * s, lumR - lumR * c + 0.143 * s, lumR - lumR * c - oneMinusLumR * s, 0.0),
                 vec4(lumG - lumG * c - lumG * s, lumG + oneMinusLumG * c + 0.140 * s, lumG - lumG * c + lumG * s, 0.0),
                 vec4(lumB - lumB * c + oneMinusLumB * s, lumB - lumB * c - 0.283 * s, lumB + oneMinusLumB * c + lumB * s, 0.0),
                 vec4(0.0, 0.0, 0.0, 1.0)
             );
             V_COLOR_OFFSET = vec4(0.0);
             break;
         }
-        case 5: {
-            // Saturate
+        case FILTER_SATURATE: {
             vColorMat = mat4(
                 vec4(invAmount * lumR + amount, invAmount * lumR, invAmount * lumR, 0.0),
                 vec4(invAmount * lumG, invAmount * lumG + amount, invAmount * lumG, 0.0),
                 vec4(invAmount * lumB, invAmount * lumB, invAmount * lumB + amount, 0.0),
                 vec4(0.0, 0.0, 0.0, 1.0)
             );
             V_COLOR_OFFSET = vec4(0.0);
             break;
         }
-        case 6: {
-            // Sepia
+        case FILTER_SEPIA: {
             vColorMat = mat4(
                 vec4(0.393 + 0.607 * invAmount, 0.349 - 0.349 * invAmount, 0.272 - 0.272 * invAmount, 0.0),
                 vec4(0.769 - 0.769 * invAmount, 0.686 + 0.314 * invAmount, 0.534 - 0.534 * invAmount, 0.0),
                 vec4(0.189 - 0.189 * invAmount, 0.168 - 0.168 * invAmount, 0.131 + 0.869 * invAmount, 0.0),
                 vec4(0.0, 0.0, 0.0, 1.0)
             );
             V_COLOR_OFFSET = vec4(0.0);
             break;
         }
-        case 10: {
-            // Color Matrix
+        case FILTER_COLOR_MATRIX: {
             vec4 mat_data[4] = fetch_from_gpu_cache_4(prim_user_data.z);
             vec4 offset_data = fetch_from_gpu_cache_1(prim_user_data.z + 4);
             vColorMat = mat4(mat_data[0], mat_data[1], mat_data[2], mat_data[3]);
             V_COLOR_OFFSET = offset_data;
             break;
         }
-        case 13: {
-            // Component Transfer
+        case FILTER_COMPONENT_TRANSFER: {
             V_TABLE_ADDRESS = prim_user_data.z;
             break;
         }
-        case 14: {
-            // Flood
+        case FILTER_FLOOD: {
             V_FLOOD_COLOR = fetch_from_gpu_cache_1(prim_user_data.z);
             break;
         }
         default: break;
     }
 }
 #endif
 
@@ -264,50 +272,45 @@ Fragment blend_brush_fs() {
     vec2 uv = V_UV * perspective_divisor;
     vec4 Cs = texture(sColor0, vec3(uv, V_LAYER));
 
     // Un-premultiply the input.
     float alpha = Cs.a;
     vec3 color = alpha != 0.0 ? Cs.rgb / alpha : Cs.rgb;
 
     switch (V_OP) {
-        case 0:
-            break;
-        case 1:
+        case FILTER_CONTRAST:
             color = Contrast(color, V_AMOUNT);
             break;
-        case 4:
+        case FILTER_INVERT:
             color = Invert(color, V_AMOUNT);
             break;
-        case 7:
+        case FILTER_BRIGHTNESS:
             color = Brightness(color, V_AMOUNT);
             break;
-        case 8: // Opacity
-            alpha *= V_AMOUNT;
-            break;
-        case 11:
+        case FILTER_SRGB_TO_LINEAR:
             color = SrgbToLinear(color);
             break;
-        case 12:
+        case FILTER_LINEAR_TO_SRGB:
             color = LinearToSrgb(color);
             break;
-        case 13: {
-            // Component Transfer
+        case FILTER_COMPONENT_TRANSFER: {
             // Get the unpremultiplied color with alpha.
             vec4 colora = vec4(color, alpha);
             colora = ComponentTransfer(colora);
             color = colora.rgb;
             alpha = colora.a;
             break;
         }
-        case 14: // Flood
+        case FILTER_FLOOD:
             color = V_FLOOD_COLOR.rgb;
             alpha = V_FLOOD_COLOR.a;
             break;
         default:
+            // Color matrix type filters (sepia, hue-rotate, etc...)
             vec4 result = vColorMat * vec4(color, alpha) + V_COLOR_OFFSET;
             result = clamp(result, vec4(0.0), vec4(1.0));
             color = result.rgb;
             alpha = result.a;
     }
 
     // Fail-safe to ensure that we don't sample outside the rendered
     // portion of a blend source.
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -1399,62 +1399,45 @@ impl BatchBuilder {
                                             clip_task_address.unwrap(),
                                             brush_flags,
                                             prim_header_index,
                                             0,
                                             prim_vis_mask,
                                         );
                                     }
                                     _ => {
-                                        let filter_mode = match filter {
-                                            Filter::Identity => 1, // matches `Contrast(1)`
-                                            Filter::Blur(..) => 0,
-                                            Filter::Contrast(..) => 1,
-                                            Filter::Grayscale(..) => 2,
-                                            Filter::HueRotate(..) => 3,
-                                            Filter::Invert(..) => 4,
-                                            Filter::Saturate(..) => 5,
-                                            Filter::Sepia(..) => 6,
-                                            Filter::Brightness(..) => 7,
-                                            Filter::Opacity(..) => 8,
-                                            Filter::DropShadows(..) => 9,
-                                            Filter::ColorMatrix(..) => 10,
-                                            Filter::SrgbToLinear => 11,
-                                            Filter::LinearToSrgb => 12,
-                                            Filter::ComponentTransfer => unreachable!(),
-                                            Filter::Flood(..) => 14,
-                                        };
+                                        // Must be kept in sync with brush_blend.glsl
+                                        let filter_mode = filter.as_int();
 
                                         let user_data = match filter {
                                             Filter::Identity => 0x10000i32, // matches `Contrast(1)`
                                             Filter::Contrast(amount) |
                                             Filter::Grayscale(amount) |
                                             Filter::Invert(amount) |
                                             Filter::Saturate(amount) |
                                             Filter::Sepia(amount) |
-                                            Filter::Brightness(amount) |
-                                            Filter::Opacity(_, amount) => {
+                                            Filter::Brightness(amount) => {
                                                 (amount * 65536.0) as i32
                                             }
                                             Filter::SrgbToLinear | Filter::LinearToSrgb => 0,
                                             Filter::HueRotate(angle) => {
                                                 (0.01745329251 * angle * 65536.0) as i32
                                             }
-                                            // Go through different paths
-                                            Filter::Blur(..) |
-                                            Filter::DropShadows(..) => {
-                                                unreachable!();
-                                            }
                                             Filter::ColorMatrix(_) => {
                                                 picture.extra_gpu_data_handles[0].as_int(gpu_cache)
                                             }
-                                            Filter::ComponentTransfer => unreachable!(),
                                             Filter::Flood(_) => {
                                                 picture.extra_gpu_data_handles[0].as_int(gpu_cache)
                                             }
+
+                                            // These filters are handled via different paths.
+                                            Filter::ComponentTransfer |
+                                            Filter::Blur(..) |
+                                            Filter::DropShadows(..) |
+                                            Filter::Opacity(..) => unreachable!(),
                                         };
 
                                         let (uv_rect_address, textures) = render_tasks.resolve_surface(
                                             surface_task.expect("bug: surface must be allocated by now"),
                                             gpu_cache,
                                         );
 
                                         let key = BatchKey::new(
@@ -1486,17 +1469,17 @@ impl BatchBuilder {
                                     }
                                 }
                             }
                             PictureCompositeMode::ComponentTransferFilter(handle) => {
                                 // This is basically the same as the general filter case above
                                 // except we store a little more data in the filter mode and
                                 // a gpu cache handle in the user data.
                                 let filter_data = &ctx.data_stores.filter_data[handle];
-                                let filter_mode : i32 = 13 |
+                                let filter_mode : i32 = Filter::ComponentTransfer.as_int() |
                                     ((filter_data.data.r_func.to_int() << 28 |
                                       filter_data.data.g_func.to_int() << 24 |
                                       filter_data.data.b_func.to_int() << 20 |
                                       filter_data.data.a_func.to_int() << 16) as i32);
 
                                 let user_data = filter_data.gpu_cache_handle.as_int(gpu_cache);
 
                                 let (uv_rect_address, textures) = render_tasks.resolve_surface(
--- a/gfx/wr/webrender/src/internal_types.rs
+++ b/gfx/wr/webrender/src/internal_types.rs
@@ -158,16 +158,39 @@ impl Filter {
                 ]
             }
             Filter::SrgbToLinear |
             Filter::LinearToSrgb |
             Filter::ComponentTransfer |
             Filter::Flood(..) => false,
         }
     }
+
+
+    pub fn as_int(&self) -> i32 {
+        // Must be kept in sync with brush_blend.glsl
+        match *self {
+            Filter::Identity => 0, // matches `Contrast(1)`
+            Filter::Contrast(..) => 0,
+            Filter::Grayscale(..) => 1,
+            Filter::HueRotate(..) => 2,
+            Filter::Invert(..) => 3,
+            Filter::Saturate(..) => 4,
+            Filter::Sepia(..) => 5,
+            Filter::Brightness(..) => 6,
+            Filter::ColorMatrix(..) => 7,
+            Filter::SrgbToLinear => 8,
+            Filter::LinearToSrgb => 9,
+            Filter::Flood(..) => 10,
+            Filter::ComponentTransfer => 11,
+            Filter::Blur(..) => 12,
+            Filter::DropShadows(..) => 13,
+            Filter::Opacity(..) => 14,
+        }
+    }
 }
 
 impl From<FilterOp> for Filter {
     fn from(op: FilterOp) -> Self {
         match op {
             FilterOp::Identity => Filter::Identity,
             FilterOp::Blur(r) => Filter::Blur(r),
             FilterOp::Brightness(b) => Filter::Brightness(b),