Closed Bug 1616411 Opened 4 years ago Closed 4 years ago

CSS decorative images should respect EXIF-orientation by default

Categories

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: mozilla-apprentice, Assigned: heycam)

References

(Regression)

Details

(Keywords: regression, site-compat)

Attachments

(9 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

A resolution was made for csswg-drafts/#4165.

[css-images] Should CSS decorative images respect EXIF-orientation by default

  • RESOLVED: default behavior for all images to respect exif orientation

Discussion.

It's difficult to tell from the meeting minutes whether image-orientation should apply to CSS images (and so provide the option of opting out of re-orienting), or if the EXIF orientation data must always be used.

Assignee: nobody → cam
Status: NEW → ASSIGNED
Summary: [css-images] Should CSS decorative images respect EXIF-orientation by default → CSS decorative images should respect EXIF-orientation by default

My reading from the sidelines is that image-orientation ought to go away and intrinsic orientation (i.e., EXIF) ought to always be respected. That doesn't seem terrible to me, but I'm not sure how compatible that is and if that's everyone's understanding at this point.

RasterImage will make use of them.

Note that there is one bug fix in this patch, which is that
OrientedImage::OrientSurface now creates a surface of the correct size.

(Previously this code was creating a surface with the underlying
image's size, rather than the correctly oriented size. But we must
not have been calling into that code with our current uses of
OrientedImage.)

We can get the size from the surface directly.

This makes EXIF orientation metadata honored by default.

Introduce OrientedPixel and UnorientedPixel typed rects and sizes and
use them throughout RasterImage so that we don't confuse which we want.

The reason for doing this rather than having the imgLoader wrap every
RasterImage it creates with an OrientedImage is that returning the
wrapper messes with various notifications, as OrientedImage is not an
ImageResource.

(It would be even better if the JPEG decoder could decode to imgFrames
handling the EXIF orientation itself, but that's a more complicated
change.)

Attachment #9135250 - Attachment is obsolete: true

I posted a patch for bug 1628532. Let me know if you'd prefer I hold off landing it.

Happy to rebase my patches on top of yours if you land first. (It's the long weekend for me here now so I may not get to landing mine until next week.)

Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/034a30a6b088
Part 1: Split out some helper methods from OrientedImage. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/4dee3e753e8e
Part 2: Don't bother passing in the size to OrientedImage::OrientSurface. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/0f54b1b12105
Part 3: Make RasterImage deal with and apply image orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/dc64af52b5f8
Part 4: Make nsLayoutUtils::OrientImage undo any automatic RasterImage orientation when required. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/1a25353a07b0
Part 4a: Make SurfaceCache aware that native image sizes can be affected by orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/0a323c33506f
Part 5: Make naturalWidth/naturalHeight getters take RasterImage orientation handling into account. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/cfee2ce9405d
Part 6: When -moz-element references an image, use the target orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/03dd88d53439
Part 7: Tests. r=tnikkel
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22986 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Backed out for failures on test_2_conformance__textures__misc__texture-upload-size.html and test_conformance__textures__misc__texture-upload-size.html

backout: https://hg.mozilla.org/integration/autoland/rev/8e04043f85c28028d96f929ca6d315292723aa9f

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=03dd88d5343947daa668587e424aff9de40b2fdd&searchStr=webgl&selectedJob=297858517

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297858517&repo=autoland&lineNumber=5266

[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - TEST-UNEXPECTED-FAIL | dom/canvas/test/webgl-conf/generated/test_conformance__textures__misc__texture-upload-size.html | Texture was smaller than the expected size 4x4
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:299:16
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - reportResults@dom/canvas/test/webgl-conf/mochi-single.html?checkout/conformance/textures/misc/texture-upload-size.html:22:14
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - reportTestResultsToHarness@dom/canvas/test/webgl-conf/checkout/js/js-test-pre.js:94:36
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - testFailed@dom/canvas/test/webgl-conf/checkout/js/js-test-pre.js:224:31
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - checkTextureSize@dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js:1458:15
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - testUpload@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:34:7
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - testImage@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:50:15
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - runNextTest/img<@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:115:18
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - EventHandlerNonNullmakeImage@dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js:2568:5
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - runNextTest@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:114:21
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - setTimeout handler
runNextTest/img<@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:116:19
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - EventHandlerNonNullmakeImage@dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js:2568:5
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - runNextTest@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:114:21
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - setTimeout handler
runNextTest@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:140:17
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - setTimeout handler*runNextTest@dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:136:17
[task 2020-04-16T02:35:40.110Z] 02:35:40 INFO - @dom/canvas/test/webgl-conf/checkout/conformance/textures/misc/texture-upload-size.html:147:1
[task 2020-04-16T02:35:40.111Z] 02:35:40 INFO - GECKO(2003) | JavaScript warning: http://mochi.test:8888/tests/dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js, line 1461: WebGL warning: texSubImage: Offset+size must be <= the size of the existing specified image.
[task 2020-04-16T02:35:40.111Z] 02:35:40 INFO - GECKO(2003) | JavaScript warning: http://mochi.test:8888/tests/dom/canvas/test/webgl-conf/checkout/js/webgl-test-utils.js, line 1466: WebGL warning: texSubImage: Offset+size must be <= the size of the existing specified image.
[task 2020-04-16T02:35:40.111Z] 02:35:40 INFO - TEST-PASS | dom/canvas/test/webgl-conf/generated/test_conformance__textures__misc__texture-upload-size.html | getError was expected value: NO_ERROR : when calling texSubImage2D with the same texture upload

Flags: needinfo?(cam)
Upstream PR was closed without merging

We need this since nsLayoutUtils::SurfaceFromElement expects the
returned frame size to be correct, and we are now wrapping a source
element's image with an OrientedImage.

Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a31395761d3b
Part 1: Split out some helper methods from OrientedImage. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/e2221bf76463
Part 2: Don't bother passing in the size to OrientedImage::OrientSurface. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/747bdbeee667
Part 3: Make RasterImage deal with and apply image orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/215c7a1c9945
Part 4: Make nsLayoutUtils::OrientImage undo any automatic RasterImage orientation when required. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/7e0db19e88cf
Part 4a: Make SurfaceCache aware that native image sizes can be affected by orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/65aaf1efc60c
Part 5: Make naturalWidth/naturalHeight getters take RasterImage orientation handling into account. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/35f6bf00a95e
Part 6: When -moz-element references an image, use the target orientation. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/c30a54ef15b5
Part 6a: Make OrientedImage::GetFrameAtSize return an appropriately sized surface. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/325e655b6024
Part 7: Tests. r=tnikkel
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR merged by moz-wptsync-bot
Regressed by: 1630964
Has Regression Range: --- → yes
Keywords: regression
Regressions: 1631187
Regressions: 1631188
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: