Bug 89052

Summary: Implement css3-images image-orientation
Product: WebKit Reporter: David Barr <davidbarr>
Component: CSSAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: 50167214, 7raivis, antonio, ap, benjamin, bfulgham, cdumez, claude.pache, cmarcelo, commit-queue, david-webkit, dbates, dpaddock, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, gyuyoung.kim, jackalmage, james, jeremya, joepeck, keith_miller, kondapallykalyan, macpherson, marc-webkit, mark.lam, menard, mikelawther, msaboff, noel.gordon, pdr, rniwa, saam, sabouhallawa, simon.fraser, syoichi, tabatkins, thakis, thorton, tzagallo, webkit-bug-importer, zcorpan
Priority: P2 Keywords: InRadar, WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-orientation
Bug Depends on: 89055, 89624, 91566, 91961, 93422, 200553, 201123, 203355    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-highsierra
none
Archive of layout-test-results from ews114 for mac-highsierra
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews101 for mac-highsierra
none
Archive of layout-test-results from ews115 for mac-highsierra
none
Patch
none
Archive of layout-test-results from ews100 for mac-highsierra
none
Archive of layout-test-results from ews114 for mac-highsierra
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description David Barr 2012-06-13 19:07:42 PDT
Add image-orientation support to WebKit.
http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-orientation

The css3-images module is at candidate recommendation.

I propose to introduce the property initially behind a compile flag.
Comment 1 David Barr 2012-06-13 19:11:42 PDT
Advertised on webkit-dev:
http://thread.gmane.org/gmane.os.opendarwin.webkit.devel/21130
Comment 2 Alexey Proskuryakov 2012-06-14 09:43:28 PDT
I wonder why this property doesn't have a value to auto-rotate based on EXIF data.
Comment 3 Simon Fraser (smfr) 2012-06-14 11:50:50 PDT
This property purposefully ignores EXIF, which I think is stupid:
"Note that some devices will "tag" an image with some metadata indicating its correct orientation, so image viewing software can do the necessary transformation themselves. Due to legacy compatibility restraints, Web browsers are required to ignore this data by default. A future level of this specification is expected to have a value that applies the metadata-specified transformation automatically."

I think images with EXIF data are far more common than those encoded sideways

