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

GlideImage with wrong width/height when model changes #5219

Closed
sky-ricardocarrapico opened this issue Jul 26, 2023 · 3 comments
Closed

GlideImage with wrong width/height when model changes #5219

sky-ricardocarrapico opened this issue Jul 26, 2023 · 3 comments

Comments

@sky-ricardocarrapico
Copy link

Glide Version: 4.15.1 / 1.0.0-alpha.3 (Compose)

Integration libraries: N/A

Device/Android Version: N/A (but it happens in both Samsung Galaxy S20 Ultra and emulator)

Issue details / Repro steps / Use case background:

If we're trying to load an image where one size is constrained and the other is not plus, if we first pass a null on the model (because we don't yet know what is the URL) and later replace with the actual URL, the view will end up taking all the available parent space instead of wrapping around the content on the dimension that is not constrained.

Given this code:

Row(
    verticalAlignment = Alignment.CenterVertically,
    modifier = Modifier
        .height(56.dp)
        .padding(horizontal = 16.dp)
) {
    Text(
        text = "pre",
        color = Color.White
    )

    GlideImage(
        model = url,
        contentDescription = null,
        contentScale = ContentScale.FillHeight,
        modifier = Modifier
            .height(32.dp),
    )

    Text(
        text = "after",
        modifier = Modifier.fillMaxWidth(),
        color = Color.White
    )
}

Suppose that url = null at the start and is later changed into an actual url. What ends up happening:

Screenshot 2023-07-26 at 11 21 30

If we don't call GlideImage with null at the start:

url?.let {
    GlideImage(...)
}

It'll be correct:

Screenshot 2023-07-26 at 11 24 01

This seems to be because the size is not invalidated when the model changes. I tried to hack around this with the following:

requestBuilderTransform = {
    it.run { if (url == null) override(Target.SIZE_ORIGINAL) else this }
}

And it fixes the issue.

Possibly related with #4916.

Thanks!

@sjudd sjudd added the Compose label Aug 3, 2023
@sjudd
Copy link
Collaborator

sjudd commented Aug 13, 2023

Let me know if you can reproduce with the latest snapshot, there've been a lot of changes since the first version

@sky-ricardocarrapico
Copy link
Author

Tested latest snapshot (1.0.0-alpha.4-20230817.020857-63) and it seems worse. Now its not even based on that url == null. Since I don't specify a width, only a height, it'll always take the full available width. So a proper "wrap_content" is not working.

I read #5230 (comment) and #5234 and it seems to be the issue.

Any way I can work around this?

Thanks!

@sjudd
Copy link
Collaborator

sjudd commented Aug 20, 2023

Ok I think this is fixed, we'll now generally default to sizing to intrinsics, at the cost of additional layout passes as the image is loaded.

@sjudd sjudd closed this as completed Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants