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

[Coil] Image component does not apply request builder transformations if the content scale is not ContentScale.Fit #394

Closed
pedrovgs opened this issue May 7, 2021 · 8 comments

Comments

@pedrovgs
Copy link

pedrovgs commented May 7, 2021

Describe the bug

After updating from accompanist-coil 0.7.1 to 0.8.0 and later to 0.9.0 and migrate from Coil I've noticed the new Image component does not apply transformations if the contentScale parameter is not ContentScale.Fit.

To Reproduce

Steps to reproduce the behavior:

  1. Load any image with rounded corners transformations or use any other transformation provided by Coil.
  2. Configure the CoilImage to show any image and use, for example ContentScale.Crop configuration.
  3. Migrate from CoilImage to the new Image component.

Expected behavior

The image shown to the user should be the same and the configured transformations should be applied. However, you'll notice the new component doesn't apply the transformation unless the contentScaleparam is ContentScale.Fit.

Environment:

  • Android OS version: All.
  • Device: Any
  • Accompanist version: Reproducible on any accompanist version >= 0.8.0
@chrisbanes
Copy link
Contributor

What transformations are you using? Or are you referring to using contentScale?

Can you paste in some example code? Are you setting an explicit size?

@pedrovgs
Copy link
Author

pedrovgs commented May 7, 2021

Hey @chrisbanes I've created a repository just in case you need to check it out. The code is similar to this snippet:

Column {
                    val imageUrl =
                        "https://play-lh.googleusercontent.com/S9lyB4MKAddMAHgSk7Kwjy1FG6yk6x7Nct5jciQxLy6LpRGDijCP7S34Z8W4wsF8RA"
                    Box(
                        Modifier
                            .height(40.dp)
                            .fillMaxWidth()) {
                        CoilImage(
                            modifier = Modifier
                                .background(Color.Green),
                            data = imageUrl,
                            contentDescription = "",
                            requestBuilder = {
                                transformations(
                                    RoundedCornersTransformation(1000f)
                                )
                            },
                            contentScale = ContentScale.Crop
                        )
                    }
                    Box(
                        Modifier
                            .height(40.dp)
                            .fillMaxWidth()) {
                        Image(
                            modifier = Modifier
                                .background(Color.Red),
                            painter = rememberCoilPainter(
                                request = imageUrl,
                                requestBuilder = {
                                    transformations(
                                        RoundedCornersTransformation(1000f)
                                    )
                                },
                            ),
                            contentDescription = "",
                            contentScale = ContentScale.Crop
                        )
                    }

                }

Check out the difference between the deprecated component and the new one:

Screenshot_1620396006

Maybe the issue is related to the component height, looks like the second image tries to apply the transformation but something wrong happens. What I don't get is why the first one works with the deprecated component and the second one doesn't. 🤔

@chrisbanes
Copy link
Contributor

I think you're missing a fillMaxSize() on the Image. Without that, the image is set to wrap content. #391 is probably related too.

@pedrovgs
Copy link
Author

pedrovgs commented May 7, 2021

@chrisbanes I tested it on the repository I shared with you and configuring fillMaxSize modifier plus the crop content size doesn't fix the issue. It changes the width but not the height of the image:

Screenshot 2021-05-07 at 16 28 16

@pedrovgs
Copy link
Author

pedrovgs commented May 7, 2021

@chrisbanes where can I find the latest snapshot published? Maybe your PR #391 fixed this issue.

@chrisbanes
Copy link
Contributor

chrisbanes commented May 7, 2021

Just tried on top of tree:

Screen Shot 2021-05-07 at 15 36 55

Box(
    Modifier
        .height(40.dp)
        .fillMaxWidth()
) {
    Image(
        modifier = Modifier
            .background(Color.Red)
            .fillMaxSize(),
        painter = rememberCoilPainter(
            request = imageUrl,
            requestBuilder = {
                transformations(
                    RoundedCornersTransformation(1000f)
                )
            },
        ),
        contentDescription = "",
        contentScale = ContentScale.Crop
    )
}

@chrisbanes
Copy link
Contributor

I'm just about to release 0.9.1 with the fix I mentioned in #391

@chrisbanes
Copy link
Contributor

@chrisbanes where can I find the latest snapshot published? Maybe your PR #391 fixed this issue.

https://google.github.io/accompanist/using-snapshot-version/

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

No branches or pull requests

2 participants