[webkit-reviews] review granted: [Bug 23147] Introduce Skia to WebKit : [Attachment 26479] Add WebCore/platform/graphics/skia files v2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jan 7 13:08:46 PST 2009
Eric Seidel <eric at webkit.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 23147: Introduce Skia to WebKit
https://bugs.webkit.org/show_bug.cgi?id=23147
Attachment 26479: Add WebCore/platform/graphics/skia files v2
https://bugs.webkit.org/attachment.cgi?id=26479&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
We only name arguments when the name adds clarity:
+ static PassRefPtr<BitmapImageSingleFrameSkia> create(
+ const SkBitmap& bitmap);
This one doesn't need a name, per the style guidelines.
I'm surprised there isn't just a m_nativeImage.size() accessor:
+ return IntSize(m_nativeImage.width(), m_nativeImage.height());
Also, there is no reason for these virtual functions to be in the headers.
They should just move to the .cpp file IMO (they're virtual, so won't be
inlined).
+ virtual void draw(GraphicsContext* ctxt, const FloatRect& dstRect,
+ const FloatRect& srcRect, CompositeOperator
compositeOp);
compositeOp and ctxt don't need to be there.
Where's BitmapImageSingleFrameSkia.cpp? :) Looks fine. Needs the above
changes and can be landed. Marking r+ assuming you'll change before landing.
If you're still sans commit-bit, then please post a new patch.
More information about the webkit-reviews
mailing list