Surface doesn't render state change when wrapped with SaveableStateProvider [406795500]

Fixed

Bug

Status Update

No update yet.

Description

ar...@gmail.com

created issue #1

Mar 27, 2025 09:54PM

Jetpack Compose version: "1.8.0-rc02

Material Library Version (M2, M3 or Both?): M3 1.3.1"

Material Compose component used: Surface

Android Studio Build: #IU-243.22562.218

Kotlin version: 2.0.21

Steps to Reproduce or Code Sample to Reproduce:

@Composable
fun RootContent() {
    var screen by remember { mutableStateOf<Screen>(Screen.First) }

    Foo(screen = screen) {
        Surface(modifier = Modifier.fillMaxSize()) {
            when (it) {
                Screen.First -> Screen(text = "Go forward", onClick = { screen = Screen.Second })
                Screen.Second -> Screen(text = "Go back", onClick = { screen = Screen.First })
            }
        }
    }
}

sealed interface Screen {
    val key: String

    data object First : Screen {
        override val key: String = "first"
    }

    data object Second : Screen {
        override val key: String = "second"
    }
}

@Composable
fun Foo(screen: Screen, content: @Composable (Screen) -> Unit) {
    val holder = rememberSaveableStateHolder()
    holder.SaveableStateProvider(key = screen.key) {
        content(screen)
    }
}

@Composable
fun Screen(text: String, onClick: () -> Unit) {
    Box(
        modifier = Modifier.fillMaxSize(),
        contentAlignment = Alignment.Center
    ) {
        Button(onClick = onClick) {
            Text(text = text)
        }
    }
}

If you run the code and press "Go forward", the screen doesn't change and the button's ripple effect sticks (freezes).

If you remove the Surface then it works fine:

@Composable
fun RootContent() {
    var screen by remember { mutableStateOf<Screen>(Screen.First) }

    Foo(screen = screen) {
//        Surface(modifier = Modifier.fillMaxSize()) {
            when (it) {
                Screen.First -> Screen(text = "Go forward", onClick = { screen = Screen.Second })
                Screen.Second -> Screen(text = "Go back", onClick = { screen = Screen.First })
            }
//        }
    }
}

Comments

cl...@google.com <cl...@google.com> #2Apr 9, 2025 01:25PM

Can you confirm if this is a regression compared to a previous version you were using?

co...@google.com <co...@google.com> #3Apr 9, 2025 06:19PM

Please provide info on the device model and API level used to reproduce the bug.

Running the sample above (with surface) at tip of tree on a pixel 7 device, I am able to observe the text change and button's ripple effect.

ar...@gmail.com <ar...@gmail.com> #4Apr 10, 2025 11:49AM

I have attached a reproducer project. I think the key point is to also wrap RootContent with Surface {}.

class MainActivity : ComponentActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        enableEdgeToEdge()

        setContent {
            Surface {
                RootContent()
            }
        }
    }
}

ComposeSurfaceBug.zip

323 KB

Download

ar...@gmail.com <ar...@gmail.com> #5Apr 10, 2025 11:51AM

Reproducible on an API 34 emulator.

Properties
avd.ini.displayname              Pixel 7 API 34
avd.ini.encoding                 UTF-8
AvdId                            Pixel_7_API_34
disk.dataPartition.size          6442450944
fastboot.chosenSnapshotFile      
fastboot.forceChosenSnapshotBoot no
fastboot.forceColdBoot           yes
fastboot.forceFastBoot           no
hw.accelerometer                 yes
hw.arc                           false
hw.audioInput                    yes
hw.battery                       yes
hw.camera.back                   virtualscene
hw.camera.front                  emulated
hw.cpu.ncore                     4
hw.device.hash2                  MD5:3db3250dab5d0d93b29353040181c7e9
hw.device.manufacturer           Google
hw.device.name                   pixel_7
hw.dPad                          no
hw.gps                           yes
hw.gpu.enabled                   yes
hw.gpu.mode                      auto
hw.initialOrientation            Portrait
hw.keyboard                      yes
hw.lcd.density                   420
hw.lcd.height                    2400
hw.lcd.width                     1080
hw.mainKeys                      no
hw.ramSize                       2048
hw.sdCard                        yes
hw.sensors.orientation           yes
hw.sensors.proximity             yes
hw.trackBall                     no
image.androidVersion.api         34
image.sysdir.1                   system-images/android-34/google_apis_playstore/arm64-v8a/
PlayStore.enabled                true
runtime.network.latency          none
runtime.network.speed            full
showDeviceFrame                  yes
skin.dynamic                     yes
tag.display                      Google Play
tag.id                           google_apis_playstore
tag.ids                          google_apis_playstore
vm.heapSize                      228

ar...@gmail.com <ar...@gmail.com> #6Apr 10, 2025 12:45PM

It's also reproducible with Compose 1.8.0-alpha05 and M3 1.2.0. Not reproducible with Compose 1.8.0-alpha04 and M3 1.2.0. Looks like the regression was introduced in Compose 1.8.0-alpha05.

cl...@google.com <cl...@google.com> Apr 10, 2025 01:46PM

Reassigned to lp...@google.com.

lp...@google.com <lp...@google.com> #7Apr 10, 2025 10:13PM

Reassigned to mo...@google.com.

Thanks for the detailed repro and information, this was really helpful.

I traced this down to http://r.android.com/3297406

Here's a CL with this repro and the revert http://r.android.com/3583351 - if you revert the changes the files except for PainterResourcesDemo, and run the demo, you'll see the ripple gets stuck and the button doesn't change

George, it looks like this CL initially was a fix to a crash - we decided not to cherry pick the fix for this this crash in 1.7. Given that the crash still exists in 1.7, maybe we should just revert this fix out of 1.8 to not block 1.8 release (assuming it is safe to do so), and try and fix this in 1.9?

lp...@google.com <lp...@google.com> #8Apr 11, 2025 12:46PM

The issue seems specifically related to ReusableContent and layers (SaveableStateProvider uses ReusableContent internally), I guess we made some assumptions that don't work in the reuse case

mo...@google.com <mo...@google.com> #9Apr 11, 2025 08:47PM

The parent layer isn't being invalidated when a layer is being removed. Not sure how that was missed in some cases, but not others. I'll investigate further to see where it is currently being invalidated and see where the right place is to invalidate the parent layer.

mo...@google.com <mo...@google.com> #10Apr 14, 2025 04:23PM

Marked as fixed.

Fixed in aosp/3583994