From 4294b2446a6e328293cb67dcec6780ec2ffcb7f6 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 24 May 2024 04:42:27 -0700 Subject: [PATCH] Simplify ReactViewGroup clip to border (#44646) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/44646 We can remove most of the code for clipping children to border radius, and recalculating paths, in ReactViewGroup, and rely on the padding box path/rect already set. I will move this to something more generic up the stack so other native components can reuse this logic. Changelog: [Internal] Reviewed By: javache Differential Revision: D57668976 fbshipit-source-id: 8b8cf956dc8689827bccba5e41751b465fd85eeb --- .../ReactAndroid/api/ReactAndroid.api | 6 +- .../uimanager/drawable/BoxShadowDrawable.kt | 2 +- .../drawable/CSSBackgroundDrawable.java | 23 +++- .../react/views/view/ReactViewGroup.java | 101 ++---------------- .../js/examples/Border/BorderExample.js | 64 +++++++++++ 5 files changed, 96 insertions(+), 100 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 474cc0588458bc..2d93de893e7550 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -5557,21 +5557,21 @@ public abstract interface class com/facebook/react/uimanager/debug/NotThreadSafe public class com/facebook/react/uimanager/drawable/CSSBackgroundDrawable : android/graphics/drawable/Drawable { public fun (Landroid/content/Context;)V - public fun borderBoxPath ()Landroid/graphics/Path; public fun draw (Landroid/graphics/Canvas;)V public fun getAlpha ()I + public fun getBorderBoxPath ()Landroid/graphics/Path; public fun getBorderColor (I)I public fun getBorderRadius ()Lcom/facebook/react/uimanager/style/BorderRadiusStyle; public fun getBorderWidthOrDefaultTo (FI)F public fun getComputedBorderRadius ()Lcom/facebook/react/uimanager/style/ComputedBorderRadius; - public fun getDirectionAwareBorderInsets ()Landroid/graphics/RectF; public fun getFullBorderWidth ()F public fun getLayoutDirection ()I public fun getOpacity ()I public fun getOutline (Landroid/graphics/Outline;)V + public fun getPaddingBoxPath ()Landroid/graphics/Path; + public fun getPaddingBoxRect ()Landroid/graphics/RectF; public fun hasRoundedBorders ()Z protected fun onBoundsChange (Landroid/graphics/Rect;)V - public fun paddingBoxPath ()Landroid/graphics/Path; public fun setAlpha (I)V public fun setBorderColor (IFF)V public fun setBorderRadius (Lcom/facebook/react/uimanager/style/BorderRadiusProp;Lcom/facebook/react/uimanager/LengthPercentage;)V diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BoxShadowDrawable.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BoxShadowDrawable.kt index 9f8f436265eeaf..25332053feb060 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BoxShadowDrawable.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BoxShadowDrawable.kt @@ -81,7 +81,7 @@ internal class BoxShadowDrawable( } with(canvas) { - clipOutPath(background.borderBoxPath()) + clipOutPath(background.getBorderBoxPath()) drawRenderNode(renderNode) } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CSSBackgroundDrawable.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CSSBackgroundDrawable.java index cd3e0dfed058e5..38bbd075d941af 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CSSBackgroundDrawable.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CSSBackgroundDrawable.java @@ -316,22 +316,35 @@ public int getColor() { return mColor; } - public Path borderBoxPath() { + public Path getBorderBoxPath() { updatePath(); return Preconditions.checkNotNull(mOuterClipPathForBorderRadius); } - public Path paddingBoxPath() { + public Path getPaddingBoxPath() { updatePath(); return Preconditions.checkNotNull(mInnerClipPathForBorderRadius); } + public RectF getPaddingBoxRect() { + RectF insets = getDirectionAwareBorderInsets(); + if (insets == null) { + return new RectF(0, 0, getBounds().width(), getBounds().height()); + } + + return new RectF( + insets.left, + insets.top, + getBounds().width() - insets.right, + getBounds().height() - insets.bottom); + } + private void drawRoundedBackgroundWithBorders(Canvas canvas) { updatePath(); canvas.save(); // Clip outer border - canvas.clipPath(borderBoxPath(), Region.Op.INTERSECT); + canvas.clipPath(getBorderBoxPath(), Region.Op.INTERSECT); // Draws the View without its border first (with background color fill) int useColor = ColorUtils.setAlphaComponent(mColor, getOpacity()); @@ -390,7 +403,7 @@ private void drawRoundedBackgroundWithBorders(Canvas canvas) { mPaint.setStyle(Paint.Style.FILL); // Clip inner border - canvas.clipPath(paddingBoxPath(), Region.Op.DIFFERENCE); + canvas.clipPath(getPaddingBoxPath(), Region.Op.DIFFERENCE); final boolean isRTL = getLayoutDirection() == View.LAYOUT_DIRECTION_RTL; int colorStart = getBorderColor(Spacing.START); @@ -1304,7 +1317,7 @@ public int getBorderColor(int position) { return CSSBackgroundDrawable.colorFromAlphaAndRGBComponents(alpha, rgb); } - public RectF getDirectionAwareBorderInsets() { + private RectF getDirectionAwareBorderInsets() { final float borderWidth = getBorderWidthOrDefaultTo(0, Spacing.ALL); final float borderTopWidth = getBorderWidthOrDefaultTo(borderWidth, Spacing.TOP); final float borderBottomWidth = getBorderWidthOrDefaultTo(borderWidth, Spacing.BOTTOM); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index 3114e2cfdc14e0..e785e56e266540 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -16,7 +16,6 @@ import android.graphics.Color; import android.graphics.Path; import android.graphics.Rect; -import android.graphics.RectF; import android.graphics.drawable.Drawable; import android.graphics.drawable.LayerDrawable; import android.view.MotionEvent; @@ -27,7 +26,6 @@ import androidx.annotation.Nullable; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; -import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReactNoCrashSoftException; import com.facebook.react.bridge.ReactSoftExceptionLogger; import com.facebook.react.bridge.UiThreadUtil; @@ -37,7 +35,6 @@ import com.facebook.react.touch.OnInterceptTouchEventListener; import com.facebook.react.touch.ReactHitSlopView; import com.facebook.react.touch.ReactInterceptingViewGroup; -import com.facebook.react.uimanager.IllegalViewOperationException; import com.facebook.react.uimanager.LengthPercentage; import com.facebook.react.uimanager.MeasureSpecAssertions; import com.facebook.react.uimanager.PointerEvents; @@ -47,15 +44,13 @@ import com.facebook.react.uimanager.ReactOverflowViewWithInset; import com.facebook.react.uimanager.ReactPointerEventsView; import com.facebook.react.uimanager.ReactZIndexedViewGroup; -import com.facebook.react.uimanager.RootView; -import com.facebook.react.uimanager.RootViewUtil; import com.facebook.react.uimanager.ViewGroupDrawingOrderHelper; import com.facebook.react.uimanager.ViewProps; import com.facebook.react.uimanager.common.UIManagerType; import com.facebook.react.uimanager.common.ViewUtil; import com.facebook.react.uimanager.drawable.CSSBackgroundDrawable; import com.facebook.react.uimanager.style.BorderRadiusProp; -import com.facebook.react.uimanager.style.ComputedBorderRadius; +import java.util.Objects; /** * Backing for a React View. Has support for borders, but since borders aren't common, lazy @@ -129,7 +124,6 @@ public void onLayoutChange( private @Nullable OnInterceptTouchEventListener mOnInterceptTouchEventListener; private boolean mNeedsOffscreenAlphaCompositing; private @Nullable ViewGroupDrawingOrderHelper mDrawingOrderHelper; - private @Nullable Path mPath; private float mBackfaceOpacity; private String mBackfaceVisibility; @@ -158,7 +152,6 @@ private void initView() { mOnInterceptTouchEventListener = null; mNeedsOffscreenAlphaCompositing = false; mDrawingOrderHelper = null; - mPath = null; mBackfaceOpacity = 1.f; mBackfaceVisibility = "visible"; } @@ -850,25 +843,18 @@ public Rect getOverflowInset() { @Override protected void dispatchDraw(Canvas canvas) { - try { - dispatchOverflowDraw(canvas); - super.dispatchDraw(canvas); - } catch (NullPointerException | StackOverflowError e) { - // Adding special exception management for StackOverflowError for logging purposes. - // This will be removed in the future. - RootView rootView = RootViewUtil.getRootView(ReactViewGroup.this); - if (rootView != null) { - rootView.handleException(e); + if (mCSSBackgroundDrawable != null + && (Objects.equals(mOverflow, ViewProps.HIDDEN) + || Objects.equals(mOverflow, ViewProps.SCROLL))) { + @Nullable Path paddingBoxPath = mCSSBackgroundDrawable.getPaddingBoxPath(); + if (paddingBoxPath != null) { + canvas.clipPath(paddingBoxPath); } else { - if (getContext() instanceof ReactContext) { - ReactContext reactContext = (ReactContext) getContext(); - reactContext.handleException( - new IllegalViewOperationException("StackOverflowException", this, e)); - } else { - throw e; - } + canvas.clipRect(mCSSBackgroundDrawable.getPaddingBoxRect()); } } + + super.dispatchDraw(canvas); } @Override @@ -887,73 +873,6 @@ protected boolean drawChild(Canvas canvas, View child, long drawingTime) { return result; } - private void dispatchOverflowDraw(Canvas canvas) { - if (mOverflow != null) { - switch (mOverflow) { - case ViewProps.VISIBLE: - if (mPath != null) { - mPath.rewind(); - } - break; - case ViewProps.HIDDEN: - case ViewProps.SCROLL: - float left = 0f; - float top = 0f; - float right = getWidth(); - float bottom = getHeight(); - - boolean hasClipPath = false; - - if (mCSSBackgroundDrawable != null) { - final RectF borderWidth = mCSSBackgroundDrawable.getDirectionAwareBorderInsets(); - - if (borderWidth.top > 0 - || borderWidth.left > 0 - || borderWidth.bottom > 0 - || borderWidth.right > 0) { - left += borderWidth.left; - top += borderWidth.top; - right -= borderWidth.right; - bottom -= borderWidth.bottom; - } - - final ComputedBorderRadius borderRadius = - mCSSBackgroundDrawable.getComputedBorderRadius(); - - if (borderRadius.hasRoundedBorders()) { - if (mPath == null) { - mPath = new Path(); - } - - mPath.rewind(); - mPath.addRoundRect( - new RectF(left, top, right, bottom), - new float[] { - Math.max(borderRadius.getTopLeft() - borderWidth.left, 0), - Math.max(borderRadius.getTopLeft() - borderWidth.top, 0), - Math.max(borderRadius.getTopRight() - borderWidth.right, 0), - Math.max(borderRadius.getTopRight() - borderWidth.top, 0), - Math.max(borderRadius.getBottomRight() - borderWidth.right, 0), - Math.max(borderRadius.getBottomRight() - borderWidth.bottom, 0), - Math.max(borderRadius.getBottomLeft() - borderWidth.left, 0), - Math.max(borderRadius.getBottomLeft() - borderWidth.bottom, 0), - }, - Path.Direction.CW); - canvas.clipPath(mPath); - hasClipPath = true; - } - } - - if (!hasClipPath) { - canvas.clipRect(new RectF(left, top, right, bottom)); - } - break; - default: - break; - } - } - } - public void setOpacityIfPossible(float opacity) { mBackfaceOpacity = opacity; setBackfaceVisibilityDependantOpacity(); diff --git a/packages/rn-tester/js/examples/Border/BorderExample.js b/packages/rn-tester/js/examples/Border/BorderExample.js index 12e9a08c3e3422..e8a7a7e87bd82b 100644 --- a/packages/rn-tester/js/examples/Border/BorderExample.js +++ b/packages/rn-tester/js/examples/Border/BorderExample.js @@ -12,9 +12,11 @@ import type {RNTesterModule} from '../../types/RNTesterTypes'; +import hotdog from '../../assets/hotdog.jpg'; import * as React from 'react'; import { DynamicColorIOS, + Image, Platform, PlatformColor, StyleSheet, @@ -214,6 +216,23 @@ const styles = StyleSheet.create({ ? DynamicColorIOS({light: 'magenta', dark: 'cyan'}) : 'black', }, + borderWithoutClipping: { + borderWidth: 10, + overflow: 'visible', + }, + borderWithClipping: { + borderWidth: 10, + overflow: 'hidden', + }, + borderWithClippingAndRadius: { + borderWidth: 10, + borderRadius: 30, + overflow: 'hidden', + }, + hotdog: { + width: 100, + height: 100, + }, }); export default ({ @@ -477,5 +496,50 @@ export default ({ ); }, }, + { + title: 'Child without clipping', + name: 'child-no-clipping', + description: + '"overflow: visible" will cause child content to show above borders', + render: function (): React.Node { + return ( + + + + ); + }, + }, + { + title: 'Child clipping', + name: 'child-clipping', + description: + '"overflow: hidden" will cause child content to be clipped to borders', + render: function (): React.Node { + return ( + + + + ); + }, + }, + { + title: 'Child clipping with radius', + name: 'child-clipping-radius', + description: + '"overflow: hidden" will cause child content to be clipped to rounded corners', + render: function (): React.Node { + return ( + + + + ); + }, + }, ], }: RNTesterModule);