Bug 221005 – Incorrect sizing of uploaded images when using background-size:cover

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Kenneth Kufluk]

Created attachment 418467 [details]
Test image for upload

On twitter.com the website, when uploading an image, you are able to hit 'edit' and zoom/crop the image.

On iPhone when adding portrait images directly from camera, there is noticable flicker when later zooming the image.
This can be reproduced on desktop safari (14 and STP) by airdropping the image to macbook.

This seems to be a rendering error when sizing an image to certain dimensions.

To reproduce in reduced case:

Visit http://jsfiddle.net/r82facvo/7/
Run the code.
Upload the attached image (floor.jpg) which was taken with iPhone 12 and converted to jpg on macbook.
Observe that one of the divs showing the image has a large white block underneath.

[Kenneth Kufluk]

Created attachment 418468 [details]
screenshot of the demo showing the erroneous white block

[Smoley]

Thanks for filing, is this jsfiddle still live? I can't seem to get it to load.

[Kenneth Kufluk]

Yes, it should still be live. If Glitch is easier, I've posted it to:
https://legend-gusty-leptoceratops.glitch.me

With the code available via the fish menu or at https://glitch.com/edit/#!/legend-gusty-leptoceratops

[Radar WebKit Bug Importer]

<rdar://problem/73692426>

[Simon Fraser (smfr)]

This looks like a bug in a framework below webkit.

[Kenneth Kufluk]

Adding a border seems to help, eg

.img { border: 1px solid transparent }

[Simon Fraser (smfr)]

I take that back. The floor.jpg image has an EXIF rotation, so we get to GraphicsContext::drawPlatformImage() and hit this branch:

            if (options.orientation() != ImageOrientation::None) {
                if (auto transform = options.orientation().transformFromDefault(imageSize).inverse())
                    subimageRect = transform.value().mapRect(subimageRect);
            }

which is where I think things go wrong. We also hit the subsequent assertion:
    ASSERT(CGImageGetHeight(subImage.get()) == currHeight - CGRectIntegral(srcRect).origin.y);

Attaching a standalone testcase.

[Simon Fraser (smfr)]

Created attachment 418610 [details]
Testcase

[Kenneth Kufluk]

Ah, that explains why the photo needed to be from the camera rather than one that had been manipulated (and thus lost the exif rotation).

[Cameron McCormack (:heycam)]

(In reply to Simon Fraser (smfr) from comment #7)
> I take that back. The floor.jpg image has an EXIF rotation, so we get to
> GraphicsContext::drawPlatformImage() and hit this branch:
> 
>             if (options.orientation() != ImageOrientation::None) {
>                 if (auto transform =
> options.orientation().transformFromDefault(imageSize).inverse())
>                     subimageRect = transform.value().mapRect(subimageRect);
>             }

I think this part's right.

> which is where I think things go wrong. We also hit the subsequent assertion:
>     ASSERT(CGImageGetHeight(subImage.get()) == currHeight -
> CGRectIntegral(srcRect).origin.y);

But this is wrong.  We need to be getting the oriented height of the image, since currHeight and srcRect are in "logical" space, and subImage deals with unoriented coordinates.  Same for the following line, which I think is causing the height to be wrongly adjusted.

[Cameron McCormack (:heycam)]

Created attachment 427634 [details]
Patch

[Cameron McCormack (:heycam)]

Created attachment 427635 [details]
Patch

[EWS]

Committed r276984 (237310@main): <https://commits.webkit.org/237310@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427635 [details].

[Kenneth Kufluk]

Thanks for the investigation and fix! I'm really excited to try it out and see the effect. ❤️🙏