[webkit-reviews] review denied: [Bug 12515] Plug-ins that draw through the Quickdraw interface fail in a CGBitmapContex. : [Attachment 13495] Allows QD based plugins to draw into offscreen bitmap contexts. (V5)

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Wed Mar 7 06:42:29 PST 2007


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 12515: Plug-ins that draw through the Quickdraw interface fail in a
CGBitmapContex.
http://bugs.webkit.org/show_bug.cgi?id=12515

Attachment 13495:  Allows QD based plugins to draw into offscreen bitmap
contexts. (V5)
http://bugs.webkit.org/attachment.cgi?id=13495&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This looks good and it is *almost* ready to go!

I looked at what effect this patch will have when the context is *not* a bitmap
context, because that's the part that could cause regressions.

And, I'm not trying to be nitpicky but ...

The patch moves the SetPort, PenNormal, ForeColor, BackColor, SetOrigin, and
SetPortClipRegion calls inside the "if (forUpdate)" if statement.

The ChangeLog doesn't say why that change is correct. Was it a mistake that
those functions were called in the "forUpdate is false" case before? If so, was
it simply inefficient to do the extra work, or was there actually a bug?

Another less important question. Is SetGWorld on the saved port from GetPort
really the best practice for restoring the old port? Or do we need to do a
GetGWorld rather than a GetPort to get the oldPort in the first place?

Otherwise looks great.

review- because the patch should contain the change to the ChangeLog (as Mark
said in his comment) but mainly because of the forUpdate change/issue above.



More information about the webkit-reviews mailing list