Don't redraw images when props don't change

Summary: Right now we assume in the Image component that any prop changes requires a redraw of the image, even if the props set are identical. Noop prop updates can be caused in Fabric by LayoutAnimations. This may go away in the future, but only when we have a new animations system. I don't think most other components need to be concerned with this, and many other components already guard against unnecessary redraws. Since the image "flashes" when it is loaded, unlike most other native components, this issue is more noticeable for images. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D25727482 fbshipit-source-id: 75ffa456bddc1208900733140ce4ff19f7e2c11e
jellynugget · Dec 29, 2020 · 249b341 · 249b341
1 parent b3defc8
commit 249b341
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 22 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java b/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java
@@ -23,6 +23,7 @@
 import android.net.Uri;
 import android.widget.Toast;
 import androidx.annotation.Nullable;
+import com.facebook.common.internal.Objects;
 import com.facebook.common.references.CloseableReference;
 import com.facebook.common.util.UriUtil;
 import com.facebook.drawee.controller.AbstractDraweeControllerBuilder;
@@ -90,8 +91,10 @@ public class ReactImageView extends GenericDraweeView {
   private ImageResizeMethod mResizeMethod = ImageResizeMethod.AUTO;
 
   public void updateCallerContext(@Nullable Object callerContext) {
-    mCallerContext = callerContext;
-    mIsDirty = true;
+    if (!Objects.equal(mCallerContext, callerContext)) {
+      mCallerContext = callerContext;
+      mIsDirty = true;
+    }
   }
 
   private class RoundedCornerPostprocessor extends BasePostprocessor {
@@ -229,6 +232,11 @@ public ReactImageView(
   }
 
   public void setShouldNotifyLoadEvents(boolean shouldNotify) {
+    // Skip update if shouldNotify is already in sync with the download listener
+    if (shouldNotify == (mDownloadListener != null)) {
+      return;
+    }
+
     if (!shouldNotify) {
       mDownloadListener = null;
     } else {
@@ -295,18 +303,25 @@ public void setBackgroundColor(int backgroundColor) {
   }
 
   public void setBorderColor(int borderColor) {
-    mBorderColor = borderColor;
-    mIsDirty = true;
+    if (mBorderColor != borderColor) {
+      mBorderColor = borderColor;
+      mIsDirty = true;
+    }
   }
 
   public void setOverlayColor(int overlayColor) {
-    mOverlayColor = overlayColor;
-    mIsDirty = true;
+    if (mOverlayColor != overlayColor) {
+      mOverlayColor = overlayColor;
+      mIsDirty = true;
+    }
   }
 
   public void setBorderWidth(float borderWidth) {
-    mBorderWidth = PixelUtil.toPixelFromDIP(borderWidth);
-    mIsDirty = true;
+    float newBorderWidth = PixelUtil.toPixelFromDIP(borderWidth);
+    if (!FloatUtil.floatsEqual(mBorderWidth, newBorderWidth)) {
+      mBorderWidth = newBorderWidth;
+      mIsDirty = true;
+    }
   }
 
   public void setBorderRadius(float borderRadius) {
@@ -329,32 +344,39 @@ public void setBorderRadius(float borderRadius, int position) {
   }
 
   public void setScaleType(ScalingUtils.ScaleType scaleType) {
-    mScaleType = scaleType;
-    mIsDirty = true;
+    if (mScaleType != scaleType) {
+      mScaleType = scaleType;
+      mIsDirty = true;
+    }
   }
 
   public void setTileMode(Shader.TileMode tileMode) {
-    mTileMode = tileMode;
-    mIsDirty = true;
+    if (mTileMode != tileMode) {
+      mTileMode = tileMode;
+      mIsDirty = true;
+    }
   }
 
   public void setResizeMethod(ImageResizeMethod resizeMethod) {
-    mResizeMethod = resizeMethod;
-    mIsDirty = true;
+    if (mResizeMethod != resizeMethod) {
+      mResizeMethod = resizeMethod;
+      mIsDirty = true;
+    }
   }
 
   public void setSource(@Nullable ReadableArray sources) {
-    mSources.clear();
+    List<ImageSource> tmpSources = new LinkedList<>();
+
     if (sources == null || sources.size() == 0) {
       ImageSource imageSource = new ImageSource(getContext(), REMOTE_TRANSPARENT_BITMAP_URI);
-      mSources.add(imageSource);
+      tmpSources.add(imageSource);
     } else {
       // Optimize for the case where we have just one uri, case in which we don't need the sizes
       if (sources.size() == 1) {
         ReadableMap source = sources.getMap(0);
         String uri = source.getString("uri");
         ImageSource imageSource = new ImageSource(getContext(), uri);
-        mSources.add(imageSource);
+        tmpSources.add(imageSource);
         if (Uri.EMPTY.equals(imageSource.getUri())) {
           warnImageSource(uri);
         }
@@ -365,28 +387,44 @@ public void setSource(@Nullable ReadableArray sources) {
           ImageSource imageSource =
               new ImageSource(
                   getContext(), uri, source.getDouble("width"), source.getDouble("height"));
-          mSources.add(imageSource);
+          tmpSources.add(imageSource);
           if (Uri.EMPTY.equals(imageSource.getUri())) {
             warnImageSource(uri);
           }
         }
       }
     }
+
+    // Don't reset sources and dirty node if sources haven't changed
+    if (mSources.equals(tmpSources)) {
+      return;
+    }
+
+    mSources.clear();
+    for (ImageSource src : tmpSources) {
+      mSources.add(src);
+    }
     mIsDirty = true;
   }
 
   public void setDefaultSource(@Nullable String name) {
-    mDefaultImageDrawable =
+    Drawable newDefaultDrawable =
         ResourceDrawableIdHelper.getInstance().getResourceDrawable(getContext(), name);
-    mIsDirty = true;
+    if (!Objects.equal(mDefaultImageDrawable, newDefaultDrawable)) {
+      mDefaultImageDrawable = newDefaultDrawable;
+      mIsDirty = true;
+    }
   }
 
   public void setLoadingIndicatorSource(@Nullable String name) {
     Drawable drawable =
         ResourceDrawableIdHelper.getInstance().getResourceDrawable(getContext(), name);
-    mLoadingImageDrawable =
+    Drawable newLoadingIndicatorSource =
         drawable != null ? (Drawable) new AutoRotateDrawable(drawable, 1000) : null;
-    mIsDirty = true;
+    if (!Objects.equal(mLoadingImageDrawable, newLoadingIndicatorSource)) {
+      mLoadingImageDrawable = newLoadingIndicatorSource;
+      mIsDirty = true;
+    }
   }
 
   public void setProgressiveRenderingEnabled(boolean enabled) {

diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/imagehelper/ImageSource.java b/ReactAndroid/src/main/java/com/facebook/react/views/imagehelper/ImageSource.java
@@ -11,6 +11,7 @@
 import android.net.Uri;
 import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Assertions;
+import java.util.Objects;
 
 /** Class describing an image source (network URI or resource) and size. */
 public class ImageSource {
@@ -29,6 +30,22 @@ public ImageSource(Context context, String source, double width, double height)
     mUri = computeUri(context);
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+    ImageSource that = (ImageSource) o;
+    return Double.compare(that.mSize, mSize) == 0
+        && isResource == that.isResource
+        && Objects.equals(mUri, that.mUri)
+        && Objects.equals(mSource, that.mSource);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(mUri, mSource, mSize, isResource);
+  }
+
   public ImageSource(Context context, String source) {
     this(context, source, 0.0d, 0.0d);
   }