Correctly batch AnimatedColor updates

Summary: Call `setWaitingForIdentifier` before we do any `setValue` calls to make sure we execute them together (and only call start/finish batch once). Only calll `updateAnimatedNodeConfig` conditionally when we changed nativeColor. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D36517302 fbshipit-source-id: ecbae2d1df69e4456620c58a922275406e22a2f8
facebook · May 23, 2022 · 406a474 · 406a474
1 parent 7e64636
commit 406a474
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 24 deletions.
diff --git a/Libraries/Animated/nodes/AnimatedColor.js b/Libraries/Animated/nodes/AnimatedColor.js
@@ -24,14 +24,17 @@ import type {ProcessedColorValue} from '../../StyleSheet/processColor';
 export type AnimatedColorConfig = $ReadOnly<{
   useNativeDriver: boolean,
 }>;
-type ColorListenerCallback = (value: string) => mixed;
+
+type ColorListenerCallback = (value: ColorValue) => mixed;
+
 export type RgbaValue = {
   +r: number,
   +g: number,
   +b: number,
   +a: number,
   ...
 };
+
 type RgbaAnimatedValue = {
   +r: AnimatedValue,
   +g: AnimatedValue,
@@ -40,6 +43,8 @@ type RgbaAnimatedValue = {
   ...
 };
 
+const NativeAnimatedAPI = NativeAnimatedHelper.API;
+
 const defaultColor: RgbaValue = {r: 0, g: 0, b: 0, a: 1.0};
 let _uniqueId = 1;
 
@@ -161,34 +166,43 @@ export default class AnimatedColor extends AnimatedWithChildren {
    * and update all the bound properties.
    */
   setValue(value: RgbaValue | ColorValue): void {
-    this.nativeColor = null;
+    let shouldUpdateNodeConfig = false;
+    if (this.__isNative) {
+      const nativeTag = this.__getNativeTag();
+      NativeAnimatedAPI.setWaitingForIdentifier(nativeTag.toString());
+    }
 
     const processedColor: RgbaValue | NativeColorValue =
       processColor(value) ?? defaultColor;
     if (isRgbaValue(processedColor)) {
-      // $FlowIgnore[incompatible-cast] - Type is verified above
-      const rgbaValue: RgbaValue = (processedColor: RgbaValue);
+      // $FlowIgnore[incompatible-type] - Type is verified above
+      const rgbaValue: RgbaValue = processedColor;
       this.r.setValue(rgbaValue.r);
       this.g.setValue(rgbaValue.g);
       this.b.setValue(rgbaValue.b);
       this.a.setValue(rgbaValue.a);
+      if (this.nativeColor != null) {
+        this.nativeColor = null;
+        shouldUpdateNodeConfig = true;
+      }
     } else {
-      // $FlowIgnore[incompatible-cast] - Type is verified above
-      this.nativeColor = (processedColor: NativeColorValue);
-    }
-
-    if (this.nativeColor) {
-      if (!this.__isNative) {
-        this.__makeNative();
+      // $FlowIgnore[incompatible-type] - Type is verified above
+      const nativeColor: NativeColorValue = processedColor;
+      if (this.nativeColor !== nativeColor) {
+        this.nativeColor = nativeColor;
+        shouldUpdateNodeConfig = true;
       }
+    }
 
+    if (this.__isNative) {
       const nativeTag = this.__getNativeTag();
-      NativeAnimatedHelper.API.setWaitingForIdentifier(nativeTag.toString());
-      NativeAnimatedHelper.API.updateAnimatedNodeConfig(
-        nativeTag,
-        this.__getNativeConfig(),
-      );
-      NativeAnimatedHelper.API.unsetWaitingForIdentifier(nativeTag.toString());
+      if (shouldUpdateNodeConfig) {
+        NativeAnimatedAPI.updateAnimatedNodeConfig(
+          nativeTag,
+          this.__getNativeConfig(),
+        );
+      }
+      NativeAnimatedAPI.unsetWaitingForIdentifier(nativeTag.toString());
     }
   }
 
@@ -275,7 +289,7 @@ export default class AnimatedColor extends AnimatedWithChildren {
    * final value after stopping the animation, which is useful for updating
    * state to match the animation position with layout.
    */
-  stopAnimation(callback?: (value: string) => void): void {
+  stopAnimation(callback?: ColorListenerCallback): void {
     this.r.stopAnimation();
     this.g.stopAnimation();
     this.b.stopAnimation();
@@ -286,16 +300,20 @@ export default class AnimatedColor extends AnimatedWithChildren {
   /**
    * Stops any animation and resets the value to its original.
    */
-  resetAnimation(callback?: (value: string) => void): void {
+  resetAnimation(callback?: ColorListenerCallback): void {
     this.r.resetAnimation();
     this.g.resetAnimation();
     this.b.resetAnimation();
     this.a.resetAnimation();
     callback && callback(this.__getValue());
   }
 
-  __getValue(): string {
-    return `rgba(${this.r.__getValue()}, ${this.g.__getValue()}, ${this.b.__getValue()}, ${this.a.__getValue()})`;
+  __getValue(): ColorValue {
+    if (this.nativeColor != null) {
+      return this.nativeColor;
+    } else {
+      return `rgba(${this.r.__getValue()}, ${this.g.__getValue()}, ${this.b.__getValue()}, ${this.a.__getValue()})`;
+    }
   }
 
   __attach(): void {

diff --git a/Libraries/Animated/nodes/AnimatedValue.js b/Libraries/Animated/nodes/AnimatedValue.js
@@ -134,9 +134,9 @@ class AnimatedValue extends AnimatedWithChildren {
       !this.__isNative /* don't perform a flush for natively driven values */,
     );
     if (this.__isNative) {
-      _executeAsAnimatedBatch(this.__getNativeTag().toString(), () => {
-        NativeAnimatedAPI.setAnimatedNodeValue(this.__getNativeTag(), value);
-      });
+      _executeAsAnimatedBatch(this.__getNativeTag().toString(), () =>
+        NativeAnimatedAPI.setAnimatedNodeValue(this.__getNativeTag(), value),
+      );
     }
   }