It's also stupid that the spec doesn't use the term "EXIF", which is what most people would search for.
Comment 4 Simon Fraser (smfr) 2012-06-14 11:51:25 PDT
David: do you have any data about how common is the issue that this property is trying to address?
Comment 5 Mike Lawther 2012-06-14 20:09:38 PDT
No hard data on this, other than searching online and finding devleopers asking on forums (eg http://stackoverflow.com/questions/1526448/rotating-an-image-using-css).

This property affects the intrinsic height/width of the image, so if eg you wanted to display a bunch of photos in a gallery and have them all portrait oriented, this would allow you to do that as well as having the layout work.
Comment 6 Simon Fraser (smfr) 2012-06-14 20:56:15 PDT
There's a thread at <http://lists.w3.org/Archives/Public/www-style/2012Feb/0311.html>
Tab says "The spec inherited this property from existing print media stuff".

I don't think this property is particularly relevant to most webkit clients. There are lots of far more pressing things to work on. If you care about image orientation, it would make far more sense to solve the EXIF problem.
Comment 7 Mike Lawther 2012-06-14 21:43:00 PDT
CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to.

Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening?
Comment 8 Tab Atkins Jr. 2012-06-15 08:51:57 PDT
(In reply to comment #7)
> CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to.
> 
> Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening?

Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something.  Fantasai and I plan to start seriously working on Images 4 soonish.

I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later.
Comment 9 Tony Chang 2012-06-22 10:36:41 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to.
> > 
> > Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening?
> 
> Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something.  Fantasai and I plan to start seriously working on Images 4 soonish.

Will can one feature detect between a level 3 implementation and a level 4 implementation?  I guess you have to set image-orientation with a level 4 value and see if you can read it back out?

> I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later.

If Images 4 is starting soonish, maybe it's better to just wait until it's underway.
Comment 10 Simon Fraser (smfr) 2012-06-22 11:07:33 PDT
Bug 19688 is about respecting EXIF.
Comment 11 Tab Atkins Jr. 2012-06-25 10:53:38 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > CSS Images Level 4 sounds like it'll add support for EXIF to image-orientation as you asked for on the thread you linked to.
> > > 
> > > Implementing the Level 3 version will lay necessary groundwork. I know of developers who would use this to eg rotate a logo to display down the side of a page (as this property affects layout). I don't see the harm in implementing this now. Would you actively object to this happening?
> > 
> > Yup, level 4 will have a value that pays attention to EXIF, probably "from-image" or something.  Fantasai and I plan to start seriously working on Images 4 soonish.
> 
> Will can one feature detect between a level 3 implementation and a level 4 implementation?  I guess you have to set image-orientation with a level 4 value and see if you can read it back out?

Yup, that's the standard way to detect new values for an existing property.

(In the future, you'll be able to do this more directly.  In CSS, you'll have the @supports rule, and I imagine there will be a related CSSOM function.)

> > I also agree that implementing what's currently in the draft is of some value, and would lay the groundwork for the EXIF-respecting ability later.
> 
> If Images 4 is starting soonish, maybe it's better to just wait until it's underway.

The property isn't super important yet, but it doesn't hurt to work on it now, and it would be nice to have a passing browser implementation of the tests that I'll be writing soon.
Comment 12 Jeremy Apthorp 2012-11-03 20:00:59 PDT
David is no longer working on Chromium -- Mike, can you reassign?
Comment 13 Marc MAURICE 2015-04-03 11:44:37 PDT
Still nothing on that feature ?

It's in Firefox since ages :
http://sethfowler.org/blog/2013/09/13/new-in-firefox-26-css-image-orientation/

see also chromium issue:
https://code.google.com/p/chromium/issues/detail?id=158753

Thanks in advance !
Marc
Comment 14 Gyuyoung Kim 2015-04-03 19:40:36 PDT
(In reply to comment #13)
> Still nothing on that feature ?
> 
> It's in Firefox since ages :
> http://sethfowler.org/blog/2013/09/13/new-in-firefox-26-css-image-
> orientation/
> 
> see also chromium issue:
> https://code.google.com/p/chromium/issues/detail?id=158753
> 
> Thanks in advance !
> Marc

Hi, current EFL and GTK ports have enabled this feature.
Comment 15 yisibl 2015-07-08 20:34:56 PDT
Nobody to fix it?
Comment 16 Gyuyoung Kim 2015-08-16 19:31:35 PDT
(In reply to comment #15)
> Nobody to fix it?

What bug or feature do you want to fix ?
Comment 17 David Shepherdson 2016-08-21 04:32:11 PDT
In my view, the thing that would really help in general would be support for 'image-orientation: from-image'. That would at least provide a way to be confident images are rendered with the same orientation in iOS and desktop WebKit.
Comment 18 Simon Fraser (smfr) 2017-04-27 11:35:44 PDT
rdar://problem/31834843
Comment 19 Antonio 2019-07-26 07:35:01 PDT
please support the image-orientation from-image feature.  
On websites like this https://www.fabsurplus.com/sdi_catalog/salesItemDetails.do?id=87930  with user generated content images, orientating the pictures correctly would require development work in order to show correctly, while they look correctly already in a browser like Mozilla Firefox that already implements this CSS3 feature that is indeed very handy in a case like this, with just the use of the image-orientation : from-image handle in CSS.
Thanks!
Comment 20 Said Abou-Hallawa 2019-07-26 15:23:32 PDT
The current draft specs is: https://drafts.csswg.org/css-images-4/#image-orientation

It states: "If the image has an orientation specified in its metadata, such as EXIF, the UA must rotate or flip the image to correctly orient it as the metadata specifies."

The MDN claims also the image-orientation property is likely to be deprecated: https://developer.mozilla.org/en-US/docs/Web/CSS/image-orientation.

The currentWebKit implementation of the image orientation is confusing. The image orientation is respected only when the image is opened as an ImageDocument, i.e. standalone image. When the image is embedded in a page as an <img> element or as a css background image, the image orientation is ignored. See RenderElement::shouldRespectImageOrientation(). 

I think it is time to automatically respect the image orientation specified in the image’s metadata.
Comment 21 Simon Fraser (smfr) 2019-07-26 15:59:32 PDT
https://github.com/w3c/csswg-drafts/issues/3799
Comment 22 Said Abou-Hallawa 2019-07-26 16:26:44 PDT
<rdar://problem/15011295>
Comment 23 Said Abou-Hallawa 2019-08-01 18:12:57 PDT
Created attachment 375375 [details]
Patch
Comment 24 EWS Watchlist 2019-08-01 19:16:55 PDT
Comment on attachment 375375 [details]
Patch

Attachment 375375 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12849555

New failing tests:
http/tests/images/draw-pattern-slow-load-large-image.html
Comment 25 EWS Watchlist 2019-08-01 19:16:57 PDT
Created attachment 375380 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 26 EWS Watchlist 2019-08-01 20:57:36 PDT
Comment on attachment 375375 [details]
Patch

Attachment 375375 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12849893

Number of test failures exceeded the failure limit.
Comment 27 EWS Watchlist 2019-08-01 20:57:38 PDT
Created attachment 375390 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 28 Said Abou-Hallawa 2019-08-02 10:40:22 PDT
Created attachment 375423 [details]
Patch
Comment 29 Said Abou-Hallawa 2019-08-02 11:02:20 PDT
Created attachment 375428 [details]
Patch
Comment 30 Simon Fraser (smfr) 2019-08-02 11:17:19 PDT
Comment on attachment 375428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375428&action=review

Can we break this up so it's not such a big rewrite of the existing code?

> Source/WebCore/css/parser/CSSPropertyParser.cpp:2902
> +    if (range.peek().id() == CSSValueAuto || range.peek().id() == CSSValueFromImage) {

There's no 'auto' in the draft spec.
Comment 31 Said Abou-Hallawa 2019-08-02 11:29:12 PDT
Created attachment 375433 [details]
Patch
Comment 32 Said Abou-Hallawa 2019-08-02 11:42:23 PDT
Comment on attachment 375428 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375428&action=review

>> Source/WebCore/css/parser/CSSPropertyParser.cpp:2902
>> +    if (range.peek().id() == CSSValueAuto || range.peek().id() == CSSValueFromImage) {
> 
> There's no 'auto' in the draft spec.

Yes this is right. I meant to use CSSValueNone.  See StyleBuilderConverter::convertImageOrientation(). There, CSSValueNone is used and not CSSValueAuto.
Comment 33 EWS Watchlist 2019-08-02 12:50:35 PDT
Comment on attachment 375433 [details]
Patch

Attachment 375433 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12853466

New failing tests:
http/tests/images/draw-pattern-slow-load-large-image.html
Comment 34 EWS Watchlist 2019-08-02 12:50:37 PDT
Created attachment 375441 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 35 EWS Watchlist 2019-08-02 13:15:42 PDT
Comment on attachment 375433 [details]
Patch

Attachment 375433 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12853511

Number of test failures exceeded the failure limit.
Comment 36 EWS Watchlist 2019-08-02 13:15:44 PDT
Created attachment 375445 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 37 Simon Pieters (:zcorpan) 2019-08-15 05:45:37 PDT
The CSSWG has changed the initial value of image-orientation to "from-image" (aka respect the EXIF orientation by default).

https://github.com/whatwg/html/issues/4495
Comment 38 Simon Pieters (:zcorpan) 2019-08-15 05:47:41 PDT
Sorry, wrong link.

https://github.com/w3c/csswg-drafts/issues/3799
Comment 39 Simon Fraser (smfr) 2019-08-15 09:02:08 PDT
(In reply to Simon Pieters from comment #37)
> The CSSWG has changed the initial value of image-orientation to "from-image"
> (aka respect the EXIF orientation by default).
> 
> https://github.com/whatwg/html/issues/4495

Actually the current proposal is to remove the property entirely, and just have the normal behavior be to respect EXIF.
Comment 40 Simon Pieters (:zcorpan) 2019-08-16 11:51:21 PDT
OK, is that plan in https://github.com/w3c/csswg-drafts/issues/4165#issuecomment-519304078 or somewhere else?
Comment 41 Said Abou-Hallawa 2019-08-19 13:44:15 PDT
Created attachment 376706 [details]
Patch
Comment 42 EWS Watchlist 2019-08-19 15:07:25 PDT
Comment on attachment 376706 [details]
Patch

Attachment 376706 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12942568

New failing tests:
http/tests/images/draw-pattern-slow-load-large-image.html
Comment 43 EWS Watchlist 2019-08-19 15:07:28 PDT
Created attachment 376710 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 44 EWS Watchlist 2019-08-19 15:46:05 PDT
Comment on attachment 376706 [details]
Patch

Attachment 376706 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12942597

New failing tests:
http/tests/images/draw-pattern-slow-load-large-image.html
Comment 45 EWS Watchlist 2019-08-19 15:46:08 PDT
Created attachment 376714 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 46 Said Abou-Hallawa 2019-08-19 19:02:14 PDT
Created attachment 376729 [details]
Patch
Comment 47 Simon Pieters (:zcorpan) 2019-08-20 07:57:30 PDT
It seems the CSSWG resolution was noted in https://github.com/w3c/csswg-drafts/issues/4164 , though the issue is still open and discussion is ongoing.
Comment 48 Said Abou-Hallawa 2019-08-22 12:58:56 PDT
Created attachment 377035 [details]
Patch
Comment 49 Simon Fraser (smfr) 2019-08-22 13:04:08 PDT
Comment on attachment 377035 [details]
Patch

I'm going to make a prediction and say that CSS WG will end up bringing back image-orientation with "none" and "from-image". Given that prediction, I think the most sensible approach right now is to leave everything behind the #ifdef, but just change the default macOS behavior.
Comment 50 Said Abou-Hallawa 2020-01-07 10:48:49 PST
Created attachment 386994 [details]
Patch
Comment 51 Simon Fraser (smfr) 2020-01-07 11:07:04 PST
Comment on attachment 386994 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386994&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        * Configurations/FeatureDefines.xcconfig:

You should say what the change is here.

> Source/WTF/ChangeLog:8
> +        * wtf/FeatureDefines.h:

Ditto.

> Source/WebCore/rendering/style/RenderStyle.h:2112
> +    if (m_rareInheritedData->imageOrientation == ImageOrientation::FromImage)

Can this function just return m_rareInheritedData->imageOrientation ?
Comment 52 Said Abou-Hallawa 2020-01-07 13:37:56 PST
Created attachment 387027 [details]
Patch
Comment 53 Said Abou-Hallawa 2020-01-07 14:08:04 PST
Created attachment 387031 [details]
Patch
Comment 54 WebKit Commit Bot 2020-01-07 22:40:37 PST
Comment on attachment 387031 [details]
Patch

Clearing flags on attachment: 387031

Committed r254187: <https://trac.webkit.org/changeset/254187>
Comment 55 WebKit Commit Bot 2020-01-07 22:40:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 56 noel gordon 2020-01-21 03:14:39 PST
Cool.  Any tests planned for drawing an EXIF image to a <canvas> element?
Comment 57 Said Abou-Hallawa 2020-01-21 09:29:40 PST
(In reply to noel gordon from comment #56)
> Cool.  Any tests planned for drawing an EXIF image to a <canvas> element?

When drawing images on a <canvas> element the EXIF image orientation is always respected. The test LayoutTests/fast/images/exif-orientation-canvas.html covers this case.
Comment 58 Simon Pieters (:zcorpan) 2020-01-21 14:41:31 PST
(In reply to Said Abou-Hallawa from comment #57)
> The test
> LayoutTests/fast/images/exif-orientation-canvas.html covers this case.

Is there a reason the tests here were not upstreamed to web-platform-tests?