Bug 1353202 - Rework Servo_AnimationCompose to rely less on pinned temporaries. draft
authorBrian Birtles <birtles@gmail.com>
Tue, 23 May 2017 14:57:17 +0900
changeset 582822 24ddceae3fe7aa644b7fbd15f310ca66dc1e259a
parent 582821 170ba44db9a5f026e6b4d8be9d0c2be5d9d34552
child 582823 1f71c04171930a08a71482a991e9a67688047a31
push id60187
push userbbirtles@mozilla.com
push dateTue, 23 May 2017 05:57:34 +0000
bugs1353202
milestone55.0a1
Bug 1353202 - Rework Servo_AnimationCompose to rely less on pinned temporaries. Some of the abstraction here, in particular, the separate declarations for from_value and to_value, will make more sense in light of the next patch in this series. MozReview-Commit-ID: BaDqVfUTlMx
servo/ports/geckolib/glue.rs
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -361,56 +361,71 @@ pub extern "C" fn Servo_AnimationCompose
         None
     };
 
     if need_underlying_value && underlying_value.is_none() {
         warn!("Underlying value should be valid when we expect to use it");
         return;
     }
 
-    // Temporaries used in the following if-block whose lifetimes we need to prolong.
+    // Extract keyframe values.
     let raw_from_value;
-    let from_composite_result;
-    let from_value = if !segment.mFromValue.mServo.mRawPtr.is_null() {
+    let keyframe_from_value = if !segment.mFromValue.mServo.mRawPtr.is_null() {
         raw_from_value = unsafe { &*segment.mFromValue.mServo.mRawPtr };
-        match segment.mFromComposite {
-            CompositeOperation::Add | CompositeOperation::Accumulate => {
-                let value_to_composite = AnimationValue::as_arc(&raw_from_value).as_ref();
-                from_composite_result = if segment.mFromComposite == CompositeOperation::Add {
-                    underlying_value.as_ref().unwrap().add(value_to_composite)
-                } else {
-                    underlying_value.as_ref().unwrap().accumulate(value_to_composite, 1)
-                };
-                from_composite_result.as_ref().unwrap_or(value_to_composite)
-            }
-            _ => { AnimationValue::as_arc(&raw_from_value) }
-        }
+        Some(AnimationValue::as_arc(&raw_from_value))
     } else {
-        underlying_value.as_ref().unwrap()
+        None
     };
 
     let raw_to_value;
-    let to_composite_result;
-    let to_value = if !segment.mToValue.mServo.mRawPtr.is_null() {
+    let keyframe_to_value = if !segment.mToValue.mServo.mRawPtr.is_null() {
         raw_to_value = unsafe { &*segment.mToValue.mServo.mRawPtr };
-        match segment.mToComposite {
-            CompositeOperation::Add | CompositeOperation::Accumulate => {
-                let value_to_composite = AnimationValue::as_arc(&raw_to_value).as_ref();
-                to_composite_result = if segment.mToComposite == CompositeOperation::Add {
-                    underlying_value.as_ref().unwrap().add(value_to_composite)
-                } else {
-                    underlying_value.as_ref().unwrap().accumulate(value_to_composite, 1)
-                };
-                to_composite_result.as_ref().unwrap_or(value_to_composite)
-            }
-            _ => { AnimationValue::as_arc(&raw_to_value) }
+        Some(AnimationValue::as_arc(&raw_to_value))
+    } else {
+        None
+    };
+
+    // Composite with underlying value.
+    // A return value of None means, "Just use keyframe_value as-is."
+    let composite_endpoint = |keyframe_value: Option<&Arc<AnimationValue>>,
+                              composite_op: CompositeOperation| -> Option<AnimationValue> {
+        match keyframe_value {
+            Some(keyframe_value) => {
+                match composite_op {
+                    CompositeOperation::Add => {
+                        debug_assert!(need_underlying_value,
+                                      "Should have detected we need an underlying value");
+                        underlying_value.as_ref().unwrap().add(keyframe_value).ok()
+                    },
+                    CompositeOperation::Accumulate => {
+                        debug_assert!(need_underlying_value,
+                                      "Should have detected we need an underlying value");
+                        underlying_value.as_ref().unwrap().accumulate(keyframe_value, 1).ok()
+                    },
+                    _ => None,
+                }
+            },
+            None => {
+                debug_assert!(need_underlying_value,
+                              "Should have detected we need an underlying value");
+                underlying_value.clone()
+            },
         }
-    } else {
-        underlying_value.as_ref().unwrap()
     };
+    let composited_from_value = composite_endpoint(keyframe_from_value, segment.mFromComposite);
+    let composited_to_value = composite_endpoint(keyframe_to_value, segment.mToComposite);
+
+    debug_assert!(keyframe_from_value.is_some() || composited_from_value.is_some(),
+                  "Should have a suitable from value to use");
+    debug_assert!(keyframe_to_value.is_some() || composited_to_value.is_some(),
+                  "Should have a suitable to value to use");
+
+    // Use the composited value if there is one, otherwise, use the original keyframe value.
+    let from_value = composited_from_value.as_ref().unwrap_or(keyframe_from_value.unwrap());
+    let to_value   = composited_to_value.as_ref().unwrap_or(keyframe_to_value.unwrap());
 
     let progress = unsafe { Gecko_GetProgressFromComputedTiming(computed_timing) };
     if segment.mToKey == segment.mFromKey {
         if progress < 0. {
             value_map.insert(property, from_value.clone());
         } else {
             value_map.insert(property, to_value.clone());
         }