[Webkit-unassigned] [Bug 16979] Patch to conditionalize some CG/Cairo support in win32

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 28 23:21:12 PST 2008


http://bugs.webkit.org/show_bug.cgi?id=16979





------- Comment #24 from eric at webkit.org  2008-01-28 23:21 PDT -------
(From update of attachment 18740)
Once you're inside a CG file, there is no need to conditionalize the header
include (see FrameCGWin.cpp).

What's
// FIXME: Ideally we'd have an isPluginView() here but we can't add that to the
open source tree right now.
?

We use static_cast in c++ code instead of c-style casts:
(float)printRect.height()

Unnecessary float-double conversion here:
float printedPagesHeight = 0.0;
0.0f instead.

Spaces after commas:
pages.append(IntRect(0,printedPagesHeight,currPageWidth,currPageHeight));

Again:
IntRect ir((int)fr.x(), (int)fr.y(),(int)fr.width(),(int)fr.height());

If I'm picking on you about existing code that's somehow showing up as + lines,
my apologies... I should be picking on aroben or whatever Apple windows hacker
violated their own published guidelines. :)

We don't generally commit commented out code:
71         /*if (fillColor.hasAlpha() ||
graphicsContext->inTransparencyLayer()) {
 72             // GDI can't handle drawing transparent text.  We have to draw
into a mask.  We draw black text on a white-filled background.
 73             // We also do this when inside transparency layers, since GDI
also can't draw onto a surface with alpha.
 74             graphicsContext->save();
 75             graphicsContext->setFillColor(Color::white);
 76             textDrawingDC =
graphicsContext->getWindowsBitmapContext(textRect);
 77             SetTextColor(hdc, RGB(0, 0, 0));
 78         } else*/

braces are on the same line as the if:
    if (platformData.syntheticOblique())
 143     {

No commented out code:
//#include "FontFallbackList.h"

It looks like someone could come by with a second pass and push more code down
into a shared GraphicsContextCG between the Mac impl and the Win impl.

Our naming style is:
setShouldApplyMacAscentHack(bool)
bool shouldApplyMacAscentHack()
not "get"

In general this patch looks great!

I'll let aroben or darin comment once more, but I think you're just about ready
to go.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list