Nesting providesDefault for a LocalProvider blocks changes from other LocalProviders from being propagated down the line [374263387]

Fixed

Bug

Status Update

No update yet.

Description

hr...@gmail.com

created issue #1

Oct 18, 2024 01:05PM

Jetpack Compose version: 1.7.1

Jetpack Compose component(s) used: ProvidableCompositionLocal and providesDefault

Android Studio Build: Jellyfish 2023.3.1 Patch 2

Kotlin version: 2.0.21

Steps to Reproduce or Code Sample to Reproduce:

Nesting multiple providesDefault for a ProvidableCompositionLocal blocks any changes from other wrapper ProvidableCompositionLocals from being propagated from the root block to the second nested block.

Example:

val LocalValue: ProvidableCompositionLocal<Int> =
    staticCompositionLocalOf { error("No value provided") }

val LocalOtherValue: ProvidableCompositionLocal<Boolean> =
    staticCompositionLocalOf { error("No other value provided") }


@Composable
fun Example() {
    var external by remember { mutableStateOf(false) }

    CompositionLocalProvider(LocalOtherValue provides external) {
        LocalOtherValue.current.also {
            LaunchedEffect(it) { println("Root block other value changed") }
        }
        CompositionLocalProvider(LocalValue providesDefault 1) {
            LocalOtherValue.current.also {
                LaunchedEffect(it) { println("First block other value changed") }
            }
            CompositionLocalProvider(LocalValue providesDefault 1) {
                LocalOtherValue.current.also {
                    LaunchedEffect(it) { println("Second block other value changed") }
                }
            }
        }
    }

    LaunchedEffect(Unit) {
        delay(1000)
        external = !external
    }
}

In this example "Second block other value changed" should get printed twice because LocalOtherValue is expected to change and trigger the LaunchedEffect inside the second block but it does not. It only changes inside the "Root block other value changed" and "First block other value changed" blocks.

As per my understanding providesDefault should only affect LocalValue.

Comments

jo...@google.com <jo...@google.com> Oct 21, 2024 05:25PM

Assigned to je...@google.com.

al...@google.com <al...@google.com> #2Oct 24, 2024 04:43PM

Reassigned to ch...@google.com.

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

ch...@google.com <ch...@google.com> #3Oct 24, 2024 05:35PM

Accepted by ch...@google.com.

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

sk...@gmail.com <sk...@gmail.com> #4Oct 25, 2024 08:16AM

Comment has been deleted.

Message last modified on Oct 25, 2024 08:16AM

hr...@gmail.com <hr...@gmail.com> #5Oct 25, 2024 08:17AM

This issue also affects Material theming changes because they use staticCompositionLocalOf under the hood. And those cannot be changed by a work-around.

ap...@google.com <ap...@google.com> #6Oct 29, 2024 04:44PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main
Author: Chuck Jazdzewski <chuckj@google.com>
Link: https://android-review.googlesource.com/3318540

Fixed providesDefault for a single provide

Expand for full commit details

Fixed `providesDefault` for a single provide 
 
If `providesDefault` is used as the sole provided value 
of a `CompostionLocalProvider` then the content of the 
provider may not update if a parent changes the value 
of a static composition local. Providing more than 
one value does not have this issue. 
 
Fixes: 374263387 
Test: new test, ./gradlew :compose:r:r:test 
Change-Id: Iabd4fbba6191abad4073ab5f302d252750683258

Files:

M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composer.kt
M compose/runtime/runtime/src/nonEmulatorCommonTest/kotlin/androidx/compose/runtime/CompositionLocalTests.kt

Hash: b70a4268ba4985d4a5baf9c14edd81290d2ded1d
Date: Thu Oct 24 17:16:39 2024

na...@google.com <na...@google.com> #7Nov 13, 2024 06:19PM

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

androidx.compose.runtime:runtime:1.8.0-alpha06
androidx.compose.runtime:runtime-android:1.8.0-alpha06
androidx.compose.runtime:runtime-jvmstubs:1.8.0-alpha06
androidx.compose.runtime:runtime-linuxx64stubs:1.8.0-alpha06