JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.87k stars 1.15k forks source link

LazyColumn displays repeated images out of their order #3189

Open mal7othify opened 1 year ago

mal7othify commented 1 year ago

Describe the bug When using LazyColumn to display images, they will be displayed but on scrolling they change to display repeated images and out of their original order in the list. Full code in this repository

Affected platforms Select one of the platforms below:

Versions

To Reproduce Steps and/or the code snippet to reproduce the behavior:

  1. Go to this repository
  2. Click run the code for iOS and desktop
  3. Scroll down quickly to the bottom, then, back to top
  4. You will see images out of their order and duplicated.
  5. Match this abnormal behavior with the normal behavior on the Android app.

Expected behavior I expect the images to be stable and displayed once based on their correct order defined in the list.

AlexeyTsvetkov commented 1 year ago

Reproduced.

A simplified example for desktop with just solid colors can be found here. When I scroll the example, violet.jpeg and black.jpeg show red and orange colors correspondingly.

Screenshot 2023-05-23 at 05 47 31

If I resize the window, the correct colors are shown.

Screenshot 2023-05-23 at 05 47 40

But now if I scroll up, red.jpeg and orange.jpeg are shown as violet and orange colors.

Screenshot 2023-05-23 at 05 51 30
m-sasha commented 1 year ago

An even simpler reproducer, without painterResource:

import androidx.compose.foundation.Image
import androidx.compose.foundation.layout.*
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.material.MaterialTheme
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.painter.ColorPainter
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.application

fun main() {
    application {
        Window(onCloseRequest = ::exitApplication) {
            MaterialTheme {
                AllImagesView()
            }
        }
    }
}

data class Item(val name: String, val color: Color)

val Items = listOf(
    Item("red", Color.Red),
    Item("orange", Color(0xffffaa00)),
    Item("yellow", Color.Yellow),
    Item("green", Color.Green),
    Item("blue", Color.Blue),
    Item("violet", Color(0xffff00ff)),
    Item("black", Color.Black),
)

@Composable
fun AllImagesView() {
    LazyColumn(
        Modifier.fillMaxWidth(),
        contentPadding = PaddingValues(8.dp)
    ) {
        items(count = Items.size) { index ->
            ImageView(Items[index])
        }
    }
}

@Composable
fun ImageView(item: Item) {
    Column(horizontalAlignment = Alignment.CenterHorizontally) {
        Text(
            text = item.name,
            modifier = Modifier.padding(top = 16.dp),
        )
        Image(
            ColorPainter(item.color),
            contentDescription = null,
            modifier = Modifier.height(200.dp).fillMaxWidth(),
            contentScale = ContentScale.Crop
        )
    }
}

The problem appears to be that when PainterModifierNodeElement.update is called, it updates the painter correctly, and then calls node.invalidateDraw() but inside, node.isAttached returns false.

m-sasha commented 1 year ago

Note that you can, for now, work around this issue by wrapping your ImageView call with key, e.g.:

        items(Items) { item ->
            key(item){
                ImageView(item)
            }
        }
m-sasha commented 1 year ago

This is a bug we inherited from upstream and it's already been fixed, but we haven't merged the fix yet. It should come in the next Compose release.

The problem is in LayoutNode.onReuse():

    override fun onReuse() {
        interopViewFactoryHolder?.onReuse()
        if (deactivated) {
            deactivated = false
            // we don't need to reset state as it was done when deactivated
        } else {
            resetModifierState()
        }
        // resetModifierState detaches all nodes, so we need to re-attach them upon reuse.
        nodes.attach()  // <-- This line was removed and after we merged from upstream, returned
    }
okushnikov commented 1 month ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.