Bug 100414

Summary:

Flip ImageOrientation coordinate system from lefthanded to righthanded

Product:

WebKit

Reporter:

Nico Weber <thakis>

Component:

New Bugs

Assignee:

Nico Weber <thakis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eae, eric, leviw, noel.gordon, senorblanco, simon.fraser, thorton, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

100191

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Nico Weber

Reported 2012-10-25 14:12:50 PDT

Flip ImageOrientation coordinate system from lefthanded to righthanded

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 attachment proposed patch, testcase, etc.

Nico Weber

Comment 1 2012-10-25 14:14:21 PDT

Created attachment 170727 [details] Patch

Nico Weber

Comment 2 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.

Nico Weber

Comment 3 2012-10-25 14:16:34 PDT

Created attachment 170728 [details] Patch

Nico Weber

Comment 4 2012-10-25 14:16:51 PDT

Created attachment 170729 [details] Patch

Eric Seidel (no email)

Comment 5 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.

Nico Weber

Comment 6 2012-10-25 14:29:23 PDT

Created attachment 170731 [details] Patch

Nico Weber

Comment 7 2012-10-25 14:29:40 PDT

Done.

Eric Seidel (no email)

Comment 8 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.

Eric Seidel (no email)

Comment 9 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.

Eric Seidel (no email)

Comment 10 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.

WebKit Review Bot

Comment 11 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>

WebKit Review Bot

Comment 12 2012-10-25 19:18:35 PDT

All reviewed patches have been landed. Closing bug.

noel gordon

Comment 13 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.

noel gordon

Comment 14 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.

Note You need to log in before you can comment on or make changes to this bug.