From 451ff70da422beefccfff5b87024145003bb49ff Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes Acosta Date: Thu, 21 Nov 2024 14:01:02 -0800 Subject: [PATCH] implement correct border-radius scaling on overlapping radii (#47886) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47886 We were not scaling border radii properly when they overlap. We were relying on Android's handling of the issue which is not compliant with the web algorithm leading to **visible gaps between layers when the radii overlapped** We are now following web spec https://www.w3.org/TR/css-backgrounds-3/#corner-overlap Changelog: [Android] [Added] Add overlapping radii resolution logic preventing incorrect rendering Reviewed By: NickGerleman Differential Revision: D66269809 fbshipit-source-id: 4ab7025dc1e41be6205ff6b828617e1e222536a9 --- .../uimanager/drawable/BackgroundDrawable.kt | 7 +-- .../uimanager/drawable/BorderDrawable.kt | 6 +- .../uimanager/drawable/OutlineDrawable.kt | 4 +- .../uimanager/style/BorderRadiusStyle.kt | 57 +++++++++++++++++-- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BackgroundDrawable.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BackgroundDrawable.kt index cf93a29bf45..4fa54fb6f8b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BackgroundDrawable.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BackgroundDrawable.kt @@ -21,7 +21,7 @@ import android.graphics.RectF import android.graphics.Shader import android.graphics.drawable.Drawable import com.facebook.react.uimanager.PixelUtil.dpToPx -import com.facebook.react.uimanager.PixelUtil.toDIPFromPixel +import com.facebook.react.uimanager.PixelUtil.pxToDp import com.facebook.react.uimanager.style.BackgroundImageLayer import com.facebook.react.uimanager.style.BorderInsets import com.facebook.react.uimanager.style.BorderRadiusStyle @@ -184,10 +184,7 @@ internal class BackgroundDrawable( computedBorderInsets = computeBorderInsets() computedBorderRadius = borderRadius?.resolve( - layoutDirection, - context, - toDIPFromPixel(bounds.width().toFloat()), - toDIPFromPixel(bounds.height().toFloat())) + layoutDirection, context, bounds.width().pxToDp(), bounds.height().pxToDp()) if (computedBorderRadius?.hasRoundedBorders() == true && computedBorderRadius?.isUniform() == false) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BorderDrawable.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BorderDrawable.kt index a6c331df7d3..20c1e8cea5b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BorderDrawable.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/BorderDrawable.kt @@ -24,8 +24,8 @@ import android.graphics.drawable.Drawable import android.os.Build import com.facebook.react.uimanager.FloatUtil.floatsEqual import com.facebook.react.uimanager.LengthPercentage -import com.facebook.react.uimanager.PixelUtil import com.facebook.react.uimanager.PixelUtil.dpToPx +import com.facebook.react.uimanager.PixelUtil.pxToDp import com.facebook.react.uimanager.Spacing import com.facebook.react.uimanager.style.BorderColors import com.facebook.react.uimanager.style.BorderInsets @@ -703,8 +703,8 @@ internal class BorderDrawable( this.borderRadius?.resolve( layoutDirection, this.context, - PixelUtil.toDIPFromPixel(outerClipTempRectForBorderRadius?.width() ?: 0f), - PixelUtil.toDIPFromPixel(outerClipTempRectForBorderRadius?.height() ?: 0f), + outerClipTempRectForBorderRadius?.width()?.pxToDp() ?: 0f, + outerClipTempRectForBorderRadius?.height()?.pxToDp() ?: 0f, ) val topLeftRadius = computedBorderRadius?.topLeft?.toPixelFromDIP() ?: CornerRadii(0f, 0f) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/OutlineDrawable.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/OutlineDrawable.kt index cdc0edde2d8..df181b2ba11 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/OutlineDrawable.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/OutlineDrawable.kt @@ -18,7 +18,7 @@ import android.graphics.PathEffect import android.graphics.PixelFormat import android.graphics.RectF import android.graphics.drawable.Drawable -import com.facebook.react.uimanager.PixelUtil.dpToPx +import com.facebook.react.uimanager.PixelUtil.pxToDp import com.facebook.react.uimanager.style.BorderRadiusStyle import com.facebook.react.uimanager.style.ComputedBorderRadius import com.facebook.react.uimanager.style.CornerRadii @@ -123,7 +123,7 @@ internal class OutlineDrawable( computedBorderRadius = borderRadius?.resolve( - layoutDirection, context, bounds.width().dpToPx(), bounds.height().dpToPx()) + layoutDirection, context, bounds.width().pxToDp(), bounds.height().pxToDp()) updateOutlineRect() if (computedBorderRadius != null && computedBorderRadius?.hasRoundedBorders() == true) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BorderRadiusStyle.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BorderRadiusStyle.kt index 18df34fe022..ea6f935974d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BorderRadiusStyle.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BorderRadiusStyle.kt @@ -107,11 +107,11 @@ public data class BorderRadiusStyle( width: Float, height: Float, ): ComputedBorderRadius { - val zeroRadii: CornerRadii = CornerRadii(0f, 0f) + val zeroRadii = CornerRadii(0f, 0f) return when (layoutDirection) { LayoutDirection.LTR -> - ComputedBorderRadius( + ensureNoOverlap( topLeft = (startStart ?: topStart ?: topLeft ?: uniform)?.resolve(width, height) ?: zeroRadii, @@ -123,10 +123,12 @@ public data class BorderRadiusStyle( bottomRight = (endEnd ?: bottomEnd ?: bottomRight ?: uniform)?.resolve(width, height) ?: zeroRadii, + width = width, + height = height, ) LayoutDirection.RTL -> if (I18nUtil.instance.doLeftAndRightSwapInRTL(context)) { - ComputedBorderRadius( + ensureNoOverlap( topLeft = (endStart ?: topEnd ?: topRight ?: uniform)?.resolve(width, height) ?: zeroRadii, @@ -139,9 +141,11 @@ public data class BorderRadiusStyle( bottomRight = (startEnd ?: bottomEnd ?: bottomLeft ?: uniform)?.resolve(width, height) ?: zeroRadii, + width = width, + height = height, ) } else { - ComputedBorderRadius( + ensureNoOverlap( topLeft = (endStart ?: topEnd ?: topLeft ?: uniform)?.resolve(width, height) ?: zeroRadii, topRight = @@ -153,9 +157,54 @@ public data class BorderRadiusStyle( bottomRight = (startEnd ?: bottomEnd ?: bottomRight ?: uniform)?.resolve(width, height) ?: zeroRadii, + width = width, + height = height, ) } else -> throw IllegalArgumentException("Expected?.resolved layout direction") } } + + /** + * "Corner curves must not overlap: When the sum of any two adjacent border radii exceeds the size + * of the border box, UAs must proportionally reduce the used values of all border radii until + * none of them overlap." Source: https://www.w3.org/TR/css-backgrounds-3/#corner-overlap + */ + private fun ensureNoOverlap( + topLeft: CornerRadii, + topRight: CornerRadii, + bottomLeft: CornerRadii, + bottomRight: CornerRadii, + width: Float, + height: Float + ): ComputedBorderRadius { + val leftInset = topLeft.horizontal + bottomLeft.horizontal + val topInset = topLeft.vertical + topRight.vertical + val rightInset = topRight.horizontal + bottomRight.horizontal + val bottomInset = bottomLeft.vertical + bottomRight.vertical + + val leftInsetScale = if (leftInset > 0) minOf(1.0f, height / leftInset) else 0f + val topInsetScale = if (topInset > 0) minOf(1.0f, width / topInset) else 0f + val rightInsetScale = if (rightInset > 0) minOf(1.0f, height / rightInset) else 0f + val bottomInsetScale = if (bottomInset > 0) minOf(1.0f, width / bottomInset) else 0f + + return ComputedBorderRadius( + topLeft = + CornerRadii( + topLeft.horizontal * minOf(topInsetScale, leftInsetScale), + topLeft.vertical * minOf(topInsetScale, leftInsetScale)), + topRight = + CornerRadii( + topRight.horizontal * minOf(topInsetScale, rightInsetScale), + topRight.vertical * minOf(topInsetScale, rightInsetScale)), + bottomLeft = + CornerRadii( + bottomLeft.horizontal * minOf(bottomInsetScale, leftInsetScale), + bottomLeft.vertical * minOf(bottomInsetScale, leftInsetScale)), + bottomRight = + CornerRadii( + bottomRight.horizontal * minOf(bottomInsetScale, rightInsetScale), + bottomRight.vertical * minOf(bottomInsetScale, rightInsetScale)), + ) + } }