From 61f01ade4288028757594f286f785d1a9fa1dc4e Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes Acosta Date: Tue, 19 Nov 2024 12:13:42 -0800 Subject: [PATCH] Optimize CompositeBackgroundDrawable (#47618) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47618 Its not optimal to reconstruct CompositeBackgroundDrawable everytime we add a layer to it. With this change I'm adding an optimization to modify the underlying `LayerDrawable` in place instead of reconstructing everything. `LayerDrawable` has a pretty constrained API and some weirdish behaviors. - `addLayer` - This API doesnt set the callback for the recently added layer so after adding the layer we also need to manually set the callback - Mutating `LayerDrawable` in-place is not straightforward, I had to add a function that will figure out where each layer should be inserted - `LayerDrawable` doesn't allow deleting an element which means that for this case in particular we do need to re-create `CompositeBackgroundDrawable` - Newer Android versions allow `null` on `LayerDrawable` layers, but older versions do not, this implementation is mostly done this way to accommodate older versions. But also, even though newer versions can have `null` set on a layer `LayerDrawable` still doesn't handle it well and we get some bugs when removing and inserting a layer which the current implementation handles by not relying on null This is all feature flagged. since it will only be enabled with the new drawables Changelog: [Internal] Reviewed By: joevilches Differential Revision: D65907786 fbshipit-source-id: c18b898ddfe58faa5b90714945ffcc82d471051d --- .../uimanager/BackgroundStyleApplicator.kt | 5 +- .../drawable/CompositeBackgroundDrawable.kt | 236 ++++++++++-------- 2 files changed, 136 insertions(+), 105 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BackgroundStyleApplicator.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BackgroundStyleApplicator.kt index c2b180482c5..0040b6e3b5a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BackgroundStyleApplicator.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/BackgroundStyleApplicator.kt @@ -336,9 +336,8 @@ public object BackgroundStyleApplicator { } } - view.background = - ensureCompositeBackgroundDrawable(view) - .withNewShadows(outerShadows = outerShadows, innerShadows = innerShadows) + view.background = ensureCompositeBackgroundDrawable(view).withNewOuterShadow(outerShadows) + view.background = ensureCompositeBackgroundDrawable(view).withNewInnerShadow(innerShadows) } @JvmStatic diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CompositeBackgroundDrawable.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CompositeBackgroundDrawable.kt index 43eff2d4644..7105d09d60a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CompositeBackgroundDrawable.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/drawable/CompositeBackgroundDrawable.kt @@ -15,6 +15,7 @@ import android.graphics.drawable.Drawable import android.graphics.drawable.LayerDrawable import android.os.Build import com.facebook.react.common.annotations.UnstableReactNativeAPI +import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags import com.facebook.react.uimanager.style.BorderInsets import com.facebook.react.uimanager.style.BorderRadiusStyle @@ -32,7 +33,7 @@ internal class CompositeBackgroundDrawable( public val originalBackground: Drawable? = null, /** Non-inset box shadows */ - public val outerShadows: LayerDrawable? = null, + outerShadows: LayerDrawable? = null, /** * CSS background layer and border rendering @@ -43,36 +44,41 @@ internal class CompositeBackgroundDrawable( public val cssBackground: CSSBackgroundDrawable? = null, /** Background rendering Layer */ - public val background: BackgroundDrawable? = null, + background: BackgroundDrawable? = null, /** Border rendering Layer */ - public val border: BorderDrawable? = null, + border: BorderDrawable? = null, /** TouchableNativeFeeback set selection background, like "SelectableBackground" */ - public val feedbackUnderlay: Drawable? = null, + feedbackUnderlay: Drawable? = null, /** Inset box-shadows */ - public val innerShadows: LayerDrawable? = null, + innerShadows: LayerDrawable? = null, /** Outline */ - public val outline: OutlineDrawable? = null, -) : - LayerDrawable( - listOf( - originalBackground, - // z-ordering of user-provided shadow-list is opposite direction of LayerDrawable - // z-ordering - // https://drafts.csswg.org/css-backgrounds/#shadow-layers - outerShadows, - cssBackground, - background, - border, - feedbackUnderlay, - innerShadows, - outline) - .toTypedArray()) { + outline: OutlineDrawable? = null, +) : LayerDrawable(emptyArray()) { + public var outerShadows: LayerDrawable? = outerShadows + private set + + public var background: BackgroundDrawable? = background + private set + + public var border: BorderDrawable? = border + private set + + public var feedbackUnderlay: Drawable? = feedbackUnderlay + private set + + public var innerShadows: LayerDrawable? = innerShadows + private set + + public var outline: OutlineDrawable? = outline + private set + // Holder value for currently set insets public var borderInsets: BorderInsets? = null + // Holder value for currently set border radius public var borderRadius: BorderRadiusStyle? = null @@ -81,6 +87,15 @@ internal class CompositeBackgroundDrawable( // previous ones. E.g. an EditText style may set padding on a TextInput, but we don't want to // constrain background color to the area inside of the padding. setPaddingMode(LayerDrawable.PADDING_MODE_STACK) + + addLayer(originalBackground, ORIGINAL_BACKGROUND_ID) + addLayer(outerShadows, OUTER_SHADOWS_ID) + addLayer(cssBackground, CSS_BACKGROUND_ID) + addLayer(background, BACKGROUND_ID) + addLayer(border, BORDER_ID) + addLayer(feedbackUnderlay, FEEDBACK_UNDERLAY_ID) + addLayer(innerShadows, INNER_SHADOWS_ID) + addLayer(outline, OUTLINE_ID) } public fun withNewCssBackground( @@ -102,92 +117,71 @@ internal class CompositeBackgroundDrawable( } } - public fun withNewBackground(background: BackgroundDrawable?): CompositeBackgroundDrawable { - return CompositeBackgroundDrawable( - context, - originalBackground, - outerShadows, - cssBackground, - background, - border, - feedbackUnderlay, - innerShadows, - outline) - .also { composite -> - composite.borderInsets = this.borderInsets - composite.borderRadius = this.borderRadius - } + public fun withNewOuterShadow(outerShadows: LayerDrawable?): CompositeBackgroundDrawable = + withNewLayer(outerShadows, OUTER_SHADOWS_ID, this::outerShadows::set) + + public fun withNewBackground(background: BackgroundDrawable?): CompositeBackgroundDrawable = + withNewLayer(background, BACKGROUND_ID, this::background::set) + + public fun withNewBorder(border: BorderDrawable?): CompositeBackgroundDrawable = + withNewLayer(border, BORDER_ID, this::border::set) + + public fun withNewFeedbackUnderlay(newUnderlay: Drawable?): CompositeBackgroundDrawable = + withNewLayer(newUnderlay, FEEDBACK_UNDERLAY_ID, this::feedbackUnderlay::set) + + public fun withNewInnerShadow(innerShadows: LayerDrawable?): CompositeBackgroundDrawable = + withNewLayer(innerShadows, INNER_SHADOWS_ID, this::innerShadows::set) + + public fun withNewOutline(outline: OutlineDrawable?): CompositeBackgroundDrawable = + withNewLayer(outline, OUTLINE_ID, this::outline::set) + + /** @return true if the layer was updated, false if it was not */ + private fun updateLayer(layer: Drawable?, id: Int): Boolean { + if (layer == null) { + return findDrawableByLayerId(id) == null + } + + if (findDrawableByLayerId(id) == null) { + insertNewLayer(layer, id) + } else { + setDrawableByLayerId(id, layer) + } + invalidateSelf() + return true } - public fun withNewShadows( - outerShadows: LayerDrawable?, - innerShadows: LayerDrawable? - ): CompositeBackgroundDrawable { - return CompositeBackgroundDrawable( - context, - originalBackground, - outerShadows, - cssBackground, - background, - border, - feedbackUnderlay, - innerShadows, - outline) - .also { composite -> - composite.borderInsets = this.borderInsets - composite.borderRadius = this.borderRadius - } + private fun insertNewLayer(layer: Drawable?, id: Int) { + layer ?: return + + if (numberOfLayers == 0) { + addLayer(layer, id) + return + } + + for (i in 0.. - composite.borderInsets = this.borderInsets - composite.borderRadius = this.borderRadius - } - } + private fun addLayer(layer: Drawable?, id: Int) { + if (layer == null) { + return + } - public fun withNewOutline(outline: OutlineDrawable): CompositeBackgroundDrawable { - return CompositeBackgroundDrawable( - context, - originalBackground, - outerShadows, - cssBackground, - background, - border, - feedbackUnderlay, - innerShadows, - outline) - .also { composite -> - composite.borderInsets = this.borderInsets - composite.borderRadius = this.borderRadius - } - } - - public fun withNewFeedbackUnderlay(newUnderlay: Drawable?): CompositeBackgroundDrawable { - return CompositeBackgroundDrawable( - context, - originalBackground, - outerShadows, - cssBackground, - background, - border, - newUnderlay, - innerShadows, - outline) - .also { composite -> - composite.borderInsets = this.borderInsets - composite.borderRadius = this.borderRadius - } + addLayer(layer) + layer.callback = this + setId(numberOfLayers - 1, id) + invalidateSelf() } /* Android's elevation implementation requires this to be implemented to know where to draw the @@ -226,4 +220,42 @@ internal class CompositeBackgroundDrawable( outline.setRect(bounds) } } + + private fun withNewLayer( + newLayer: T, + id: Int, + setNewLayer: (T) -> Unit, + ): CompositeBackgroundDrawable where T : Drawable? { + setNewLayer(newLayer) + if (ReactNativeFeatureFlags.enableNewBackgroundAndBorderDrawables()) { + if (updateLayer(newLayer, id)) { + return this + } + } + return CompositeBackgroundDrawable( + context, + originalBackground, + outerShadows, + cssBackground, + background, + border, + feedbackUnderlay, + innerShadows, + outline) + .also { composite -> + composite.borderInsets = this.borderInsets + composite.borderRadius = this.borderRadius + } + } + + private companion object { + private const val ORIGINAL_BACKGROUND_ID: Int = 0 + private const val OUTER_SHADOWS_ID: Int = 1 + private const val CSS_BACKGROUND_ID: Int = 2 + private const val BACKGROUND_ID: Int = 3 + private const val BORDER_ID: Int = 4 + private const val FEEDBACK_UNDERLAY_ID: Int = 5 + private const val INNER_SHADOWS_ID: Int = 6 + private const val OUTLINE_ID: Int = 7 + } }