Closed Bug 1616411 Opened 5 years ago Closed 5 years ago

CSS decorative images should respect EXIF-orientation by default

Categories

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

Product:
Core
Component:
Layout: Images, Video, and HTML Frames
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox77 --- fixed

People

(Reporter: mozilla-apprentice, Assigned: heycam)

References

(Regression)

Details

(Keywords: regression, site-compat)

Attachments

(9 files, 1 obsolete file)

Bug 1616411 - Part 1: Split out some helper methods from OrientedImage.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 2: Don't bother passing in the size to OrientedImage::OrientSurface.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 3: Make RasterImage deal with and apply image orientation.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 4: Make nsLayoutUtils::OrientImage undo any automatic RasterImage orientation when required.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 5: Make naturalWidth/naturalHeight getters take RasterImage orientation handling into account.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 6: When -moz-element references an image, use the target orientation.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 7: Tests.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 4a: Make SurfaceCache aware that native image sizes can be affected by orientation.
(deleted), text/x-phabricator-request
Details
Bug 1616411 - Part 6a: Make OrientedImage::GetFrameAtSize return an appropriately sized surface.
(deleted), text/x-phabricator-request
Details

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.)

Attached file Bug 1616411 - Part 7: Tests. (deleted) —
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 handlerrunNextTest/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 handlerrunNextTest@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

Creator:
Created:
Updated:
Size: