.
<rdar://problem/83753956>
Filed https://github.com/whatwg/html/issues/7210 to get clarity in the HTML spec about exactly how EXIF orientation and the image-orientation property should influence createImageBitmap.
Created attachment 441558 [details] Patch
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/31287
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Created attachment 441560 [details] Patch
Created attachment 441561 [details] Patch
Comment on attachment 441561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441561&action=review > Source/WebCore/html/ImageBitmap.cpp:391 > + auto orientation = imageForRender->orientation(); > + if (orientation == ImageOrientation::FromImage) > + orientation = ImageOrientation::None; I've had to write similar code to this elsewhere. I think we should try to push this complexity somewhere lower level. I'm not sure where, though. > Source/WebCore/platform/graphics/ImageOrientation.h:111 > + ImageOrientation flipY() const Can this be named something like "ImageOrientationWithFlippedY()"? It sounds like it would be a non-const function.
Comment on attachment 441561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441561&action=review > Source/WebCore/html/ImageBitmap.cpp:238 > +static ImageOrientation imageOrientationForOrientation(ImageBitmapOptions::Orientation imageBitmapOrientation, ImageOrientation imageOrientation = ImageOrientation::None) This function is confusing: 1. The name does not reveal what it converts from and what it converts to. We have ImageBitmapOptions::Orientation and ImageOrientation. So which one is "imageOrientation" and which one is "Orientation"? 2. The second name of the second parameter "imageOrientation" does not reveal what it represents. Can it be something "defaultImageOrientation"? > Source/WebCore/html/ImageBitmap.cpp:389 > + auto orientation = imageForRender->orientation(); imageForRender -> imageForRenderer
(In reply to Myles C. Maxfield from comment #8) > I've had to write similar code to this elsewhere. I think we should try to > push this complexity somewhere lower level. I'm not sure where, though. It might be an artifact of Image::orientation() having a default implementation that returns FromImage. Maybe it should be returning None. (I'll file a followup.)
(In reply to Said Abou-Hallawa from comment #9) > > Source/WebCore/html/ImageBitmap.cpp:238 > > +static ImageOrientation imageOrientationForOrientation(ImageBitmapOptions::Orientation imageBitmapOrientation, ImageOrientation imageOrientation = ImageOrientation::None) > > This function is confusing: I think you're right that the name is no longer appropriate. I'll rename. (But I guess I'll leave the other two similarly named functions, interpolationQualityForResizeQuality and alphaPremultiplicationForPremultiplyAlpha, since they're pure conversion functions.)
Created attachment 441678 [details] Patch for landing
(In reply to Cameron McCormack (:heycam) from comment #10) > (In reply to Myles C. Maxfield from comment #8) > > I've had to write similar code to this elsewhere. I think we should try to > > push this complexity somewhere lower level. I'm not sure where, though. > > It might be an artifact of Image::orientation() having a default > implementation that returns FromImage. Maybe it should be returning None. > (I'll file a followup.) Filed bug 231934.
Committed r284436 (243198@main): <https://commits.webkit.org/243198@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441678 [details].
Anyone know when this will land in Safari? Currently, in Safari 15, EXIF orientations 5-8 are not handled correctly. Oddly enough, 1-4 are handled just fine. We still have to use ImageElement in Safari (instead of ImageBitmap) when rendering images to Canvas / WebGL texture to work around this bug. I'm not positive if this exact bug is tracked by this issue, but it seems to be related.
(In reply to Micah from comment #15) > Anyone know when this will land in Safari? This change was included in Safari 15.2. > Currently, in Safari 15, EXIF orientations 5-8 are not handled correctly. > Oddly enough, 1-4 are handled just fine. > > We still have to use ImageElement in Safari (instead of ImageBitmap) when > rendering images to Canvas / WebGL texture to work around this bug. I'm not > positive if this exact bug is tracked by this issue, but it seems to be > related. Either sounds like a related issue, or the fix for this bug wasn't complete. Could you file a new bug with a description of the issue you're seeing and CC me? Thank you.
(In reply to Cameron McCormack (:heycam) from comment #16) > (In reply to Micah from comment #15) > > Anyone know when this will land in Safari? > > This change was included in Safari 15.2. > > > Currently, in Safari 15, EXIF orientations 5-8 are not handled correctly. > > Oddly enough, 1-4 are handled just fine. > > > > We still have to use ImageElement in Safari (instead of ImageBitmap) when > > rendering images to Canvas / WebGL texture to work around this bug. I'm not > > positive if this exact bug is tracked by this issue, but it seems to be > > related. > > Either sounds like a related issue, or the fix for this bug wasn't complete. > Could you file a new bug with a description of the issue you're seeing and > CC me? Thank you. Thanks Cameron! I created a new issue here: https://bugs.webkit.org/show_bug.cgi?id=242137 Turns out I was confused about EXIF orientations 1-4 working. Looks like none of them are working (except 1 of course).