WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
92330
[CSS] Pass an image orientation data to drawImage()
https://bugs.webkit.org/show_bug.cgi?id=92330
Summary
[CSS] Pass an image orientation data to drawImage()
David Barr
Reported
2012-07-25 21:11:47 PDT
In support of ongoing css3-images image-orientation implementation, extend the platform graphics interfaces to accept an explicit orientation.
Attachments
Patch
(40.67 KB, patch)
2012-07-25 21:36 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Patch
(42.75 KB, patch)
2012-07-25 21:53 PDT
,
David Barr
no flags
Details
Formatted Diff
Diff
Patch for EWS
(40.41 KB, patch)
2013-04-10 02:23 PDT
,
Gyuyoung Kim
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Patch for EWS
(40.72 KB, patch)
2013-04-11 00:15 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(41.23 KB, patch)
2013-04-24 22:42 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
(604.04 KB, application/zip)
2013-04-25 01:29 PDT
,
Build Bot
no flags
Details
Patch
(41.26 KB, patch)
2013-07-13 05:27 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Updated patch based on Bug 118454
(42.10 KB, patch)
2013-07-18 01:38 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(13.93 KB, patch)
2013-08-16 02:52 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(13.95 KB, patch)
2013-08-16 08:19 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(14.08 KB, patch)
2013-08-19 21:32 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
(485.49 KB, application/zip)
2013-08-20 01:28 PDT
,
Build Bot
no flags
Details
Patch
(14.08 KB, patch)
2013-08-20 01:35 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Show Obsolete
(10)
View All
Add attachment
proposed patch, testcase, etc.
David Barr
Comment 1
2012-07-25 21:36:17 PDT
Created
attachment 154534
[details]
Patch
Early Warning System Bot
Comment 2
2012-07-25 21:47:58 PDT
Comment on
attachment 154534
[details]
Patch
Attachment 154534
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/13361323
Early Warning System Bot
Comment 3
2012-07-25 21:51:38 PDT
Comment on
attachment 154534
[details]
Patch
Attachment 154534
[details]
did not pass qt-wk2-ews (qt): Output:
http://queues.webkit.org/results/13344531
Gyuyoung Kim
Comment 4
2012-07-25 21:52:37 PDT
Comment on
attachment 154534
[details]
Patch
Attachment 154534
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/13353492
David Barr
Comment 5
2012-07-25 21:53:48 PDT
Created
attachment 154536
[details]
Patch
Tony Chang
Comment 6
2012-07-26 14:21:18 PDT
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.
Simon Fraser (smfr)
Comment 7
2012-07-26 14:24:39 PDT
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).
Tony Chang
Comment 8
2012-09-14 13:56:03 PDT
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?
Gyuyoung Kim
Comment 9
2013-04-10 01:17:25 PDT
If David doesn't continue to work on this bug anymore, I'd like to take this bug over. David, please give your comment.
David Barr
Comment 10
2013-04-10 01:25:13 PDT
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.
Gyuyoung Kim
Comment 11
2013-04-10 01:27:31 PDT
(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.
Gyuyoung Kim
Comment 12
2013-04-10 02:23:02 PDT
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. :)
Gyuyoung Kim
Comment 13
2013-04-10 02:27:59 PDT
(In reply to
comment #12
) don't mind -> you mind.
Build Bot
Comment 14
2013-04-10 02:54:50 PDT
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
Build Bot
Comment 15
2013-04-10 03:15:04 PDT
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
Build Bot
Comment 16
2013-04-10 03:19:22 PDT
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
Gyuyoung Kim
Comment 17
2013-04-11 00:15:18 PDT
Created
attachment 197514
[details]
Patch for EWS
Gyuyoung Kim
Comment 18
2013-04-11 04:23:45 PDT
Comment on
attachment 154536
[details]
Patch Clearing r? because of out of date.
Gyuyoung Kim
Comment 19
2013-04-11 16:57:10 PDT
(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.
Gyuyoung Kim
Comment 20
2013-04-24 22:42:51 PDT
Created
attachment 199619
[details]
Patch
Gyuyoung Kim
Comment 21
2013-04-24 22:43:28 PDT
Rebased.
Build Bot
Comment 22
2013-04-25 01:29:16 PDT
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
Build Bot
Comment 23
2013-04-25 01:29:21 PDT
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
Gyuyoung Kim
Comment 24
2013-07-13 05:27:14 PDT
Created
attachment 206602
[details]
Patch
Gyuyoung Kim
Comment 25
2013-07-13 07:40:53 PDT
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
Gyuyoung Kim
Comment 26
2013-07-18 01:38:07 PDT
Created
attachment 206964
[details]
Updated patch based on
Bug 118454
Gyuyoung Kim
Comment 27
2013-07-18 01:39:18 PDT
I'd like to work detail work on
Bug 91566
. So, this patch should be landed first. Please review.
Gyuyoung Kim
Comment 28
2013-07-27 23:37:15 PDT
CC'ing Beth, would you please review this patch ?
Beth Dakin
Comment 29
2013-07-29 10:25:11 PDT
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?
Beth Dakin
Comment 30
2013-07-29 10:27:15 PDT
(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.
Gyuyoung Kim
Comment 31
2013-07-31 19:28:11 PDT
(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.
Gyuyoung Kim
Comment 32
2013-07-31 19:31:36 PDT
(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.
Gyuyoung Kim
Comment 33
2013-08-01 17:43:14 PDT
Beth, I think we can handle it on new bug. So, I file a
bug 119418
. I will submit a patch to there soon.
Gyuyoung Kim
Comment 34
2013-08-16 02:52:40 PDT
Created
attachment 208899
[details]
Patch
Gyuyoung Kim
Comment 35
2013-08-16 02:57:31 PDT
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.
kov's GTK+ EWS bot
Comment 36
2013-08-16 03:26:04 PDT
Comment on
attachment 208899
[details]
Patch
Attachment 208899
[details]
did not pass gtk-ews (gtk): Output:
http://webkit-queues.appspot.com/results/1471439
Gyuyoung Kim
Comment 37
2013-08-16 08:19:09 PDT
Created
attachment 208926
[details]
Patch
Beth Dakin
Comment 38
2013-08-19 11:55:18 PDT
This patch looks good. Quick question before I r+ though, is there any was to test this?
Tim Horton
Comment 39
2013-08-19 11:59:08 PDT
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?
Gyuyoung Kim
Comment 40
2013-08-19 20:50:17 PDT
(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.
Gyuyoung Kim
Comment 41
2013-08-19 21:32:15 PDT
Created
attachment 209156
[details]
Patch
Gyuyoung Kim
Comment 42
2013-08-19 21:35:58 PDT
(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.
Build Bot
Comment 43
2013-08-20 01:28:36 PDT
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
Build Bot
Comment 44
2013-08-20 01:28:41 PDT
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
Gyuyoung Kim
Comment 45
2013-08-20 01:35:04 PDT
Created
attachment 209167
[details]
Patch
Gyuyoung Kim
Comment 46
2013-08-20 01:39:39 PDT
(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.
WebKit Commit Bot
Comment 47
2013-08-20 16:50:03 PDT
Comment on
attachment 209167
[details]
Patch Clearing flags on attachment: 209167 Committed
r154375
: <
http://trac.webkit.org/changeset/154375
>
WebKit Commit Bot
Comment 48
2013-08-20 16:50:11 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 49
2013-08-21 13:33:58 PDT
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.
Gyuyoung Kim
Comment 50
2013-08-21 18:26:30 PDT
(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.
Antonio Gomes
Comment 51
2013-08-22 08:28:56 PDT
(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 ..
Tim Horton
Comment 52
2013-11-05 14:46:36 PST
This caused
https://bugs.webkit.org/show_bug.cgi?id=123831
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug