Skip to content

Commit

Permalink
Don't update dimensions for a new ReactRootView if they haven't changed
Browse files Browse the repository at this point in the history
Summary:
ReactRootView currently caches the last seen DisplayMetrics so we can compare them against the current values in DisplayMetricsHolder to determine if the screen dimensions have changed. (If they have changed, we emit an event to notify JS of the new dimensions).

However, ReactRootView initializes these member variables with empty DisplayMetrics, which means that the first time we check against them in onGlobalLayout, we will *always* emit an event to JS.

This seems reasonable if you only have one ReactRootView, but if you create a new RRV for each RN screen in your app, then you're going to get updated dimensions on each navigation event, even though the screen dimensions have probably not changed.

In this diff, I'm no longer storing the DisplayMetrics in ReactRootView at all, but instead am using temporary variables to check the new DisplayMetrics values against the old.

Changelog: [Android][Fixed] Only update dimensions in ReactRootView if they've changed

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D19395326

fbshipit-source-id: c01aee73064764503c9b49208032c790b83a1d29
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Jan 18, 2020
1 parent a89a553 commit cc3e27d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 33 deletions.
32 changes: 2 additions & 30 deletions ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
import android.content.Context;
import android.graphics.Canvas;
import android.graphics.Rect;
import android.os.Build;
import android.os.Bundle;
import android.util.AttributeSet;
import android.util.DisplayMetrics;
import android.view.KeyEvent;
import android.view.MotionEvent;
import android.view.Surface;
Expand Down Expand Up @@ -683,8 +681,6 @@ private class CustomGlobalLayoutListener implements ViewTreeObserver.OnGlobalLay

private int mKeyboardHeight = 0;
private int mDeviceRotation = 0;
private DisplayMetrics mWindowMetrics = new DisplayMetrics();
private DisplayMetrics mScreenMetrics = new DisplayMetrics();

/* package */ CustomGlobalLayoutListener() {
DisplayMetricsHolder.initDisplayMetricsIfNotInitialized(getContext().getApplicationContext());
Expand Down Expand Up @@ -749,32 +745,8 @@ private void checkForDeviceOrientationChanges() {
}

private void checkForDeviceDimensionsChanges() {
// Get current display metrics.
DisplayMetricsHolder.initDisplayMetrics(getContext());
// Check changes to both window and screen display metrics since they may not update at the
// same time.
if (!areMetricsEqual(mWindowMetrics, DisplayMetricsHolder.getWindowDisplayMetrics())
|| !areMetricsEqual(mScreenMetrics, DisplayMetricsHolder.getScreenDisplayMetrics())) {
mWindowMetrics.setTo(DisplayMetricsHolder.getWindowDisplayMetrics());
mScreenMetrics.setTo(DisplayMetricsHolder.getScreenDisplayMetrics());
emitUpdateDimensionsEvent();
}
}

private boolean areMetricsEqual(DisplayMetrics displayMetrics, DisplayMetrics otherMetrics) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
return displayMetrics.equals(otherMetrics);
} else {
// DisplayMetrics didn't have an equals method before API 17.
// Check all public fields manually.
return displayMetrics.widthPixels == otherMetrics.widthPixels
&& displayMetrics.heightPixels == otherMetrics.heightPixels
&& displayMetrics.density == otherMetrics.density
&& displayMetrics.densityDpi == otherMetrics.densityDpi
&& displayMetrics.scaledDensity == otherMetrics.scaledDensity
&& displayMetrics.xdpi == otherMetrics.xdpi
&& displayMetrics.ydpi == otherMetrics.ydpi;
}
// DeviceInfoModule caches the last dimensions emitted to JS, so we don't need to check here.
emitUpdateDimensionsEvent();
}

private void emitOrientationChanged(final int newRotation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactNoCrashSoftException;
import com.facebook.react.bridge.ReactSoftException;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
Expand All @@ -30,6 +32,7 @@ public class DeviceInfoModule extends ReactContextBaseJavaModule

private @Nullable ReactApplicationContext mReactApplicationContext;
private float mFontScale;
private @Nullable ReadableMap mPreviousDisplayMetrics;

public DeviceInfoModule(ReactApplicationContext reactContext) {
super(reactContext);
Expand Down Expand Up @@ -83,9 +86,15 @@ public void emitUpdateDimensionsEvent() {
}

if (mReactApplicationContext.hasActiveCatalystInstance()) {
mReactApplicationContext
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
.emit("didUpdateDimensions", DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale));
// Don't emit an event to JS if the dimensions haven't changed
WritableNativeMap displayMetrics =
DisplayMetricsHolder.getDisplayMetricsNativeMap(mFontScale);
if (!displayMetrics.equals(mPreviousDisplayMetrics)) {
mReactApplicationContext
.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
.emit("didUpdateDimensions", displayMetrics);
mPreviousDisplayMetrics = displayMetrics;
}
} else {
ReactSoftException.logSoftException(
NAME,
Expand Down

0 comments on commit cc3e27d

Please sign in to comment.