Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compose GlideImage does not repaint Image when model changed #4916

Closed
vborodinramit opened this issue Oct 2, 2022 · 2 comments · Fixed by #4917
Closed

Compose GlideImage does not repaint Image when model changed #4916

vborodinramit opened this issue Oct 2, 2022 · 2 comments · Fixed by #4917
Labels

Comments

@vborodinramit
Copy link

vborodinramit commented Oct 2, 2022

Glide Version: Glide 4.14.1, glide:compose:1.0.0-alpha.0

Issue details / Repro steps / Use case background:

for image selection i use:
implementation 'com.github.akshaaatt:Cropper:1.00'

@Composable
fun SelectAvatar() {
    var avatarUri by rememberSaveable { mutableStateOf<Uri?>(null) }
    val selectAvatarLauncher = rememberLauncherForActivityResult(CropImageContract()) { result ->
        if (result.isSuccessful) {
            avatarUri = result.uriContent
        }
    }

    fun selectAvatar() {
        selectAvatarLauncher.launch(options {})
    }

    Column{
        TextButton(onClick = ::selectAvatar) {Text(text="Select Avatar")}
        if (avatarUri != null)
            GlideImage(model = avatarUri, modifier = Modifier.size(100.dp), contentDescription = "")
    }
}

When i click button first time and select image - it appears correctly, but when i click button second time and select new image GlideImage does not redraw itself and i see old picture.
I checked with Logs - avatarUri updated correctly and Compose recomposed.

@tomasziwaszko
Copy link

It has to be something wrong with using "remember" inside a GlideImage

@sjudd sjudd added the bug label Oct 4, 2022
@sjudd
Copy link
Collaborator

sjudd commented Oct 5, 2022

Yeah in particular, the model is supposed to be part of RequestBuilder, but it's not included in RequestBuilder's equals/hashcode method.

In fact a few things in RequestBuilder are not included and it's kind of unsafe by default because RequestBuilder inherits from a class that does implement equals/hashcode.

sjudd added a commit that referenced this issue Oct 7, 2022
…ncrete classes.

It's a little tricky to get this right because we've defined
equals/hashcode in the base classes, but are implementing them in the
concrete classes. I think this implementation is actually safe because
the base classes are abstract. However making equals transitive requires
the weird equals() only implementations in the classes that have no
properties but are concrete implementations of the base classes.

A safer way to do this would be to move the the equals/hashcode
implementation out of the base class and re-implement it in each
subclass. However that would require exposing a bunch of internal
instance variables to subclasses. Given the existing structure and the
public nature of the base classes, this seems like a reasonable
compromise.

Fixes #4916
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants