Fix rarely happening layout tree inconsistency
Sometimes the LayoutNodes added or invalidated during the subcomposition were not marked as needsRemeasure due to incorrect assumption that if the parent is measuring and (layout.measureIteration != measureIteration) then the child will be measured soon. It will not happen if the child has no needsRemeasure set to true. They were in fact remeasured not because of the needsRemeasure, but because of the changed constraints. New logic is simpler and handles this case, plus layout consistency checker is updated to be aware about the new logic.
Test: tests are passing, new test
Change-Id: I1701075900a09d939d62152f4179c69056d6c02e
diff --git a/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/WithConstraintsTest.kt b/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/WithConstraintsTest.kt
index b495d89..c048bf9 100644
--- a/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/WithConstraintsTest.kt
+++ b/ui/ui-framework/src/androidTest/java/androidx/ui/core/test/WithConstraintsTest.kt
@@ -643,6 +643,38 @@
assertEquals(LayoutDirection.Rtl, resultLayoutDirection)
}
+ @Test
+ fun withConstraintsChildIsMeasuredEvenWithDefaultConstraints() {
+ val compositionLatch = CountDownLatch(1)
+ val childMeasureLatch = CountDownLatch(1)
+ val zeroConstraints = Constraints.fixed(0.ipx, 0.ipx)
+ rule.runOnUiThreadIR {
+ activity.setContentInFrameLayout {
+ Layout(measureBlock = { measurables, _, _ ->
+ layout(0.ipx, 0.ipx) {
+ // there was a bug when the child of WithConstraints wasn't marking
+ // needsRemeasure and it was only measured because the constraints
+ // have been changed. to verify needRemeasure is true we measure the
+ // children with the default zero constraints so it will be equals to the
+ // initial constraints
+ measurables.first().measure(zeroConstraints).place(0.ipx, 0.ipx)
+ }
+ }, children = {
+ WithConstraints { _, _ ->
+ compositionLatch.countDown()
+ Layout(children = {}) { _, _, _ ->
+ childMeasureLatch.countDown()
+ layout(0.ipx, 0.ipx) {}
+ }
+ }
+ })
+ }
+ }
+
+ assertTrue(compositionLatch.await(1, TimeUnit.SECONDS))
+ assertTrue(childMeasureLatch.await(1, TimeUnit.SECONDS))
+ }
+
private fun takeScreenShot(size: Int): Bitmap {
assertTrue(drawLatch.await(1, TimeUnit.SECONDS))
val bitmap = rule.waitAndScreenShot()
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/AndroidOwner.kt b/ui/ui-platform/src/main/java/androidx/ui/core/AndroidOwner.kt
index a8a2ebd..86e09ee 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/AndroidOwner.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/AndroidOwner.kt
@@ -323,18 +323,14 @@
while (layout.affectsParentSize && layout.parentLayoutNode != null) {
val parent = layout.parentLayoutNode!!
if (parent.isMeasuring || parent.isLayingOut) {
- if (layout.measureIteration == measureIteration) {
- // the node we want to remeasure is the child of the parent which is
- // currently being measured and this parent did already measure us as a
- // child. so we have to postpone the measure request till the end of
- // the measuring pass to remeasure our parent again after it.
- // this can happen if the already measured child was requested to be
- // remeasured for example if the used @Model has been modified and the
- // frame has been committed during the measuring pass.
+ if (!layout.needsRemeasure) {
+ layout.needsRemeasure = true
+ // parent is currently measuring and we set needsRemeasure to true so if
+ // the parent didn't yet try to measure the node it will remeasure it.
+ // if the parent didn't plan to measure during this pass then needsRemeasure
+ // stay 'true' and we will manually call 'onRequestMeasure' for all
+ // the not-measured nodes in 'postponedMeasureRequests'.
postponedMeasureRequests.add(layout)
- } else {
- // otherwise we finished. this child wasn't measured yet, will be
- // measured soon.
}
consistencyChecker?.assertConsistent()
return
@@ -489,7 +485,9 @@
// execute postponed `onRequestMeasure`
if (postponedMeasureRequests.isNotEmpty()) {
postponedMeasureRequests.forEach {
- if (it.isAttached()) {
+ // if it was detached or already measured by the parent then skip it
+ if (it.isAttached() && it.needsRemeasure) {
+ it.needsRemeasure = false
onRequestMeasure(it)
}
}
diff --git a/ui/ui-platform/src/main/java/androidx/ui/core/LayoutTreeConsistencyChecker.kt b/ui/ui-platform/src/main/java/androidx/ui/core/LayoutTreeConsistencyChecker.kt
index 2b62ef5..c82d917 100644
--- a/ui/ui-platform/src/main/java/androidx/ui/core/LayoutTreeConsistencyChecker.kt
+++ b/ui/ui-platform/src/main/java/androidx/ui/core/LayoutTreeConsistencyChecker.kt
@@ -70,20 +70,22 @@
return relayoutNodes.contains(this)
}
if (needsRemeasure) {
+ if (postponedMeasureRequests.contains(this)) {
+ // this node is waiting to be measured by parent or if this will not happen
+ // `onRequestMeasure` will be called for all items in `postponedMeasureRequests`
+ return true
+ }
if (parent.isMeasuring || parent.isLayingOut) {
- return !duringMeasureLayout() ||
- parent.measureIteration != measureIteration
+ return parent.measureIteration != measureIteration
} else {
- val parentRemeasureScheduled = parent.needsRemeasure ||
- postponedMeasureRequests.contains(parent)
if (affectsParentSize) {
// node and parent both not yet laid out -> parent remeasure
// should be scheduled
- return parentRemeasureScheduled
+ return parent.needsRemeasure
} else {
// node is not affecting parent size and parent relayout(or
// remeasure, as it includes relayout) is scheduled
- return parent.needsRelayout || parentRemeasureScheduled
+ return parent.needsRelayout || parent.needsRemeasure
}
}
}