Bug 17442 - Correct Windows Cairo (GraphicsContext) implementation
Summary: Correct Windows Cairo (GraphicsContext) implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-19 22:24 PST by Brent Fulgham
Modified: 2008-02-23 23:27 PST (History)
2 users (show)

See Also:


Attachments
Initial patch based on discussions with Alp Toker (13.21 KB, patch)
2008-02-20 13:38 PST, Brent Fulgham
aroben: review+
Details | Formatted Diff | Diff
Updated patch with missing non-Windows stanza (13.44 KB, patch)
2008-02-22 23:53 PST, Brent Fulgham
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2008-02-19 22:24:51 PST
My initial Cairo implementation on Windows had a few mistakes:

1.  It improperly put some "GraphicsContextPlatformPrivate" code in the general Windows implementation.
2.  The higher-level calls need to call the platform-private windows calls that keep the WorldTransform in sync.
3.  The concatCTM method didn't even do anything with the passed transform -- a clear bug!
4.  SVG does not rotate or scale properly in the current implementation.
Comment 1 Brent Fulgham 2008-02-20 13:38:31 PST
Created attachment 19237 [details]
Initial patch based on discussions with Alp Toker
Comment 2 Adam Roben (:aroben) 2008-02-22 09:52:26 PST
Comment on attachment 19237 [details]
Initial patch based on discussions with Alp Toker

Looks like you have some strange indenting in the ChangeLog.

r=me
Comment 3 Matt Lilek 2008-02-22 16:25:23 PST
Committed revision 30500.

Comment 4 Mark Rowe (bdash) 2008-02-22 20:17:16 PST
This patch completely broke the non-Windows case.  Unconditional calls such as:

m_data->save();

are using method which are declared:

#if PLATFORM(WIN)
     // On Windows, we need to update the HDC for form controls to draw in the right place.
     void save();


I'm going to roll this out so it can be addressed.
Comment 5 Mark Rowe (bdash) 2008-02-22 20:20:06 PST
Rolled out in r30500.
Comment 6 Brent Fulgham 2008-02-22 22:25:24 PST
It looks like I screwed up the patch by omitting the non-Windows case.  I have a building GTK Webkit on a Linux box, which I will use to test this patch again.
Comment 7 Brent Fulgham 2008-02-22 23:53:46 PST
Created attachment 19287 [details]
Updated patch with missing non-Windows stanza
Comment 8 Brent Fulgham 2008-02-22 23:54:25 PST
Yes, I agree that the #if/def is perhaps clunky, but it's the approach used in the CoreGraphics implementations on Mac and WIndows.

I have now built and run this patch on Windows, Mac, and GTK+.  Current patch was generated against today's SVN head.
Comment 9 Darin Adler 2008-02-23 09:32:33 PST
Comment on attachment 19287 [details]
Updated patch with missing non-Windows stanza

ChangeLog has some tabs in it. When working on WebKit patches please make sure your editor is set in a mode that won't add tabs.

r=me
Comment 10 Matt Lilek 2008-02-23 20:16:06 PST
Committed revision 30533.
Comment 11 Matt Lilek 2008-02-23 20:17:13 PST
(In reply to comment #5)
> Rolled out in r30500.
> 

It was actually rolled out in r30513.
Comment 12 Robert Blaut 2008-02-23 23:14:18 PST
(In reply to comment #11)
> (In reply to comment #5)
> > Rolled out in r30500.
> > 
> 
> It was actually rolled out in r30513.
> 

So why the bug is resolved as FIXED?
Comment 13 Matt Lilek 2008-02-23 23:27:38 PST
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #5)
> > > Rolled out in r30500.
> > > 
> > 
> > It was actually rolled out in r30513.
> > 
> 
> So why the bug is resolved as FIXED?
> 

Because I recommitted in r30533 (comment 10) after Brent tweaked it.  I just happened to see that comment and mentioned it for the sake of correctness.