[webkit-reviews] review denied: [Bug 23147] Introduce Skia to WebKit : [Attachment 26472] Add WebCore/svg/graphics/skia files

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jan 6 14:18:26 PST 2009


Eric Seidel <eric at webkit.org> has denied 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 26472: Add WebCore/svg/graphics/skia files
https://bugs.webkit.org/attachment.cgi?id=26472&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
We don't use SVGPaintServerGradientSkia anymore.  GradientSkia.cpp should be
taking care of this.  There is a x-platform implementation of
SVGPaintServerGradient which uses Gradient from platform instead.

SVGPaintServerPatternSkia is slated to die (the same death as
SVGPaintServerGradient*, but it looks like that patch hasn't landed yet).

I'm surprised we don't have a nicer way than this:
+	 IntRect textBoundary =
const_cast<RenderObject*>(object)->absoluteBoundingBoxRect();
+	 targetRect =
object->absoluteTransform().inverse().mapRect(textBoundary);
it seems CG does the same thing though. :(

We use 0 instead of NULL for c++ code according to the style guidelines:
+    context->platformContext()->setGradient(NULL);

We don't generally commit ifdef'd out code, so this should be removed:
+  // TODO(jhaas): implement me
+#if 0
+    cairo_t* cr = context->platformContext();
+    cairo_surface_t* surface = mask()->surface();
+    if (!surface)
+	 return;
+    cairo_pattern_t* mask = cairo_pattern_create_for_surface(surface);
+    cairo_mask(cr, mask);
+    cairo_pattern_destroy(mask);
+#endif

Otherwise it looks fine.  I think you can delete the
SVGPaintServerGradientSkia* files from the build and it should still work.


More information about the webkit-reviews mailing list