Support alignment lines reads from LayoutModifier
The CL adds support for reading alignment lines in the measure and
positioning blocks of LayoutModifier. Although the API of LayoutModifier
was already supporting alignment lines reads, trying to do so was
resulting in a crash or buggy values due to missing wire up. Note that
providing an alignment line in LayoutModifier is still unsupported and
will be fixed in a further CL.
Relnote: Fixed alignment lines reads from LayoutModifier.
Bug: 155736449
Test: AndroidLayoutDrawTest
Change-Id: I9dd1dbeded5f7d6e3d0b7f50a09fb1b0618ca769
diff --git a/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/AndroidLayoutDrawTest.kt b/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/AndroidLayoutDrawTest.kt
index bade43c..29e09ad 100644
--- a/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/AndroidLayoutDrawTest.kt
+++ b/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/AndroidLayoutDrawTest.kt
@@ -73,6 +73,7 @@
import androidx.ui.graphics.Path
import androidx.ui.graphics.Shape
import androidx.ui.layout.ltr
+import androidx.ui.layout.offset
import androidx.ui.layout.padding
import androidx.ui.layout.rtl
import androidx.ui.unit.Density
@@ -115,12 +116,14 @@
val excessiveAssertions = AndroidOwnerExtraAssertionsRule()
private lateinit var activity: TestActivity
private lateinit var drawLatch: CountDownLatch
+ private lateinit var density: Density
@Before
fun setup() {
activity = activityTestRule.activity
activity.hasFocusLatch.await(5, TimeUnit.SECONDS)
drawLatch = CountDownLatch(1)
+ density = Density(activity)
}
// Tests that simple drawing works with layered squares
@@ -1426,6 +1429,157 @@
}
@Test
+ fun testAlignmentLines_readFromModifier_duringMeasurement() = with(density) {
+ val testVerticalLine = VerticalAlignmentLine(::min)
+ val testHorizontalLine = HorizontalAlignmentLine(::max)
+
+ val assertLines: Modifier.(IntPx, IntPx) -> Modifier = { vertical, horizontal ->
+ this + object : LayoutModifier {
+ override fun MeasureScope.measure(
+ measurable: Measurable,
+ constraints: Constraints,
+ layoutDirection: LayoutDirection
+ ): MeasureScope.MeasureResult {
+ val placeable = measurable.measure(constraints)
+ assertEquals(vertical, placeable[testVerticalLine])
+ assertEquals(horizontal, placeable[testHorizontalLine])
+ return layout(placeable.width, placeable.height) {
+ placeable.place(0.ipx, 0.ipx)
+ }
+ }
+ }
+ }
+
+ testAlignmentLinesReads(testVerticalLine, testHorizontalLine, assertLines)
+ }
+
+ @Test
+ fun testAlignmentLines_readFromModifier_duringPositioning_before() = with(density) {
+ val testVerticalLine = VerticalAlignmentLine(::min)
+ val testHorizontalLine = HorizontalAlignmentLine(::max)
+
+ val assertLines: Modifier.(IntPx, IntPx) -> Modifier = { vertical, horizontal ->
+ this + object : LayoutModifier {
+ override fun MeasureScope.measure(
+ measurable: Measurable,
+ constraints: Constraints,
+ layoutDirection: LayoutDirection
+ ): MeasureScope.MeasureResult {
+ val placeable = measurable.measure(constraints)
+ return layout(placeable.width, placeable.height) {
+ assertEquals(vertical, placeable[testVerticalLine])
+ assertEquals(horizontal, placeable[testHorizontalLine])
+ placeable.place(0.ipx, 0.ipx)
+ }
+ }
+ }
+ }
+
+ testAlignmentLinesReads(testVerticalLine, testHorizontalLine, assertLines)
+ }
+
+ @Test
+ fun testAlignmentLines_readFromModifier_duringPositioning_after() = with(density) {
+ val testVerticalLine = VerticalAlignmentLine(::min)
+ val testHorizontalLine = HorizontalAlignmentLine(::max)
+
+ val assertLines: Modifier.(IntPx, IntPx) -> Modifier = { vertical, horizontal ->
+ this + object : LayoutModifier {
+ override fun MeasureScope.measure(
+ measurable: Measurable,
+ constraints: Constraints,
+ layoutDirection: LayoutDirection
+ ): MeasureScope.MeasureResult {
+ val placeable = measurable.measure(constraints)
+ return layout(placeable.width, placeable.height) {
+ placeable.place(0.ipx, 0.ipx)
+ assertEquals(vertical, placeable[testVerticalLine])
+ assertEquals(horizontal, placeable[testHorizontalLine])
+ }
+ }
+ }
+ }
+
+ testAlignmentLinesReads(testVerticalLine, testHorizontalLine, assertLines)
+ }
+
+ private fun Density.testAlignmentLinesReads(
+ testVerticalLine: VerticalAlignmentLine,
+ testHorizontalLine: HorizontalAlignmentLine,
+ assertLines: Modifier.(IntPx, IntPx) -> Modifier
+ ) {
+ val layoutLatch = CountDownLatch(7)
+ activityTestRule.runOnUiThreadIR {
+ activity.setContent {
+ val layout = @Composable { modifier: Modifier ->
+ Layout(modifier = modifier, children = {}) { _, _, _ ->
+ layout(
+ 0.ipx,
+ 0.ipx,
+ mapOf(
+ testVerticalLine to 10.ipx,
+ testHorizontalLine to 20.ipx
+ )
+ ) {
+ layoutLatch.countDown()
+ }
+ }
+ }
+
+ layout(Modifier.assertLines(10.ipx, 20.ipx))
+ layout(Modifier.assertLines(30.ipx, 30.ipx).offset(20.ipx.toDp(), 10.ipx.toDp()))
+ layout(Modifier
+ .assertLines(30.ipx, 30.ipx)
+ .drawLayer()
+ .offset(20.ipx.toDp(), 10.ipx.toDp())
+ )
+ layout(Modifier
+ .assertLines(30.ipx, 30.ipx)
+ .background(Color.Blue)
+ .drawLayer()
+ .offset(20.ipx.toDp(), 10.ipx.toDp())
+ .drawLayer()
+ .background(Color.Blue)
+ )
+ layout(Modifier
+ .background(Color.Blue)
+ .assertLines(30.ipx, 30.ipx)
+ .background(Color.Blue)
+ .drawLayer()
+ .offset(20.ipx.toDp(), 10.ipx.toDp())
+ .drawLayer()
+ .background(Color.Blue)
+ )
+ Wrap(
+ Modifier
+ .background(Color.Blue)
+ .assertLines(30.ipx, 30.ipx)
+ .background(Color.Blue)
+ .drawLayer()
+ .offset(20.ipx.toDp(), 10.ipx.toDp())
+ .drawLayer()
+ .background(Color.Blue)
+ ) {
+ layout(Modifier)
+ }
+ Wrap(
+ Modifier
+ .background(Color.Blue)
+ .assertLines(40.ipx, 50.ipx)
+ .background(Color.Blue)
+ .drawLayer()
+ .offset(20.ipx.toDp(), 10.ipx.toDp())
+ .drawLayer()
+ .background(Color.Blue)
+ ) {
+ layout(Modifier.offset(10.ipx.toDp(), 20.ipx.toDp()))
+ }
+ }
+ }
+ assertTrue(layoutLatch.await(1, TimeUnit.SECONDS))
+ }
+
+ @Test
fun testLayoutBeforeDraw_forRecomposingNodesNotAffectingRootSize() {
val model = OffsetModel(0.ipx)
var latch = CountDownLatch(1)
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/ComponentNodes.kt b/ui/ui-platform/src/main/java/androidx/ui/core/ComponentNodes.kt
index 7901e87..158f238 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/ComponentNodes.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/ComponentNodes.kt
@@ -558,6 +558,11 @@
private set
/**
+ * `true` while doing [calculateAlignmentLines]
+ */
+ private var isCalculatingAlignmentLines = false
+
+ /**
* `true` when the current node is positioned during the measure pass,
* since it needs to compute alignment lines.
*/
@@ -579,7 +584,7 @@
*/
var needsRelayout = false
internal set(value) {
- require(!isMeasuring)
+ require(!isMeasuring || isCalculatingAlignmentLines)
require(!isLayingOut)
field = value
}
@@ -984,6 +989,7 @@
}
internal fun calculateAlignmentLines(): Map<AlignmentLine, IntPx> {
+ isCalculatingAlignmentLines = true
alignmentLinesRead = true
alignmentLinesQueryOwner = this
alignmentLinesQueriedSinceLastLayout = true
@@ -991,6 +997,7 @@
needsRelayout = true
layout()
}
+ isCalculatingAlignmentLines = false
return alignmentLines
}
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/DelegatingLayoutNodeWrapper.kt b/ui/ui-platform/src/main/java/androidx/ui/core/DelegatingLayoutNodeWrapper.kt
index 80d4b9b..67f0806 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/DelegatingLayoutNodeWrapper.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/DelegatingLayoutNodeWrapper.kt
@@ -23,8 +23,6 @@
import androidx.ui.unit.IntPxPosition
import androidx.ui.unit.PxPosition
import androidx.ui.unit.ipx
-import androidx.ui.unit.round
-import androidx.ui.unit.toPx
/**
* [LayoutNodeWrapper] with default implementations for methods.
@@ -64,15 +62,18 @@
}
}
- override fun get(line: AlignmentLine): IntPx? {
- val value = wrapped[line] ?: return null
- val px = value.toPx()
- val pos = wrapped.toParentPosition(PxPosition(px, px))
- return if (line is HorizontalAlignmentLine) pos.y.round() else pos.y.round()
- }
+ override fun get(line: AlignmentLine): IntPx? = wrapped[line]
override fun place(position: IntPxPosition) {
this.position = position
+
+ // The wrapper only runs their placement block to obtain our position, which allows them
+ // to calculate the offset of an alignment line we have already provided a position for.
+ // No need to place our wrapped as well (we might have actually done this already in
+ // get(line), to obtain the position of the alignment line the wrapper currently needs
+ // our position in order ot know how to offset the value we provided).
+ if (wrappedBy?.isShallowPlacing == true) return
+
with(InnerPlacementScope) {
this.parentLayoutDirection = measureScope.layoutDirection
val previousParentWidth = parentWidth
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/InnerPlaceable.kt b/ui/ui-platform/src/main/java/androidx/ui/core/InnerPlaceable.kt
index db54e55..5da930c 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/InnerPlaceable.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/InnerPlaceable.kt
@@ -110,8 +110,16 @@
}
override fun place(position: IntPxPosition) {
- layoutNode.isPlaced = true
this.position = position
+
+ // The wrapper only runs their placement block to obtain our position, which allows them
+ // to calculate the offset of an alignment line we have already provided a position for.
+ // No need to place our wrapped as well (we might have actually done this already in
+ // get(line), to obtain the position of the alignment line the wrapper currently needs
+ // our position in order ot know how to offset the value we provided).
+ if (wrappedBy?.isShallowPlacing == true) return
+
+ layoutNode.isPlaced = true
layoutNode.layout()
}
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/LayoutNodeWrapper.kt b/ui/ui-platform/src/main/java/androidx/ui/core/LayoutNodeWrapper.kt
index a3eb00d..bc2fe55 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/LayoutNodeWrapper.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/LayoutNodeWrapper.kt
@@ -82,6 +82,10 @@
return layoutNode.layoutNodeWrapper.wrappedBy
}
+ // True when the wrapper is running its own placing block to obtain the position of the
+ // wrapped, but is not interested in the position of the wrapped of the wrapped.
+ var isShallowPlacing = false
+
// TODO(mount): This is not thread safe.
private var rectCache: RectF? = null
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/ModifiedLayoutNode.kt b/ui/ui-platform/src/main/java/androidx/ui/core/ModifiedLayoutNode.kt
index 690e5e51..8464d33 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/ModifiedLayoutNode.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/ModifiedLayoutNode.kt
@@ -62,8 +62,21 @@
measureScope.maxIntrinsicHeight(wrapped, width, layoutDirection)
}
- override operator fun get(line: AlignmentLine): IntPx? =
- measureResult.alignmentLines.getOrElse(line, { wrapped[line] })
+ override operator fun get(line: AlignmentLine): IntPx? {
+ if (measureResult.alignmentLines.containsKey(line)) {
+ return measureResult.alignmentLines[line]
+ }
+ val positionInWrapped = wrapped[line] ?: return null
+ // Place our wrapped to obtain their position inside ourselves.
+ isShallowPlacing = true
+ place(this.position)
+ isShallowPlacing = false
+ return if (line is HorizontalAlignmentLine) {
+ positionInWrapped + wrapped.position.y
+ } else {
+ positionInWrapped + wrapped.position.x
+ }
+ }
override fun draw(canvas: Canvas) {
withPositionTranslation(canvas) {