Calling Activity.setContentView again breaks recomposition of composables in the view hierarchy [197773820]

Fixed

Bug

Status Update

No update yet.

Description

lo...@gmail.com

created issue #1

Aug 26, 2021 10:51AM

Hello,

I found a bug that cost me one day to understand what triggers it, and I want to thank Adam Powell for helping me a bit to figure out what's going on.

TL;DR

In short, if you call setContentView with the same view hierarchy a second time after the initial render, the composables in the hierarchy will stop recomposing, which breaks/freezes everything in them.

Jetpack Compose release version: 1.1.0-alpha02 (also reproduces with 1.0 and 1.0.1)

Android Studio Build: irrelevant

Kotlin version: irrelevant

Reproducer

I attached a self-contained Kotlin file (MainActivity.kt) that reproduces the issue.

You can paste it in any Compose project over the existing MainActivity and try yourself.

However, I put below reproducing steps anyway.

Steps to Reproduce:

Create a ComposeView that displays a State that changes over time.
Put that instance into a local property.
Call setContentView passing that instance in the onCreate function of the host ComponentActivity.
Launch a coroutine in lifecycleScope
Call awaitFrame(), or delay(1500)
Call setContentView again, with the very same instance of the ComposeView created earlier.

Expected outcome

The composable keeps display the updates of the State instance.

Actual result

Once the setContentView is called the second time, the composable stops displaying the updates of the State instance.

The Slack thread from start to finish

If you want to see what was the process from facing the issue, to finding why it happened, you can take a look at this thread on Kotlin's Slack:

https://kotlinlang.slack.com/archives/CJLTWPH7S/p1629894233127300

What is likely the culprit IMHO

In the Android platform, the setContentView method in Activity is idempotent. Calling it a second time with the same view hierarchy while the Activity is not in the destroyed state will have no effect.

However, that contract seems to have been broken by the ComponentActivity class from AndroidX, as it's also calling a function named initViewTreeOwners().

The name of that function indicates that it's been designed for initialization, something which by definition, is supposed to happen only once. However, a setter doesn't generally have such constraints, at least, not because it's a setter (as the setContentView function is).

I think that this initViewTreeOwners() function being called again is what messes up ComposeView or its underlying mechanisms, putting them in a state they should not be in.

However, the issue might not be in ComponentActivity itself, but in the Compose runtime, or both.

Thanks for reading, and have a great day!

Louis CAD

MainActivity.kt

2.2 KB

Download

Comments

je...@google.com <je...@google.com> Aug 26, 2021 12:00PM

Assigned to ad...@google.com.

lo...@gmail.com <lo...@gmail.com> #2Aug 26, 2021 12:55PM

Project: platform/frameworks/support
Branch: androidx-main

commit a330c0d3bcdd41326f37968a60e6084ad4a2e32c
Author: Chet Haase <chet@google.com>
Date: Wed Jul 05 07:26:46 2023

Convert APIs using PointF to use Float instead

PointF is a convenient mechanism for passing around x.y values
representing 2D points. But there are downsides, including:
- Converting to PointF: You may not have the data in PointF form
to begin with, so using an API which takes PointF requires converting
the data to that form (including allocating a PointF object every time)
- Mutability: Point structures can be mutated internally, causing
unpredictability in what that mutation means. Should the library
react to those changes? Ignore them? Do defensive copies (requiring
even more allocations)? Using primitive types like Float make the
behavior more obvious (by making the data inherently immutable).
- Allocations: Whenever we use object types, there are necessarily
allocations on the Java heap for them. This puts pressure on the GC
at both allocation and collection time. Given the amount of points
being passed around (especially at morph creation time, when curves
are being split and created), this causes a lot of PointF objects to
be allocated (even temporarily). Using Float avoids that problem.

Also fixed bug with unclosed paths causing discontinuity at the
start/end point.

Bug: 276466399
Bug: 290254314
Test: integration and unit tests pass
Relnote: PointF parameters changed to Float pairs
Change-Id: Id4705d27c7be31b26ade8186b99fffe2e2f8450e

M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicShapeTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonMeasureTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/RoundedPolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/ShapesTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/TestUtils.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/CubicShape.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FeatureMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FloatMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Morph.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/PolygonMeasure.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Shapes.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Utils.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/DebugDraw.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/ShapeEditor.kt
M graphics/integration-tests/testapp/src/main/java/androidx/graphics/shapes/test/MaterialShapes.kt

https://android-review.googlesource.com/2649119

ad...@google.com <ad...@google.com> #3Aug 27, 2021 09:11PM

Accepted by ad...@google.com.

PointF was the main (possibly only) mutability issue, marking this as fixed

lo...@gmail.com <lo...@gmail.com> #4Aug 27, 2021 10:17PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.graphics:graphics-shapes:1.0.0-alpha04

ap...@google.com <ap...@google.com> #5Oct 1, 2021 08:19PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 83da539603920a4d33f851ac5de473f413684579
Author: Adam Powell <adamp@google.com>
Date: Wed Sep 08 15:27:34 2021

Fix ComposeView CompositionContext caching

Fix a bug where the CompositionContext cached by ComposeView to handle
being placed into a ViewOverlay could be reused even if that
CompositionContext is a Recomposer that shut down.

Test: RecomposerTests, WindowRecomposerTest
Fixes: 197773820
Relnote: "Recomposer.state has been deprecated and replaced by
Recomposer.currentState to change its type to a StateFlow"

Change-Id: Ic2ab34c19176704fe2f6cd081607dfb92d86ea3c

M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/platform/ComposeViewOverlayTest.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/ComposeView.android.kt
M compose/runtime/runtime/src/test/kotlin/androidx/compose/runtime/RecomposerTests.kt
M compose/runtime/runtime/api/restricted_current.txt
M compose/runtime/runtime/api/current.txt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/platform/WindowRecomposerTest.kt
M compose/runtime/runtime/api/public_plus_experimental_current.txt

https://android-review.googlesource.com/1822599

lo...@gmail.com <lo...@gmail.com> #6Feb 9, 2022 09:13PM

Thank you for fixing this!

I've been able to remove the workaround code I had, and it works like a charm.

All the best, Louis CAD