From 53457998a6fdc56ed3942be7ffc7d02d851695ac Mon Sep 17 00:00:00 2001 From: Jesse Wilson at Work Date: Wed, 23 Oct 2024 13:34:35 -0400 Subject: [PATCH] Implement ViewBox with manual measurement (#2397) * Implement ViewBox with manual measurement Previously ViewBox delegated to FrameLayout. This worked okay but it was potentially difficult to track behaviors between View and UIView. This implementation is based on the UIViewBox class implementation. This fixes a bug where layouts incorrectly didn't handle changes when modifiers were removed. This changes how Box's margins are implemented. Previously the margins were external, which required the hosting layout to honor them as layout constraints. That was not supported when the enclosing layout was a Row or Column. * Sort imports --- CHANGELOG.md | 1 + .../app/cash/redwood/layout/view/ViewBox.kt | 355 +++++++++++------- ..._RTL_testChildrenModifierChanges_empty.png | 4 +- ...ayout.view_ViewBoxTest_RTL_testMargins.png | 4 +- ...iewBoxTest_RTL_testMarginsAndAlignment.png | 4 +- ...Test_testChildrenModifierChanges_empty.png | 4 +- ...od.layout.view_ViewBoxTest_testMargins.png | 4 +- ...ew_ViewBoxTest_testMarginsAndAlignment.png | 4 +- 8 files changed, 239 insertions(+), 141 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a31f6699e..2c6759d496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Changed: Fixed: - Fix a layout bug where children of fixed-with `Row` containers were assigned the wrong width. - Fix inconsistencies between iOS and Android for `Column` and `Row` layouts. +- Correctly update the layout when a Box's child's modifiers are removed. Breaking: - Replace `CodeListener` with new functions in `RedwoodView`. This puts the loading/error/ready state with the UI that displays that state. diff --git a/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt b/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt index de70eafcb4..bd0de8026a 100644 --- a/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt +++ b/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt @@ -16,19 +16,17 @@ package app.cash.redwood.layout.view import android.content.Context -import android.view.Gravity import android.view.View +import android.view.View.LAYOUT_DIRECTION_LTR +import android.view.View.MeasureSpec +import android.view.View.MeasureSpec.makeMeasureSpec import android.view.ViewGroup -import android.view.ViewGroup.LayoutParams.MATCH_PARENT -import android.view.ViewGroup.LayoutParams.WRAP_CONTENT -import android.widget.FrameLayout -import androidx.core.view.updateLayoutParams import app.cash.redwood.Modifier import app.cash.redwood.layout.api.Constraint import app.cash.redwood.layout.api.CrossAxisAlignment import app.cash.redwood.layout.modifier.Height import app.cash.redwood.layout.modifier.HorizontalAlignment -import app.cash.redwood.layout.modifier.Margin as MarginModifier +import app.cash.redwood.layout.modifier.Margin as RedwoodMargin import app.cash.redwood.layout.modifier.Size import app.cash.redwood.layout.modifier.VerticalAlignment import app.cash.redwood.layout.modifier.Width @@ -36,17 +34,18 @@ import app.cash.redwood.layout.widget.Box import app.cash.redwood.ui.Density import app.cash.redwood.ui.Margin import app.cash.redwood.widget.ViewGroupChildren +import app.cash.redwood.widget.Widget internal class ViewBox( context: Context, -) : FrameLayout(context), +) : ViewGroup(context), Box { private val density = Density(context.resources) private var horizontalAlignment = CrossAxisAlignment.Start private var verticalAlignment = CrossAxisAlignment.Start - private var width = Constraint.Wrap - private var height = Constraint.Wrap - private var margin: Margin? = null + private var widthConstraint = Constraint.Wrap + private var heightConstraint = Constraint.Wrap + private var margin: Margin = Margin.Zero override var modifier: Modifier = Modifier @@ -54,171 +53,269 @@ internal class ViewBox( override val children = ViewGroupChildren(this) - override fun generateDefaultLayoutParams(): LayoutParams { - return LayoutParams(WRAP_CONTENT, WRAP_CONTENT) - } + private val measurer = Measurer() override fun width(width: Constraint) { - this.width = width - invalidate() + this.widthConstraint = width + requestLayout() } override fun height(height: Constraint) { - this.height = height - invalidate() - } - - override fun setLayoutParams(params: ViewGroup.LayoutParams?) { - if (params != null) { - margin?.let { margin -> - maybeUpdateLayoutParams(margin, params) - } - } - super.setLayoutParams(params) + this.heightConstraint = height + requestLayout() } override fun margin(margin: Margin) { this.margin = margin - - layoutParams?.let { params -> - maybeUpdateLayoutParams(margin, params) - // Write instance back out to indicate it has changed. - super.setLayoutParams(params) - } - } - - private fun maybeUpdateLayoutParams(margin: Margin, params: ViewGroup.LayoutParams) { - if (params is MarginLayoutParams) { - with(density) { - params.marginStart = margin.start.toPxInt() - params.topMargin = margin.top.toPxInt() - params.marginEnd = margin.end.toPxInt() - params.bottomMargin = margin.bottom.toPxInt() - } - } + requestLayout() } override fun horizontalAlignment(horizontalAlignment: CrossAxisAlignment) { this.horizontalAlignment = horizontalAlignment - invalidate() + requestLayout() } override fun verticalAlignment(verticalAlignment: CrossAxisAlignment) { this.verticalAlignment = verticalAlignment - invalidate() + requestLayout() } - /** Flush Redwood's modifiers into FrameLayout's LayoutParams before it measures. */ override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { - val widthMode = if (width == Constraint.Fill) { - MeasureSpec.EXACTLY - } else { - MeasureSpec.getMode(widthMeasureSpec) - } val widthSize = MeasureSpec.getSize(widthMeasureSpec) + val widthMode = MeasureSpec.getMode(widthMeasureSpec) + val heightSize = MeasureSpec.getSize(heightMeasureSpec) + val heightMode = MeasureSpec.getMode(heightMeasureSpec) - val heightMode = if (height == Constraint.Fill) { - MeasureSpec.EXACTLY - } else { - MeasureSpec.getMode(heightMeasureSpec) + // Short circuit if this box's dimensions are already fully specified. + if (widthMode == MeasureSpec.EXACTLY && heightMode == MeasureSpec.EXACTLY) { + setMeasuredDimension(widthSize, heightSize) + return } - val heightSize = MeasureSpec.getSize(heightMeasureSpec) - for (child in children.widgets) { - child.value.updateLayoutParams { - setFrom(child.modifier) - } + measurer.box( + layoutDirection = layoutDirection, + boxDensity = density, + boxHorizontalAlignment = horizontalAlignment, + boxVerticalAlignment = verticalAlignment, + boxMargin = margin, + boxWidth = widthSize, + boxWidthUnspecified = widthMode == MeasureSpec.UNSPECIFIED, + boxHeight = heightSize, + boxHeightUnspecified = heightMode == MeasureSpec.UNSPECIFIED, + ) + + var maxWidth = 0 + var maxHeight = 0 + for (widget in children.widgets) { + measurer.measure(widget = widget) + maxWidth = maxOf(maxWidth, measurer.width + measurer.marginWidth) + maxHeight = maxOf(maxHeight, measurer.height + measurer.marginHeight) + } + + val resultWidth = when { + widthMode == MeasureSpec.EXACTLY -> widthSize + widthMode == MeasureSpec.AT_MOST -> maxWidth.coerceAtMost(widthSize) + else -> maxWidth + } + val resultHeight = when { + heightMode == MeasureSpec.EXACTLY -> heightSize + heightMode == MeasureSpec.AT_MOST -> maxHeight.coerceAtMost(heightSize) + else -> maxHeight } - super.onMeasure( - MeasureSpec.makeMeasureSpec(widthSize, widthMode), - MeasureSpec.makeMeasureSpec(heightSize, heightMode), + setMeasuredDimension(resultWidth, resultHeight) + } + + override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) { + measurer.box( + layoutDirection = layoutDirection, + boxDensity = density, + boxHorizontalAlignment = horizontalAlignment, + boxVerticalAlignment = verticalAlignment, + boxMargin = margin, + boxWidth = (right - left).coerceAtLeast(0), + boxHeight = (bottom - top).coerceAtLeast(0), ) + + for (widget in children.widgets) { + measurer.measure(widget) + measurer.layout(widget) + } } +} - private fun LayoutParams.setFrom(modifier: Modifier) { - var horizontalAlignment = horizontalAlignment - var verticalAlignment = verticalAlignment - var requestedWidth = Int.MIN_VALUE - var requestedHeight = Int.MIN_VALUE - - modifier.forEachScoped { childModifier -> - // Check for modifier overrides in the children, otherwise default to the Box's alignment - // values. - when (childModifier) { - is HorizontalAlignment -> { - horizontalAlignment = childModifier.alignment - } +/** + * Measures and lays out one child view at a time. + * + * This class is mutable and reused to avoid object allocation. + */ +private class Measurer { + // Inputs from the box. + var layoutDirection = LAYOUT_DIRECTION_LTR + var boxDensity = Density(1.0) + var boxHorizontalAlignment = CrossAxisAlignment.Start + var boxVerticalAlignment = CrossAxisAlignment.Start + var boxMarginStart = 0 + var boxMarginEnd = 0 + var boxMarginTop = 0 + var boxMarginBottom = 0 + var boxWidthUnspecified = false + var boxHeightUnspecified = false - is VerticalAlignment -> { - verticalAlignment = childModifier.alignment - } + // The available space for the child view and its margins. + var frameWidth = -1 + var frameHeight = -1 - is Width -> { - requestedWidth = with(density) { childModifier.width.toPxInt() } - } + // Inputs from the child widget. + var horizontalAlignment = CrossAxisAlignment.Start + var verticalAlignment = CrossAxisAlignment.Start + var marginStart = 0 + var marginEnd = 0 + var marginTop = 0 + var marginBottom = 0 + var requestedWidth = -1 + var requestedHeight = -1 - is Height -> { - requestedHeight = with(density) { childModifier.height.toPxInt() } - } + // Measurement results. + var width = -1 + var height = -1 - is Size -> { - requestedWidth = with(density) { childModifier.width.toPxInt() } - requestedHeight = with(density) { childModifier.height.toPxInt() } - } + val marginWidth: Int + get() = marginStart + marginEnd + val marginHeight: Int + get() = marginTop + marginBottom + + /** Configure the enclosing box. */ + fun box( + layoutDirection: Int, + boxDensity: Density, + boxHorizontalAlignment: CrossAxisAlignment, + boxVerticalAlignment: CrossAxisAlignment, + boxMargin: Margin, + boxWidth: Int, + boxWidthUnspecified: Boolean = false, + boxHeight: Int, + boxHeightUnspecified: Boolean = false, + ) { + this.layoutDirection = layoutDirection + this.boxDensity = boxDensity + this.boxHorizontalAlignment = boxHorizontalAlignment + this.boxVerticalAlignment = boxVerticalAlignment + with(boxDensity) { + boxMarginStart = boxMargin.start.toPxInt() + boxMarginEnd = boxMargin.end.toPxInt() + boxMarginTop = boxMargin.top.toPxInt() + boxMarginBottom = boxMargin.bottom.toPxInt() + } + this.frameWidth = (boxWidth - boxMarginStart - boxMarginEnd).coerceAtLeast(0) + this.boxWidthUnspecified = boxWidthUnspecified + this.frameHeight = (boxHeight - boxMarginTop - boxMarginBottom).coerceAtLeast(0) + this.boxHeightUnspecified = boxHeightUnspecified + } - is MarginModifier -> { - with(density) { - marginStart = childModifier.margin.start.toPxInt() - marginEnd = childModifier.margin.end.toPxInt() - topMargin = childModifier.margin.top.toPxInt() - bottomMargin = childModifier.margin.bottom.toPxInt() + /** Measure [widget]. Always call [box] first. */ + fun measure(widget: Widget) { + this.horizontalAlignment = boxHorizontalAlignment + this.verticalAlignment = boxVerticalAlignment + this.marginStart = 0 + this.marginEnd = 0 + this.marginTop = 0 + this.marginBottom = 0 + this.requestedWidth = -1 + this.requestedHeight = -1 + + with(boxDensity) { + widget.modifier.forEachScoped { childModifier -> + when (childModifier) { + is HorizontalAlignment -> horizontalAlignment = childModifier.alignment + is VerticalAlignment -> verticalAlignment = childModifier.alignment + is Width -> requestedWidth = childModifier.width.toPxInt() + is Height -> requestedHeight = childModifier.height.toPxInt() + is Size -> { + requestedWidth = childModifier.width.toPxInt() + requestedHeight = childModifier.height.toPxInt() + } + + is RedwoodMargin -> { + marginStart = maxOf(marginStart, childModifier.margin.start.toPxInt()) + marginEnd = maxOf(marginEnd, childModifier.margin.end.toPxInt()) + marginTop = maxOf(marginTop, childModifier.margin.top.toPxInt()) + marginBottom = maxOf(marginBottom, childModifier.margin.bottom.toPxInt()) } } } } - width = if (requestedWidth != Int.MIN_VALUE) { - requestedWidth - } else { - horizontalAlignment.toDimension() + val availableWidth = (frameWidth - marginWidth).coerceAtLeast(0) + val availableHeight = (frameHeight - marginHeight).coerceAtLeast(0) + + val widthMeasureSpec = when { + requestedWidth != -1 -> makeMeasureSpec(requestedWidth, MeasureSpec.EXACTLY) + horizontalAlignment == CrossAxisAlignment.Stretch -> + makeMeasureSpec(availableWidth, MeasureSpec.EXACTLY) + boxWidthUnspecified -> makeMeasureSpec(availableWidth, MeasureSpec.UNSPECIFIED) + else -> makeMeasureSpec(availableWidth, MeasureSpec.AT_MOST) } - height = if (requestedHeight != Int.MIN_VALUE) { - requestedHeight - } else { - verticalAlignment.toDimension() + + val heightMeasureSpec = when { + requestedHeight != -1 -> makeMeasureSpec(requestedHeight, MeasureSpec.EXACTLY) + verticalAlignment == CrossAxisAlignment.Stretch -> + makeMeasureSpec(availableHeight, MeasureSpec.EXACTLY) + boxHeightUnspecified -> makeMeasureSpec(availableHeight, MeasureSpec.UNSPECIFIED) + else -> makeMeasureSpec(availableHeight, MeasureSpec.AT_MOST) } - gravity = toGravity(horizontalAlignment, verticalAlignment) - } - private fun toGravity( - horizontalAlignment: CrossAxisAlignment, - verticalAlignment: CrossAxisAlignment, - ): Int { - val horizontalGravity = when (horizontalAlignment) { - CrossAxisAlignment.Start -> Gravity.START - CrossAxisAlignment.Center -> Gravity.CENTER_HORIZONTAL - CrossAxisAlignment.End -> Gravity.END - CrossAxisAlignment.Stretch -> Gravity.FILL_HORIZONTAL - else -> throw AssertionError() + widget.value.measure(widthMeasureSpec, heightMeasureSpec) + + width = when { + requestedWidth != -1 -> requestedWidth + horizontalAlignment == CrossAxisAlignment.Stretch -> availableWidth + else -> widget.value.measuredWidth } - val verticalGravity = when (verticalAlignment) { - CrossAxisAlignment.Start -> Gravity.TOP - CrossAxisAlignment.Center -> Gravity.CENTER_VERTICAL - CrossAxisAlignment.End -> Gravity.BOTTOM - CrossAxisAlignment.Stretch -> Gravity.FILL_VERTICAL - else -> throw AssertionError() + + height = when { + requestedHeight != -1 -> requestedHeight + verticalAlignment == CrossAxisAlignment.Stretch -> availableHeight + else -> widget.value.measuredHeight } - return horizontalGravity or verticalGravity } - private fun CrossAxisAlignment.toDimension(): Int { - return when (this) { - CrossAxisAlignment.Start -> WRAP_CONTENT - CrossAxisAlignment.Center -> WRAP_CONTENT - CrossAxisAlignment.End -> WRAP_CONTENT - CrossAxisAlignment.Stretch -> MATCH_PARENT - else -> throw AssertionError() + /** Layout [widget]. Always call [measure] first. */ + fun layout(widget: Widget) { + val marginLeft: Int + val marginRight: Int + val alignRight: Boolean + + if (layoutDirection == View.LAYOUT_DIRECTION_RTL) { + marginLeft = marginEnd + boxMarginEnd + marginRight = marginStart + boxMarginStart + alignRight = horizontalAlignment == CrossAxisAlignment.Start + } else { + marginLeft = marginStart + boxMarginStart + marginRight = marginEnd + boxMarginEnd + alignRight = horizontalAlignment == CrossAxisAlignment.End + } + + val left = when { + horizontalAlignment == CrossAxisAlignment.Center -> { + marginLeft + (frameWidth - width) / 2 + } + alignRight -> { + (boxMarginStart + frameWidth + boxMarginEnd) - marginRight - width + } + else -> marginLeft } + + val top = when (verticalAlignment) { + CrossAxisAlignment.Center -> { + boxMarginTop + marginTop + (frameHeight - height) / 2 + } + CrossAxisAlignment.End -> { + boxMarginTop + frameHeight - marginBottom - height + } + else -> boxMarginTop + marginTop + } + + widget.value.layout(left, top, left + width, top + height) } } diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png index ca2a08de20..0673865974 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:50b9c85846f850d18a937a524b9c5110bbc8f7788c813d701b523029510f4a16 -size 43147 +oid sha256:804ef6f58c103cb616ad2ab610c4287eba85e4225228330aa9cfac9b40eee6ba +size 42200 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png index 0c1a3e1c2b..007e79bd1a 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c23c04e5cf84ac043a05f101bffedd61dab17d87876a3ac38d78927bf29c36fe -size 4237 +oid sha256:40fed9e52c8241f10518017ad53f90c9f3e2c183bb4bfb925af373c6be511247 +size 4175 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png index 56eed696e2..58f739c1cf 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:e6d995a87ee3eb14794c8056b0750b5a6d340b5c28e8931d134655ca046d0c72 -size 9473 +oid sha256:3204ac7fd815cfa3f5f078c4ac79538a6646e16281a9cb06842f8349b88462d5 +size 9474 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png index ae24faa6b9..924fcdcc39 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:cd12803cda1cfaec124d2de3931c13a097aa6472d8c494ca7862ca1875f95803 -size 42875 +oid sha256:5ed76589be96daa6352e17401d40dc2fb826547c2bd7726e0a6731bcf531fcb6 +size 42322 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png index 147628e9a6..20aee469b4 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4de42288244c75fcf904f566bb10f86042c37e90b5302ce75c1150ecb4b18ebd -size 4253 +oid sha256:aecf04b9e93f7e5058908cd38122af8049cba17b4cade2945cd34886acfd84d6 +size 4194 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png index 75be3f73f5..6bd387d011 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a7bf7a96c20b583f844fe8c8354adce35494cf080e1998fcef1c112f9f12fa7b -size 9515 +oid sha256:bb2dc0c8bddaa5591399db9a3fcbbf3ae66567737b230347968dc9849066febb +size 9481