Bug 100414 - Flip ImageOrientation coordinate system from lefthanded to righthanded
Summary: Flip ImageOrientation coordinate system from lefthanded to righthanded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nico Weber
URL:
Keywords:
Depends on:
Blocks: 100191
  Show dependency tree / graph
 
Reported: 2012-10-25 14:12 PDT by Nico Weber
Modified: 2012-11-27 13:43 PST (History)
8 users (show)

See Also:

Attachments
Patch (9.96 KB, patch)
2012-10-25 14:14 PDT, Nico Weber 		no flags Details | Formatted Diff | Diff
Patch (10.71 KB, patch)
2012-10-25 14:16 PDT, Nico Weber 		no flags Details | Formatted Diff | Diff
Patch (9.45 KB, patch)
2012-10-25 14:16 PDT, Nico Weber 		no flags Details | Formatted Diff | Diff
Patch (9.79 KB, patch)
2012-10-25 14:29 PDT, Nico Weber 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nico Weber 2012-10-25 14:12:50 PDT 
Flip ImageOrientation coordinate system from lefthanded to righthanded
Comment 1 Nico Weber 2012-10-25 14:14:21 PDT 
Created attachment 170727 [details]
Patch
Comment 2 Nico Weber 2012-10-25 14:15:48 PDT 
This is the promised follow-up patch. As expected, the skia code becomes a bit shorter, the cg code a bit longer. I don't care too much if we check this in or not. WebKit's coordinate system is generally top-left / right-handed, so this would make the world a tiny bit more consistent I guess.
Comment 3 Nico Weber 2012-10-25 14:16:34 PDT 
Created attachment 170728 [details]
Patch
Comment 4 Nico Weber 2012-10-25 14:16:51 PDT 
Created attachment 170729 [details]
Patch
Comment 5 Eric Seidel (no email) 2012-10-25 14:24:24 PDT 
Comment on attachment 170729 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        This is a pure refactoring and has no observable effects.

Ideally you woudl explain teh justification in the ChangeLOg.

Something along the lines of "platform/graphics, noteably GraphicsContext is defined to use a right handed coord system, yet ImageOrientation was an outlier from platform/graphicsin that it used a left-handed coord system (like how CoreGraphics does).  This patch fixes ImageOrientation to match the rest of platform/graphics."

or some such.
Comment 6 Nico Weber 2012-10-25 14:29:23 PDT 
Created attachment 170731 [details]
Patch
Comment 7 Nico Weber 2012-10-25 14:29:40 PDT 
Done.
Comment 8 Eric Seidel (no email) 2012-10-25 14:31:13 PDT 
Consistency is good.  We really should document some of platform/graphics better some day.  http://trac.webkit.org/wiki/LayoutUnit has been helpful to me so many times. I wish we had more documentation like it.
Comment 9 Eric Seidel (no email) 2012-10-25 14:36:38 PDT 
Comment on attachment 170731 [details]
Patch

I'm a fan of consistency.  Thank you for following up on this.
Comment 10 Eric Seidel (no email) 2012-10-25 14:37:07 PDT 
Noel:  Hopefully we're all on the same page now?  I look forward to your comments and reviews when you return.
Comment 11 WebKit Review Bot 2012-10-25 19:18:31 PDT 
Comment on attachment 170731 [details]
Patch

Clearing flags on attachment: 170731

Committed r132555: <http://trac.webkit.org/changeset/132555>
Comment 12 WebKit Review Bot 2012-10-25 19:18:35 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 13 noel gordon 2012-11-26 20:10:59 PST 
(In reply to comment #10)
> Noel:  Hopefully we're all on the same page now?  I look forward to your comments and reviews when you return.

Sorry for the delayed reply, this went under my radar since it was not present in the meta bug.  Let's add it there for posterity.
Comment 14 noel gordon 2012-11-27 13:43:10 PST 
(In reply to comment #10)

I also considered that we should order the orientation values numerically in ImageOrientation.cpp.  I filed bug 101596 about it.