Apply modifiers as they change in UIViewFlexContainer (#2358)

* Apply modifiers as they change in UIViewFlexContainer This reduces the amount of preparation required to measure a layout. * Fix the remove index. Hooray for tests.
cashapp · Oct 3, 2024 · b3fc024 · b3fc024
1 parent 372600f
commit b3fc024
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 60 deletions.
diff --git a/build-support/src/main/resources/app/cash/redwood/buildsupport/flexboxHelpers.kt b/build-support/src/main/resources/app/cash/redwood/buildsupport/flexboxHelpers.kt
@@ -98,32 +98,36 @@ internal fun CrossAxisAlignment.toAlignSelf() = when (this) {
  *
  * Also also note that `Float.NaN` is used by these properties, and that `Float.NaN != Float.NaN`.
  * So even deciding whether a value has changed is tricky.
+ *
+ * Returns true if the node became dirty as a consequence of this call.
  */
-internal fun Node.applyModifier(parentModifier: Modifier, density: Density) {
+internal fun Node.applyModifier(parentModifier: Modifier, density: Density): Boolean {
+  val wasDirty = isDirty()
+
   // Avoid unnecessary mutations to the Node because it marks itself dirty its properties change.
-  var oldMarginStart = marginStart
+  val oldMarginStart = marginStart
   var newMarginStart = Float.NaN
-  var oldMarginEnd = marginEnd
+  val oldMarginEnd = marginEnd
   var newMarginEnd = Float.NaN
-  var oldMarginTop = marginTop
+  val oldMarginTop = marginTop
   var newMarginTop = Float.NaN
-  var oldMarginBottom = marginBottom
+  val oldMarginBottom = marginBottom
   var newMarginBottom = Float.NaN
-  var oldAlignSelf = alignSelf
+  val oldAlignSelf = alignSelf
   var newAlignSelf = AlignSelf.Auto
-  var oldRequestedMinWidth = requestedMinWidth
+  val oldRequestedMinWidth = requestedMinWidth
   var newRequestedMinWidth = Float.NaN
-  var oldRequestedMaxWidth = requestedMaxWidth
+  val oldRequestedMaxWidth = requestedMaxWidth
   var newRequestedMaxWidth = Float.NaN
-  var oldRequestedMinHeight = requestedMinHeight
+  val oldRequestedMinHeight = requestedMinHeight
   var newRequestedMinHeight = Float.NaN
-  var oldRequestedMaxHeight = requestedMaxHeight
+  val oldRequestedMaxHeight = requestedMaxHeight
   var newRequestedMaxHeight = Float.NaN
-  var oldFlexGrow = flexGrow
+  val oldFlexGrow = flexGrow
   var newFlexGrow = 0f
-  var oldFlexShrink = flexShrink
+  val oldFlexShrink = flexShrink
   var newFlexShrink = 0f
-  var oldFlexBasis = flexBasis
+  val oldFlexBasis = flexBasis
   var newFlexBasis = -1f
 
   parentModifier.forEachScoped { childModifier ->
@@ -193,6 +197,8 @@ internal fun Node.applyModifier(parentModifier: Modifier, density: Density) {
   if (newFlexGrow neq oldFlexGrow) flexGrow = newFlexGrow
   if (newFlexShrink neq oldFlexShrink) flexShrink = newFlexShrink
   if (newFlexBasis neq oldFlexBasis) flexBasis = newFlexBasis
+
+  return !wasDirty && isDirty()
 }
 
 /**

diff --git a/redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/UIViewBox.kt b/redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/UIViewBox.kt
@@ -97,21 +97,20 @@ internal class UIViewBox :
 
     val children = UIViewChildren(
       container = this,
-      insert = { widget, view, _, index ->
+      insert = { index, widget ->
         if (widget is ResizableWidget<*>) {
           widget.sizeListener = object : ResizableWidget.SizeListener {
             override fun invalidateSize() {
               this@View.invalidateSize()
             }
           }
         }
-        insertSubview(view, index.convert<NSInteger>())
+        insertSubview(widget.value, index.convert<NSInteger>())
       },
       remove = { index, count ->
-        val views = Array(count) {
-          typedSubviews[index].also(UIView::removeFromSuperview)
+        for (i in index until index + count) {
+          typedSubviews[index].removeFromSuperview()
         }
-        return@UIViewChildren views
       },
       invalidateSize = ::invalidateSize,
     )

diff --git a/...layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/UIViewFlexContainer.kt b/...layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/UIViewFlexContainer.kt
@@ -36,32 +36,35 @@ internal class UIViewFlexContainer(
 ) : YogaFlexContainer<UIView>,
   ResizableWidget<UIView>,
   ChangeListener {
-  private val yogaView: YogaUIView = YogaUIView(
-    applyModifier = { node, _ ->
-      node.applyModifier(node.context as Modifier, Density.Default)
-    },
-  )
+  private val yogaView: YogaUIView = YogaUIView()
   override val rootNode: Node get() = yogaView.rootNode
   override val density: Density get() = Density.Default
   override val value: UIView get() = yogaView
   override val children: UIViewChildren = UIViewChildren(
     container = value,
-    insert = { widget, view, modifier, index ->
-      val node = view.asNode(context = modifier)
+    insert = { index, widget ->
+      val view = widget.value
+      val node = view.asNode()
       if (widget is ResizableWidget<*>) {
         widget.sizeListener = NodeSizeListener(node, view, this@UIViewFlexContainer)
       }
       yogaView.rootNode.children.add(index, node)
+
+      // Always apply changes *after* adding a node to its parent.
+      node.applyModifier(widget.modifier, density)
+
       value.insertSubview(view, index.convert<NSInteger>())
     },
     remove = { index, count ->
       yogaView.rootNode.children.remove(index, count)
-      Array(count) {
-        value.typedSubviews[index].also(UIView::removeFromSuperview)
+      for (i in index until index + count) {
+        value.typedSubviews[index].removeFromSuperview()
       }
     },
-    updateModifier = { modifier, index ->
-      yogaView.rootNode.children[index].context = modifier
+    onModifierUpdated = { index, widget ->
+      val node = yogaView.rootNode.children[index]
+      val nodeBecameDirty = node.applyModifier(widget.modifier, density)
+      invalidateSize(nodeBecameDirty = nodeBecameDirty)
     },
     invalidateSize = ::invalidateSize,
   )
@@ -93,8 +96,8 @@ internal class UIViewFlexContainer(
     invalidateSize()
   }
 
-  internal fun invalidateSize() {
-    if (rootNode.markDirty()) {
+  internal fun invalidateSize(nodeBecameDirty: Boolean = false) {
+    if (rootNode.markDirty() || nodeBecameDirty) {
       // The node was newly-dirty. Propagate that up the tree.
       val sizeListener = this.sizeListener
       if (sizeListener != null) {
@@ -108,10 +111,9 @@ internal class UIViewFlexContainer(
   }
 }
 
-private fun UIView.asNode(context: Any?): Node {
+private fun UIView.asNode(): Node {
   val childNode = Node()
   childNode.measureCallback = UIViewMeasureCallback(this)
-  childNode.context = context
   return childNode
 }
 

diff --git a/redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/YogaUIView.kt b/redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/YogaUIView.kt
@@ -25,9 +25,7 @@ import platform.UIKit.UIScrollViewDelegateProtocol
 import platform.UIKit.UIView
 import platform.UIKit.UIViewNoIntrinsicMetric
 
-internal class YogaUIView(
-  private val applyModifier: (Node, Int) -> Unit,
-) : UIScrollView(cValue { CGRectZero }), UIScrollViewDelegateProtocol {
+internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDelegateProtocol {
   val rootNode = Node()
 
   var widthConstraint = Constraint.Wrap
@@ -101,10 +99,6 @@ internal class YogaUIView(
     rootNode.requestedWidth = width
     rootNode.requestedHeight = height
 
-    for ((index, node) in rootNode.children.withIndex()) {
-      applyModifier(node, index)
-    }
-
     rootNode.measureOnly(Size.UNDEFINED, Size.UNDEFINED)
 
     return CGSizeMake(rootNode.width.toDouble(), rootNode.height.toDouble())

diff --git a/redwood-widget/api/redwood-widget.klib.api b/redwood-widget/api/redwood-widget.klib.api
@@ -109,7 +109,7 @@ abstract interface <#A: kotlin/Any> app.cash.redwood.widget/ResizableWidget : ap
 
 // Targets: [ios]
 final class app.cash.redwood.widget/UIViewChildren : app.cash.redwood.widget/Widget.Children<platform.UIKit/UIView> { // app.cash.redwood.widget/UIViewChildren|null[0]
-    constructor <init>(platform.UIKit/UIView, kotlin/Function4<app.cash.redwood.widget/Widget<platform.UIKit/UIView>, platform.UIKit/UIView, app.cash.redwood/Modifier, kotlin/Int, kotlin/Unit> = ..., kotlin/Function2<kotlin/Int, kotlin/Int, kotlin/Array<platform.UIKit/UIView>> = ..., kotlin/Function2<app.cash.redwood/Modifier, kotlin/Int, kotlin/Unit> = ..., kotlin/Function0<kotlin/Unit> = ...) // app.cash.redwood.widget/UIViewChildren.<init>|<init>(platform.UIKit.UIView;kotlin.Function4<app.cash.redwood.widget.Widget<platform.UIKit.UIView>,platform.UIKit.UIView,app.cash.redwood.Modifier,kotlin.Int,kotlin.Unit>;kotlin.Function2<kotlin.Int,kotlin.Int,kotlin.Array<platform.UIKit.UIView>>;kotlin.Function2<app.cash.redwood.Modifier,kotlin.Int,kotlin.Unit>;kotlin.Function0<kotlin.Unit>){}[0]
+    constructor <init>(platform.UIKit/UIView, kotlin/Function2<kotlin/Int, app.cash.redwood.widget/Widget<platform.UIKit/UIView>, kotlin/Unit> = ..., kotlin/Function2<kotlin/Int, kotlin/Int, kotlin/Unit> = ..., kotlin/Function0<kotlin/Unit> = ..., kotlin/Function2<kotlin/Int, app.cash.redwood.widget/Widget<platform.UIKit/UIView>, kotlin/Unit> = ...) // app.cash.redwood.widget/UIViewChildren.<init>|<init>(platform.UIKit.UIView;kotlin.Function2<kotlin.Int,app.cash.redwood.widget.Widget<platform.UIKit.UIView>,kotlin.Unit>;kotlin.Function2<kotlin.Int,kotlin.Int,kotlin.Unit>;kotlin.Function0<kotlin.Unit>;kotlin.Function2<kotlin.Int,app.cash.redwood.widget.Widget<platform.UIKit.UIView>,kotlin.Unit>){}[0]
 
     final val widgets // app.cash.redwood.widget/UIViewChildren.widgets|{}widgets[0]
         final fun <get-widgets>(): kotlin.collections/List<app.cash.redwood.widget/Widget<platform.UIKit/UIView>> // app.cash.redwood.widget/UIViewChildren.widgets.<get-widgets>|<get-widgets>(){}[0]

diff --git a/redwood-widget/src/iosMain/kotlin/app/cash/redwood/widget/UIViewChildren.kt b/redwood-widget/src/iosMain/kotlin/app/cash/redwood/widget/UIViewChildren.kt
@@ -15,7 +15,6 @@
  */
 package app.cash.redwood.widget
 
-import app.cash.redwood.Modifier
 import kotlinx.cinterop.convert
 import platform.UIKit.UIStackView
 import platform.UIKit.UIView
@@ -24,40 +23,48 @@ import platform.darwin.NSInteger
 @ObjCName("UIViewChildren", exact = true)
 public class UIViewChildren(
   private val container: UIView,
-  private val insert: (Widget<UIView>, UIView, Modifier, Int) -> Unit = when (container) {
-    is UIStackView -> { _, view, _, index -> container.insertArrangedSubview(view, index.convert()) }
-    else -> { _, view, _, index -> container.insertSubview(view, index.convert<NSInteger>()) }
+  private val insert: (index: Int, widget: Widget<UIView>) -> Unit = when (container) {
+    is UIStackView -> { index, widget ->
+      container.insertArrangedSubview(widget.value, index.convert())
+    }
+    else -> { index, widget ->
+      container.insertSubview(widget.value, index.convert<NSInteger>())
+    }
   },
-  private val remove: (index: Int, count: Int) -> Array<UIView> = when (container) {
-    is UIStackView -> { index, count -> container.typedArrangedSubviews.remove(index, count) }
-    else -> { index, count -> container.typedSubviews.remove(index, count) }
+  private val remove: (index: Int, count: Int) -> Unit = when (container) {
+    is UIStackView -> { index, count ->
+      container.typedArrangedSubviews.removeFromSuperview(index, count)
+    }
+    else -> { index, count ->
+      container.typedSubviews.removeFromSuperview(index, count)
+    }
   },
-  private val updateModifier: (Modifier, Int) -> Unit = { _, _ -> },
   private val invalidateSize: () -> Unit = { (container.superview ?: container).setNeedsLayout() },
+  private val onModifierUpdated: (index: Int, widget: Widget<UIView>) -> Unit = { _, _ ->
+    invalidateSize()
+  },
 ) : Widget.Children<UIView> {
   private val _widgets = ArrayList<Widget<UIView>>()
   override val widgets: List<Widget<UIView>> get() = _widgets
 
   override fun insert(index: Int, widget: Widget<UIView>) {
     _widgets.add(index, widget)
-    insert(widget, widget.value, widget.modifier, index)
+    insert.invoke(index, widget)
     invalidateSize()
   }
 
   override fun move(fromIndex: Int, toIndex: Int, count: Int) {
     _widgets.move(fromIndex, toIndex, count)
 
-    val subviews = remove.invoke(fromIndex, count)
+    remove.invoke(fromIndex, count)
 
     val newIndex = if (toIndex > fromIndex) {
       toIndex - count
     } else {
       toIndex
     }
-    subviews.forEachIndexed { offset, view ->
-      val subviewIndex = newIndex + offset
-      val widget = widgets[subviewIndex]
-      insert(widget, view, widget.modifier, subviewIndex)
+    for (i in newIndex until newIndex + count) {
+      insert.invoke(i, widgets[i])
     }
     invalidateSize()
   }
@@ -76,8 +83,7 @@ public class UIViewChildren(
   }
 
   override fun onModifierUpdated(index: Int, widget: Widget<UIView>) {
-    updateModifier(widget.modifier, index)
-    invalidateSize()
+    onModifierUpdated.invoke(index, widget)
   }
 
   override fun detach() {
@@ -92,8 +98,8 @@ public class UIViewChildren(
   }
 }
 
-private fun List<UIView>.remove(index: Int, count: Int): Array<UIView> {
-  return Array(count) { offset ->
-    this[index + offset].also(UIView::removeFromSuperview)
+private fun List<UIView>.removeFromSuperview(index: Int, count: Int) {
+  for (i in index until index + count) {
+    this[i].removeFromSuperview()
   }
 }
diff --git a/redwood-yoga/src/commonMain/kotlin/app/cash/redwood/yoga/Node.kt b/redwood-yoga/src/commonMain/kotlin/app/cash/redwood/yoga/Node.kt
@@ -114,6 +114,8 @@ public class Node internal constructor(
     return true
   }
 
+  public fun isDirty(): Boolean = native.isDirty()
+
   public fun markEverythingDirty() {
     native.markDirtyAndPropogateDownwards()
   }