[Webkit-unassigned] [Bug 12515] Plug-ins that draw through the Quickdraw interface fail in a CGBitmapContex.

[bugzilla-daemon at webkit.org]

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


sam at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12836|review?                     |review-
               Flag|                            |




------- Comment #2 from sam at webkit.org  2007-01-31 19:28 PDT -------
(From update of attachment 12836)
A couple of style issues with the patch.

We usually #define these to be equal to '1', not 'true'
+#define ALLOW_QUICKDRAW_OFFSCREEN_DRAWING true

There shouldn't be spaces around cgByteOrder and the brace should go on the
same line
+    switch ( cgByteOrder )
+    {

Spaces are needed around the =, and the extra newline seems unneeded
+        case kCGBitmapByteOrder16Little:
+            format= k16LE555PixelFormat;
+            break;
+        case kCGBitmapByteOrder32Little:
+            format= k32BGRAPixelFormat;
+            break;
+
+        case kCGBitmapByteOrder16Big:
+            format = k16BE555PixelFormat;
+            break;
+        case kCGBitmapByteOrder32Big:
+            format= k32ARGBPixelFormat;
+            break;

Please use ASSERT_NOT_REACHED() instead of ASSERT(0) and the ending brace is
indented too far.
+        default:
+            ASSERT(0); // should not be here.
+        }

You might want to consider not using the local variable 'format' at all just
return from inside the switch.


Spaces inside the braces here
+enum ContextType_priv {kCGBitmapContextType = 4};

There are two spaces between the words 'struct' and 'CGContext_priv'.  Please
put the * next to the word 'void'.
+struct  CGContext_priv {
+    void * s1;

We tend to just use 'unsigned' in these cases.
+    unsigned int s4;

Please remove or explain this comment
+    // blah blah blah

Please use a static_cast<> here.  There is also extra whitespace inside the
parenthesis.
+    CGContext_priv* context = (CGContext_priv*) contextRef;
+    return ( context->type == kCGBitmapContextType);

Are all the newlines necessary? 
+
+#endif
+
+
+
+

Braces should be on the same line.
+            if (IsBitmapContext(currentContext))
+            {

Two spaces are being used again here.
+                Rect  offscreenBounds;
+                int rowBytes =  CGBitmapContextGetBytesPerRow(currentContext);
+                offscreenBounds.top =  0;
+                offscreenBounds.left =  0;
+                offscreenBounds.right = 
CGBitmapContextGetWidth(currentContext);

The brace should be on the same line, the space on the right of the 0 should be
removed, and you can change this to if (!pixelFormat) {.
+                if (pixelFormat == 0 )
+                {

Split on to multiple lines.
+                    offscreenBounds.top = offscreenBounds.left =
offscreenBounds.right = offscreenBounds.bottom = 0;

Need spaces on right of commas.
+                QDErr err = NewGWorldFromPtr(&pOffScreenGWorld,
pixelFormat,&offscreenBounds,0,0, 0, (char*)bits, rowBytes);

Brace should be on the same line.
+                if (!err)
+                {

Same here.
+            }
+            else

and here
+            if (IsBitmapContext(currentContext))
+            {

You seem to just be adding extra whitespace here, please revert it.
-        [self setWindowIfNecessary];
+        [self setWindowIfNecessary];  

and here
-    if (drawingModel == NPDrawingModelQuickDraw && !isTransparent &&
event->what == updateEvt) {
+    if (drawingModel == NPDrawingModelQuickDraw && !isTransparent  &&
event->what == updateEvt) {

Finally, one last brace should be on the same line
+            if (newWebView)
+            {

There are also a couple places where you added trailing whitespace, but that is
just a person pet peeve :)

Marking r-.


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