201123 – EXIF orientation should be respected when rendering images

RESOLVED FIXED 201123

EXIF orientation should be respected when rendering images

https://bugs.webkit.org/show_bug.cgi?id=201123

Summary EXIF orientation should be respected when rendering images

Said Abou-Hallawa

Reported 2019-08-25 01:33:09 PDT

The EXIF orientation should be respected when rendering images in the following cases: 1. The image is the src of an <img> element 2. The image is the css background image of an HTML element 3. The image is the src of an SVG <image> element 4. The image is the src of an SVG <feImage> element 5. Drawing the image on a canvas 6. The image is the css content of an HTML element 7. The image is an ImageDocument 8. The image is data of an <object> element This should be the behavior on all ports till the css image-orientation specs is finalized.

Attachments
Patch (247.15 KB, patch) 2019-08-25 01:58 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (252.79 KB, patch) 2019-08-27 09:34 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (250.14 KB, patch) 2019-08-27 12:25 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (257.24 KB, patch) 2019-08-27 23:04 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (256.28 KB, patch) 2019-08-28 08:38 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch for review (100.63 KB, patch) 2019-08-28 10:04 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (103.49 KB, patch) 2019-08-28 15:23 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (103.52 KB, patch) 2019-08-28 16:01 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Patch (161.89 KB, patch) 2019-08-31 14:45 PDT, Said Abou-Hallawa	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Said Abou-Hallawa

Comment 1 2019-08-25 01:58:16 PDT

Created attachment 377221 [details] Patch

Radar WebKit Bug Importer

Comment 2 2019-08-26 16:50:54 PDT

<rdar://problem/54728386>

Said Abou-Hallawa

Comment 3 2019-08-27 09:34:17 PDT

Created attachment 377346 [details] Patch

Said Abou-Hallawa

Comment 4 2019-08-27 12:25:04 PDT

Created attachment 377362 [details] Patch

Said Abou-Hallawa

Comment 5 2019-08-27 23:04:23 PDT

Created attachment 377428 [details] Patch

Said Abou-Hallawa

Comment 6 2019-08-28 08:38:37 PDT

Created attachment 377444 [details] Patch

Said Abou-Hallawa

Comment 7 2019-08-28 10:04:12 PDT

Created attachment 377454 [details] Patch for review

Said Abou-Hallawa

Comment 8 2019-08-28 15:23:18 PDT

Created attachment 377493 [details] Patch

Said Abou-Hallawa

Comment 9 2019-08-28 16:01:15 PDT

Created attachment 377502 [details] Patch

Simon Fraser (smfr)

Comment 10 2019-08-28 17:55:58 PDT

Comment on attachment 377502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377502&action=review > LayoutTests/fast/images/exif-orientation-composited.html:-3 > -<html> > -<head> > -<script> This is an important test to keep. It tests the directly-composited image code path (see RenderLayerBacking::isDirectlyCompositedImage()).

Said Abou-Hallawa

Comment 11 2019-08-31 14:45:25 PDT

Created attachment 377807 [details] Patch

WebKit Commit Bot

Comment 12 2019-08-31 17:47:46 PDT

Comment on attachment 377807 [details] Patch Clearing flags on attachment: 377807 Committed r249364: <https://trac.webkit.org/changeset/249364>

WebKit Commit Bot

Comment 13 2019-08-31 17:47:48 PDT

All reviewed patches have been landed. Closing bug.

noel gordon

Comment 14 2019-09-01 17:09:35 PDT

(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 377502 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377502&action=review > > > LayoutTests/fast/images/exif-orientation-composited.html:-3 > > -<html> > > -<head> > > -<script> > > This is an important test to keep. It tests the directly-composited image > code path (see RenderLayerBacking::isDirectlyCompositedImage()).

noel gordon

Comment 15 2019-09-01 17:33:04 PDT

Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review Very nice set of tests, and thank you for adding them. Couple of comments ... > LayoutTests/fast/images/exif-orientation-background-expected.html:11 > + width: 102; Maybe 102px here? Noticed px missing here and in other tests. > LayoutTests/fast/images/exif-orientation-composited.html:20 > + -webkit-transform: translateZ(0); > + border: 1px solid black; Last I looked at webkit code in detail (a few years back), having CSS border style on a image disallowed direct compositing of the image. Maybe border style no longer does that?

Simon Fraser (smfr)

Comment 16 2019-09-03 11:12:50 PDT

(In reply to noel gordon from comment #15) > Comment on attachment 377807 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377807&action=review > > Very nice set of tests, and thank you for adding them. Couple of comments > ... > > > LayoutTests/fast/images/exif-orientation-background-expected.html:11 > > + width: 102; > > Maybe 102px here? Noticed px missing here and in other tests. > > > LayoutTests/fast/images/exif-orientation-composited.html:20 > > + -webkit-transform: translateZ(0); > > + border: 1px solid black; > > Last I looked at webkit code in detail (a few years back), having CSS border > style on a image disallowed direct compositing of the image. Maybe border > style no longer does that? You're right; borders will kick you out of the directly composited image code path.

Said Abou-Hallawa

Comment 17 2019-09-03 12:00:46 PDT

https://bugs.webkit.org/show_bug.cgi?id=201432 addresses the last review comments.

noel gordon

Comment 18 2019-09-04 03:57:56 PDT

Thanks for checking, Said.

Manuel Rego Casasnovas

Comment 19 2019-11-28 01:15:25 PST

Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review > Source/WebCore/rendering/RenderElement.cpp:-2063 > - if (settings().shouldRespectImageOrientation() && is<HTMLImageElement>(element())) If I'm not wrong this was the only usage of shouldRespectImageOrientation(). After this change do we still need "shouldRespectImageOrientation" flag defined at WebCore/page/Settings.yaml?

Said Abou-Hallawa

Comment 20 2019-12-02 10:58:22 PST

Comment on attachment 377807 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377807&action=review >> Source/WebCore/rendering/RenderElement.cpp:-2063 >> - if (settings().shouldRespectImageOrientation() && is<HTMLImageElement>(element())) > > If I'm not wrong this was the only usage of shouldRespectImageOrientation(). > After this change do we still need "shouldRespectImageOrientation" flag defined at WebCore/page/Settings.yaml? No. We do not really need it. I kept the shouldRespectImageOrientation setting because the preference key WebKitShouldRespectImageOrientation is still using it. But surely we need to clean up this area.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component Images

Assignee

Said Abou-Hallawa

Reported

2019-08-25 01:33 PDT

Modified

2019-12-02 10:58 PST History

CC List

5 users Show

URL

Keywords InRadar

Depends on

201059

Blocks

89052

Dependencies

tree graph