Bug 11433 – Fixes to get WebKit to run on windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Bertrand Guiheneuf]

Bug fixes to get WebKit to compile again on Win32. 
Mostly adaptions to the new loader. 
Also implemented AffineTranform for cairo.

[Bertrand Guiheneuf]

Created attachment 11241 [details]
Patch to get WebKit to compile and run on win32. Requires AffineTransformCairo.cpp too.

[Bertrand Guiheneuf]

Created attachment 11242 [details]
Implementation of AffineTransform for Cairo

[Maciej Stachowiak]

Comment on attachment 11242 [details]
Implementation of AffineTransform for Cairo

Thanks for hte patch!

Mostly this looks good. However, the implementation of mapRect is not correct. Mapping a rect is not equivalent to mapping a point and a distance. Consider the case of a rotated rect. In that case, you need to get the final bounding box accounting for rotation. This is what the CG and Qt implementatios will do.

Also, the patch should include a ChangeLog entry.

Please address these issues and resubmit.

[Bertrand Guiheneuf]

Hmm, yes, you're right, my implementation of rect transformation made no sense. Sorry for that. I'll resubmit later today. 

[Maciej Stachowiak]

I think this patch, unfortunately, has been overtaken by the pace of other changes.

> BrowserExtensionWin.cpp

BrowserExtension is gone now, these changes will need to be made elsewhere.

The other changes look ok. Please update the BrowserExtension changes and make the fix I mentioned to AffineTransformCairo (I think the right way to map a rect is to map each corner and find the bounding box). 

This bug also included an AffineTransformCairo, so you may not need it if that one ends first: http://bugs.webkit.org/show_bug.cgi?id=11405

[Maciej Stachowiak]

Comment on attachment 11241 [details]
Patch to get WebKit to compile and run on win32. Requires AffineTransformCairo.cpp too.

r- for the minor issues above, please fix and resubmit. Also please make sure to include a ChangeLog.

[Bertrand Guiheneuf]

Created attachment 11349 [details]
Adaption of win32 patch to current TOT

* Adapt to recent changes
* Add changelog

[Bertrand Guiheneuf]

Created attachment 11350 [details]
Fix mapRect 

mapRect now returns the enclosing rect of transformed rect.

[Bertrand Guiheneuf]

Created attachment 11351 [details]
Main patch with correct change logs

Moved webkit related change descriptions into WebKit/ChangeLog 
Sorry for the spam.

[Maciej Stachowiak]

Comment on attachment 11350 [details]
Fix mapRect 

It's against the coding style guidelines to write single-line ifs with elses like this:

if (px < enclosingRectMinX) enclosingRectMinX = px;
else if (px > enclosingRectMaxX) enclosingRectMaxX = px;
    
It would probably be simpler to write this by computing all four transformed points and then using std::min and std::max to get the final coordinates, but this way is fine too if you make it follow the style guidelines.

r- for the style issue.

[Bertrand Guiheneuf]

I dont' see anything about this identation style in http://webkit.org/coding/coding-style.html
Please explain.

[Bertrand Guiheneuf]

Created attachment 11372 [details]
AffineTransformCairo.cpp

added the missing CR...

[Maciej Stachowiak]

Comment on attachment 11372 [details]
AffineTransformCairo.cpp

r=me

[Maciej Stachowiak]

Comment on attachment 11351 [details]
Main patch with correct change logs

r=me

[Alexey Proskuryakov]

Committed revision 17592.