From 249b341a414ecab6bf43c120b42ccf72d2758765 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 29 Dec 2020 15:52:10 -0800 Subject: [PATCH] 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 --- .../react/views/image/ReactImageView.java | 82 ++++++++++++++----- .../react/views/imagehelper/ImageSource.java | 17 ++++ 2 files changed, 77 insertions(+), 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 index 68ada3dfc70..9869fe878de 100644 --- 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.graphics.drawable.Drawable; 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 class ReactImageView extends GenericDraweeView { } 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 class ReactImageView extends GenericDraweeView { } 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 class ReactImageView extends GenericDraweeView { } 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 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 class ReactImageView extends GenericDraweeView { 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 index 9f95b5bae9a..065ebbe6366 100644 --- 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.content.Context; 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 class ImageSource { 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); }