From 57613e93e0340d5274ff9a1fb77d1f34049cb127 Mon Sep 17 00:00:00 2001
From: Glenn Watson <github@intuitionlibrary.com>
Date: Thu, 7 Sep 2017 12:55:29 +1000
Subject: [PATCH] Fix bug in handling redundant opacity filters.

If a filter had a property binding, and that value resolved
to opacity=1, WR can skip applying an expensive filter
operation on that stacking context.
---
 webrender/src/frame.rs            | 48 +++++++++++++++++++++++++++----
 webrender_api/src/display_item.rs | 17 -----------
 2 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/webrender/src/frame.rs b/webrender/src/frame.rs
index 5f80ec7fd4..7bca02efbd 100644
--- a/webrender/src/frame.rs
+++ b/webrender/src/frame.rs
@@ -189,6 +189,46 @@ pub struct Frame {
     frame_builder: Option<FrameBuilder>,
 }
 
+trait FilterOpHelpers {
+    fn resolve(self, properties: &SceneProperties) -> FilterOp;
+    fn is_noop(&self) -> bool;
+}
+
+impl FilterOpHelpers for FilterOp {
+    fn resolve(self, properties: &SceneProperties) -> FilterOp {
+        match self {
+            FilterOp::Opacity(ref value) => {
+                let amount = properties.resolve_float(value, 1.0);
+                FilterOp::Opacity(PropertyBinding::Value(amount))
+            }
+            _ => self
+        }
+    }
+
+    fn is_noop(&self) -> bool {
+        match *self {
+            FilterOp::Blur(length) => length == 0.0,
+            FilterOp::Brightness(amount) => amount == 1.0,
+            FilterOp::Contrast(amount) => amount == 1.0,
+            FilterOp::Grayscale(amount) => amount == 0.0,
+            FilterOp::HueRotate(amount) => amount == 0.0,
+            FilterOp::Invert(amount) => amount == 0.0,
+            FilterOp::Opacity(value) => {
+                match value {
+                    PropertyBinding::Value(amount) => {
+                        amount == 1.0
+                    }
+                    PropertyBinding::Binding(..) => {
+                        panic!("bug: binding value should be resolved");
+                    }
+                }
+            }
+            FilterOp::Saturate(amount) => amount == 1.0,
+            FilterOp::Sepia(amount) => amount == 0.0,
+        }
+    }
+}
+
 trait StackingContextHelpers {
     fn mix_blend_mode_for_compositing(&self) -> Option<MixBlendMode>;
     fn filter_ops_for_compositing(&self,
@@ -211,15 +251,11 @@ impl StackingContextHelpers for StackingContext {
                                   properties: &SceneProperties) -> Vec<FilterOp> {
         let mut filters = vec![];
         for filter in display_list.get(input_filters) {
+            let filter = filter.resolve(properties);
             if filter.is_noop() {
                 continue;
             }
-            if let FilterOp::Opacity(ref value) = filter {
-                let amount = properties.resolve_float(value, 1.0);
-                filters.push(FilterOp::Opacity(PropertyBinding::Value(amount)));
-            } else {
-                filters.push(filter);
-            }
+            filters.push(filter);
         }
         filters
     }
diff --git a/webrender_api/src/display_item.rs b/webrender_api/src/display_item.rs
index ac5282421f..833f2be63d 100644
--- a/webrender_api/src/display_item.rs
+++ b/webrender_api/src/display_item.rs
@@ -384,23 +384,6 @@ pub enum FilterOp {
     Sepia(f32),
 }
 
-impl FilterOp {
-    pub fn is_noop(&self) -> bool {
-        match *self {
-            FilterOp::Blur(length) if length == 0.0 => true,
-            FilterOp::Brightness(amount) if amount == 1.0 => true,
-            FilterOp::Contrast(amount) if amount == 1.0 => true,
-            FilterOp::Grayscale(amount) if amount == 0.0 => true,
-            FilterOp::HueRotate(amount) if amount == 0.0 => true,
-            FilterOp::Invert(amount) if amount == 0.0 => true,
-            FilterOp::Opacity(amount) if amount == PropertyBinding::Value(1.0) => true,
-            FilterOp::Saturate(amount) if amount == 1.0 => true,
-            FilterOp::Sepia(amount) if amount == 0.0 => true,
-            _ => false,
-        }
-    }
-}
-
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct IframeDisplayItem {
     pub pipeline_id: PipelineId,