From 9a51fa8e15acc052b5d271c369dfa4b227d657c2 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Wed, 5 Apr 2017 09:07:22 -0700 Subject: [PATCH] Improve z-index implementation on Android Summary: Use `getChildDrawingOrder` instead of reordering views. The old implementation didn't work properly when `removeClippedSubviews` was enabled and this one should have better performance since we don't play with the view hierarchy at all. This fixes weird bugs with sticky headers in `SectionList` and allows removing the hack that disabled `removeClippedSubviews` when using sticky section headers. **Test plan** Tested using the SectionList and ListViewPaging examples that use sticky headers which uses z-index. Closes https://github.com/facebook/react-native/pull/13105 Reviewed By: sahrens Differential Revision: D4765869 Pulled By: achen1 fbshipit-source-id: be3c824658a3ce965b6e7324ad95c77cbd8a86ae --- .../UIExplorer/js/ListViewPagingExample.js | 1 + Examples/UIExplorer/js/SectionListExample.js | 6 +- Libraries/Components/ScrollView/ScrollView.js | 4 +- .../ViewGroupDrawingOrderHelper.java | 103 ++++++++++++++++++ .../react/uimanager/ViewGroupManager.java | 53 +-------- .../react/views/view/ReactViewGroup.java | 34 ++++++ .../react/views/view/ReactViewManager.java | 1 - 7 files changed, 149 insertions(+), 53 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java diff --git a/Examples/UIExplorer/js/ListViewPagingExample.js b/Examples/UIExplorer/js/ListViewPagingExample.js index 58d7359e5..f140f2994 100644 --- a/Examples/UIExplorer/js/ListViewPagingExample.js +++ b/Examples/UIExplorer/js/ListViewPagingExample.js @@ -195,6 +195,7 @@ class ListViewPagingExample extends React.Component { initialListSize={10} pageSize={4} scrollRenderAheadDistance={500} + stickySectionHeadersEnabled /> ); } diff --git a/Examples/UIExplorer/js/SectionListExample.js b/Examples/UIExplorer/js/SectionListExample.js index 441aa19cb..8b795c5bf 100644 --- a/Examples/UIExplorer/js/SectionListExample.js +++ b/Examples/UIExplorer/js/SectionListExample.js @@ -59,7 +59,7 @@ const VIEWABILITY_CONFIG = { }; const renderSectionHeader = ({section}) => ( - + SECTION HEADER: {section.key} @@ -144,6 +144,7 @@ class SectionListExample extends React.PureComponent { refreshing={false} renderItem={this._renderItemComponent} renderSectionHeader={renderSectionHeader} + stickySectionHeadersEnabled sections={[ {renderItem: renderStackedItem, key: 's1', data: [ {title: 'Item In Header Section', text: 'Section s1', key: '0'}, @@ -186,6 +187,9 @@ class SectionListExample extends React.PureComponent { } const styles = StyleSheet.create({ + header: { + backgroundColor: '#e9eaed', + }, headerText: { padding: 4, }, diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index aa1f7e376..b0bc86b9d 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -650,9 +650,7 @@ const ScrollView = React.createClass({ {...contentSizeChangeProps} ref={this._setInnerViewRef} style={contentContainerStyle} - removeClippedSubviews={ - hasStickyHeaders && Platform.OS === 'android' ? false : this.props.removeClippedSubviews - } + removeClippedSubviews={this.props.removeClippedSubviews} collapsable={false}> {children} ; diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java new file mode 100644 index 000000000..da577ae08 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupDrawingOrderHelper.java @@ -0,0 +1,103 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.react.uimanager; + +import android.view.View; +import android.view.ViewGroup; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; + +import javax.annotation.Nullable; + +/** + * Helper to handle implementing ViewGroups with custom drawing order based on z-index. + */ +public class ViewGroupDrawingOrderHelper { + private final ViewGroup mViewGroup; + private int mNumberOfChildrenWithZIndex = 0; + private @Nullable int[] mDrawingOrderIndices; + + public ViewGroupDrawingOrderHelper(ViewGroup viewGroup) { + mViewGroup = viewGroup; + } + + /** + * This should be called every time a view is added to the ViewGroup in {@link ViewGroup#addView}. + * @param view The view that is being added + */ + public void handleAddView(View view) { + if (ViewGroupManager.getViewZIndex(view) != null) { + mNumberOfChildrenWithZIndex++; + } + + mDrawingOrderIndices = null; + } + + /** + * This should be called every time a view is removed from the ViewGroup in {@link ViewGroup#removeView} + * and {@link ViewGroup#removeViewAt}. + * @param view The view that is being removed. + */ + public void handleRemoveView(View view) { + if (ViewGroupManager.getViewZIndex(view) != null) { + mNumberOfChildrenWithZIndex--; + } + + mDrawingOrderIndices = null; + } + + /** + * If the ViewGroup should enable drawing order. ViewGroups should call + * {@link ViewGroup#setChildrenDrawingOrderEnabled} with the value returned from this method when + * a view is added or removed. + */ + public boolean shouldEnableCustomDrawingOrder() { + return mNumberOfChildrenWithZIndex > 0; + } + + /** + * The index of the child view that should be drawn. This should be used in + * {@link ViewGroup#getChildDrawingOrder}. + */ + public int getChildDrawingOrder(int childCount, int index) { + if (mDrawingOrderIndices == null) { + ArrayList viewsToSort = new ArrayList<>(); + for (int i = 0; i < childCount; i++) { + viewsToSort.add(mViewGroup.getChildAt(i)); + } + // Sort the views by zIndex + Collections.sort(viewsToSort, new Comparator() { + @Override + public int compare(View view1, View view2) { + Integer view1ZIndex = ViewGroupManager.getViewZIndex(view1); + if (view1ZIndex == null) { + view1ZIndex = 0; + } + + Integer view2ZIndex = ViewGroupManager.getViewZIndex(view2); + if (view2ZIndex == null) { + view2ZIndex = 0; + } + + return view1ZIndex - view2ZIndex; + } + }); + + mDrawingOrderIndices = new int[childCount]; + for (int i = 0; i < childCount; i++) { + View child = viewsToSort.get(i); + mDrawingOrderIndices[i] = mViewGroup.indexOfChild(child); + } + } + return mDrawingOrderIndices[index]; + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java index 6a728db8f..103529a0e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.java @@ -19,13 +19,15 @@ import java.util.WeakHashMap; import android.view.View; import android.view.ViewGroup; +import javax.annotation.Nullable; + /** * Class providing children management API for view managers of classes extending ViewGroup. */ public abstract class ViewGroupManager extends BaseViewManager { - public static WeakHashMap mZIndexHash = new WeakHashMap<>(); + private static WeakHashMap mZIndexHash = new WeakHashMap<>(); @Override public LayoutShadowNode createShadowNodeInstance() { @@ -43,7 +45,6 @@ public abstract class ViewGroupManager public void addView(T parent, View child, int index) { parent.addView(child, index); - reorderChildrenByZIndex(parent); } /** @@ -61,54 +62,10 @@ public abstract class ViewGroupManager public static void setViewZIndex(View view, int zIndex) { mZIndexHash.put(view, zIndex); - // zIndex prop gets set BEFORE the view is added, so parent may be null. - ViewGroup parent = (ViewGroup) view.getParent(); - if (parent != null) { - reorderChildrenByZIndex(parent); - } } - public static void reorderChildrenByZIndex(ViewGroup view) { - // Optimization: loop through the zIndexHash to test if there are any non-zero zIndexes - // If there aren't any, we can just return out - Collection zIndexes = mZIndexHash.values(); - boolean containsZIndexedElement = false; - for (Integer zIndex : zIndexes) { - if (zIndex != 0) { - containsZIndexedElement = true; - break; - } - } - if (!containsZIndexedElement) { - return; - } - - // Add all children to a sortable ArrayList - ArrayList viewsToSort = new ArrayList<>(); - for (int i = 0; i < view.getChildCount(); i++) { - viewsToSort.add(view.getChildAt(i)); - } - // Sort the views by zIndex - Collections.sort(viewsToSort, new Comparator() { - @Override - public int compare(View view1, View view2) { - Integer view1ZIndex = mZIndexHash.get(view1); - if (view1ZIndex == null) { - view1ZIndex = 0; - } - - Integer view2ZIndex = mZIndexHash.get(view2); - if (view2ZIndex == null) { - view2ZIndex = 0; - } - return view1ZIndex - view2ZIndex; - } - }); - // Call .bringToFront on the sorted list of views - for (int i = 0; i < viewsToSort.size(); i++) { - viewsToSort.get(i).bringToFront(); - } - view.invalidate(); + public static @Nullable Integer getViewZIndex(View view) { + return mZIndexHash.get(view); } public int getChildCount(T parent) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index f69abaf02..3729b0e8c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -31,6 +31,7 @@ import com.facebook.react.uimanager.PointerEvents; import com.facebook.react.uimanager.ReactClippingViewGroup; import com.facebook.react.uimanager.ReactClippingViewGroupHelper; import com.facebook.react.uimanager.ReactPointerEventsView; +import com.facebook.react.uimanager.ViewGroupDrawingOrderHelper; /** * Backing for a React View. Has support for borders, but since borders aren't common, lazy @@ -96,9 +97,12 @@ public class ReactViewGroup extends ViewGroup implements private @Nullable ReactViewBackgroundDrawable mReactBackgroundDrawable; private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener; private boolean mNeedsOffscreenAlphaCompositing = false; + private final ViewGroupDrawingOrderHelper mDrawingOrderHelper; public ReactViewGroup(Context context) { super(context); + + mDrawingOrderHelper = new ViewGroupDrawingOrderHelper(this); } @Override @@ -374,6 +378,36 @@ public class ReactViewGroup extends ViewGroup implements } } + @Override + public void addView(View child, int index, LayoutParams params) { + // This will get called for every overload of addView so there is not need to override every method. + mDrawingOrderHelper.handleAddView(child); + setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder()); + + super.addView(child, index, params); + } + + @Override + public void removeView(View view) { + mDrawingOrderHelper.handleRemoveView(view); + setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder()); + + super.removeView(view); + } + + @Override + public void removeViewAt(int index) { + mDrawingOrderHelper.handleRemoveView(getChildAt(index)); + setChildrenDrawingOrderEnabled(mDrawingOrderHelper.shouldEnableCustomDrawingOrder()); + + super.removeViewAt(index); + } + + @Override + protected int getChildDrawingOrder(int childCount, int index) { + return mDrawingOrderHelper.getChildDrawingOrder(childCount, index); + } + @Override public PointerEvents getPointerEvents() { return mPointerEvents; diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java index d0070359d..3700d201a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java @@ -208,7 +208,6 @@ public class ReactViewManager extends ViewGroupManager { } else { parent.addView(child, index); } - reorderChildrenByZIndex(parent); } @Override