Bug 89052 – Implement css3-images image-orientation

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[David Barr]

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.

[David Barr]

Advertised on webkit-dev:
http://thread.gmane.org/gmane.os.opendarwin.webkit.devel/21130

[Alexey Proskuryakov]

I wonder why this property doesn't have a value to auto-rotate based on EXIF data.

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

David: do you have any data about how common is the issue that this property is trying to address?

[Mike Lawther]

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.

[Simon Fraser (smfr)]

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.

[Mike Lawther]

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?

[Tab Atkins Jr.]

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

[Tony Chang]

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

[Simon Fraser (smfr)]

Bug 19688 is about respecting EXIF.

[Tab Atkins Jr.]

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

[Jeremy Apthorp]

David is no longer working on Chromium -- Mike, can you reassign?

[Marc MAURICE]

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

[Gyuyoung Kim]

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

[yisibl]

Nobody to fix it？

[Gyuyoung Kim]

(In reply to comment #15)
> Nobody to fix it？

What bug or feature do you want to fix ?

[David Shepherdson]

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.

[Simon Fraser (smfr)]

rdar://problem/31834843

[Antonio]

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!

[Said Abou-Hallawa]

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.

[Simon Fraser (smfr)]

https://github.com/w3c/csswg-drafts/issues/3799

[Said Abou-Hallawa]

<rdar://problem/15011295>

[Said Abou-Hallawa]

Created attachment 375375 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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.

[EWS Watchlist]

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

[Said Abou-Hallawa]

Created attachment 375423 [details]
Patch

[Said Abou-Hallawa]

Created attachment 375428 [details]
Patch

[Simon Fraser (smfr)]

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.

[Said Abou-Hallawa]

Created attachment 375433 [details]
Patch

[Said Abou-Hallawa]

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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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.

[EWS Watchlist]

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

[Simon Pieters (:zcorpan)]

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

[Simon Pieters (:zcorpan)]

Sorry, wrong link.

https://github.com/w3c/csswg-drafts/issues/3799

[Simon Fraser (smfr)]

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

[Simon Pieters (:zcorpan)]

OK, is that plan in https://github.com/w3c/csswg-drafts/issues/4165#issuecomment-519304078 or somewhere else?

[Said Abou-Hallawa]

Created attachment 376706 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Said Abou-Hallawa]

Created attachment 376729 [details]
Patch

[Simon Pieters (:zcorpan)]

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.

[Said Abou-Hallawa]

Created attachment 377035 [details]
Patch

[Simon Fraser (smfr)]

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.

[Said Abou-Hallawa]

Created attachment 386994 [details]
Patch

[Simon Fraser (smfr)]

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 ?

[Said Abou-Hallawa]

Created attachment 387027 [details]
Patch

[Said Abou-Hallawa]

Created attachment 387031 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 387031 [details]
Patch

Clearing flags on attachment: 387031

Committed r254187: <https://trac.webkit.org/changeset/254187>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[noel gordon]

Cool.  Any tests planned for drawing an EXIF image to a <canvas> element?

[Said Abou-Hallawa]

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

[Simon Pieters (:zcorpan)]

(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?