In support of ongoing css3-images image-orientation implementation, extend the platform graphics interfaces to accept an explicit orientation.
Created attachment 154534 [details] Patch
Comment on attachment 154534 [details] Patch Attachment 154534 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13361323
Comment on attachment 154534 [details] Patch Attachment 154534 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13344531
Comment on attachment 154534 [details] Patch Attachment 154534 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13353492
Created attachment 154536 [details] Patch
Comment on attachment 154536 [details] Patch Simon would be a good reviewer for this patch. It feels like we should somehow merge ImageOrientationEnum and RespectImageOrientationEnum instead of passing them both around. Maybe a struct or maybe we can add RespectImageOrientationEnum to the ImageOrientation class.
Comment on attachment 154536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154536&action=review > Source/WebCore/page/Frame.cpp:1109 > + ImageOrientationEnum orientation = OriginTopLeft; OriginTopLeft is so ambiguous here given the various things that can affect rotation (encoded orientation, EXIF, context transform etc).
Comment on attachment 154536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154536&action=review >> Source/WebCore/page/Frame.cpp:1109 >> + ImageOrientationEnum orientation = OriginTopLeft; > > OriginTopLeft is so ambiguous here given the various things that can affect rotation (encoded orientation, EXIF, context transform etc). Would it be better to use DefaultImageOrientation here? I'm not sure if that's any better. Would it help to change the parameter to be named something like orientationFromCSS?
If David doesn't continue to work on this bug anymore, I'd like to take this bug over. David, please give your comment.
I am limited to working on this in my leisure time, as much as I'd like to see this complete. I welcome anyone who has time to push it forward.
(In reply to comment #10) > I am limited to working on this in my leisure time, > as much as I'd like to see this complete. > > I welcome anyone who has time to push it forward. Thanks, I will submit a rebased patch with previous reviewer's comments. Please take a look it when you are available.
Created attachment 197226 [details] Patch for EWS David, if you don't mind to add my name as second, please let me know. I will remove myself then. :)
(In reply to comment #12) don't mind -> you mind.
Comment on attachment 197226 [details] Patch for EWS Attachment 197226 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17693012
Comment on attachment 197226 [details] Patch for EWS Attachment 197226 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/17744005
Comment on attachment 197226 [details] Patch for EWS Attachment 197226 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/17723049
Created attachment 197514 [details] Patch for EWS
Comment on attachment 154536 [details] Patch Clearing r? because of out of date.
(In reply to comment #7) > (From update of attachment 154536 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154536&action=review > > > Source/WebCore/page/Frame.cpp:1109 > > + ImageOrientationEnum orientation = OriginTopLeft; > > OriginTopLeft is so ambiguous here given the various things that can affect rotation (encoded orientation, EXIF, context transform etc). Simon, could you take a look latest patch again ? I change OriginTopLeft with DefaultImageOrientation.
Created attachment 199619 [details] Patch
Rebased.
Comment on attachment 199619 [details] Patch Attachment 199619 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/113579 New failing tests: editing/pasteboard/4641033.html
Created attachment 199633 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2
Created attachment 206602 [details] Patch
I succeed to pass image-orientation layout test using a patch on Bug 118454. But, the patch only can pass on efl and gtk ports yet. I think I'm able to support mac and qt port soon. To enable css-image-orientation, this patch needs to be landed first. Simon, could you take a look last patch ? Below is to test if css-image-orientation can work for efl and gtk. https://bugs.webkit.org/attachment.cgi?id=206593&action=review
Created attachment 206964 [details] Updated patch based on Bug 118454
I'd like to work detail work on Bug 91566. So, this patch should be landed first. Please review.
CC'ing Beth, would you please review this patch ?
Comment on attachment 206964 [details] Updated patch based on Bug 118454 View in context: https://bugs.webkit.org/attachment.cgi?id=206964&action=review > Source/WebCore/ChangeLog:12 > + No new tests; interface change only - no change in behavior. If there is no behavior change, what is the point of this patch? Please edit the ChangeLog to explain the point of this patch. Is it laying groundwork for a behavior change?
(In reply to comment #6) > (From update of attachment 154536 [details]) > Simon would be a good reviewer for this patch. > > It feels like we should somehow merge ImageOrientationEnum and RespectImageOrientationEnum instead of passing them both around. Maybe a struct or maybe we can add RespectImageOrientationEnum to the ImageOrientation class. I agree with this comment as well.
(In reply to comment #30) > (In reply to comment #6) > > (From update of attachment 154536 [details] [details]) > > Simon would be a good reviewer for this patch. > > > > It feels like we should somehow merge ImageOrientationEnum and RespectImageOrientationEnum instead of passing them both around. Maybe a struct or maybe we can add RespectImageOrientationEnum to the ImageOrientation class. > > I agree with this comment as well. Ok, I will update patch according to this comment. Thank you for your review.
(In reply to comment #29) > (From update of attachment 206964 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206964&action=review > > > Source/WebCore/ChangeLog:12 > > + No new tests; interface change only - no change in behavior. > > If there is no behavior change, what is the point of this patch? Please edit the ChangeLog to explain the point of this patch. Is it laying groundwork for a behavior change? Yes, this patch is one of groundwork in order to let draw layer know image orientation value. I will update ChangeLog in next patch. Thanks.
Beth, I think we can handle it on new bug. So, I file a bug 119418. I will submit a patch to there soon.
Created attachment 208899 [details] Patch
Bug 119418 covered the patch of original this bug as well. So, role of this bug needs to be changed. Latest patch is to pass information of image orientation to drawImage() function so that the function knows what is desired orientation.
Comment on attachment 208899 [details] Patch Attachment 208899 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1471439
Created attachment 208926 [details] Patch
This patch looks good. Quick question before I r+ though, is there any was to test this?
Comment on attachment 208926 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208926&action=review > Source/WebCore/rendering/RenderImage.cpp:359 > + orientationDescription.setImageOrientationEnum(style()->imageOrientation()); Why don't we orientationDescription.setRespectImageOrientation here like we do down below?
(In reply to comment #38) > This patch looks good. Quick question before I r+ though, is there any was to test this? There is a test for image-orientation in below path. LayoutTests/fast/css/image-orientation/image-orientation.html Because this patch is one of steps to support it, this patch doesn't mention the test yet. Bug 91566 will supports the test after landing this patch.
Created attachment 209156 [details] Patch
(In reply to comment #39) > (From update of attachment 208926 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208926&action=review > > > Source/WebCore/rendering/RenderImage.cpp:359 > > + orientationDescription.setImageOrientationEnum(style()->imageOrientation()); > > Why don't we orientationDescription.setRespectImageOrientation here like we do down below? Ok, fixed it.
Comment on attachment 209156 [details] Patch Attachment 209156 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1503789 New failing tests: fast/workers/termination-with-port-messages.html
Created attachment 209165 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Created attachment 209167 [details] Patch
(In reply to comment #43) > (From update of attachment 209156 [details]) > Attachment 209156 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/1503789 > > New failing tests: > fast/workers/termination-with-port-messages.html It looks this failing is unrelated with this patch. I upload this patch again.
Comment on attachment 209167 [details] Patch Clearing flags on attachment: 209167 Committed r154375: <http://trac.webkit.org/changeset/154375>
All reviewed patches have been landed. Closing bug.
Comment on attachment 209167 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209167&action=review > Source/WebCore/ChangeLog:1 > +2013-08-19 David Barr <davidbarr@chromium.org>, Gyuyoung Kim <gyuyoung.kim@samsung.com> Wrong format. We never list two people on the first line like this. Instead, mention the original author in the long description.
(In reply to comment #49) > (From update of attachment 209167 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209167&action=review > > > Source/WebCore/ChangeLog:1 > > +2013-08-19 David Barr <davidbarr@chromium.org>, Gyuyoung Kim <gyuyoung.kim@samsung.com> > > Wrong format. We never list two people on the first line like this. > Instead, mention the original author in the long description. IIRC, I saw similar line in ChangeLog before. Anyway, I will do that in next patch. Thank you for notifying it to me.
(In reply to comment #50) > (In reply to comment #49) > > (From update of attachment 209167 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=209167&action=review > > > > > Source/WebCore/ChangeLog:1 > > > +2013-08-19 David Barr <davidbarr@chromium.org>, Gyuyoung Kim <gyuyoung.kim@samsung.com> > > > > Wrong format. We never list two people on the first line like this. > > Instead, mention the original author in the long description. > > IIRC, I saw similar line in ChangeLog before. Anyway, I will do that in next patch. Thank you for notifying it to me. I have seen and used it as well. Should be ok ..
This caused https://bugs.webkit.org/show_bug.cgi?id=123831