[Webkit-unassigned] [Bug 30960] [CAIRO] shadow support for Canvas and SVG

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30960


Simon Fraser (smfr) <simon.fraser at apple.com> changed:

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




--- Comment #5 from Simon Fraser (smfr) <simon.fraser at apple.com>  2009-11-06 13:04:03 PDT ---
(From update of attachment 42227)
> Index: WebCore/platform/graphics/GraphicsContext.h
> ===================================================================
> +        void createPlatformShadow(PassOwnPtr<ImageBuffer> buffer, Color shadowColor, FloatRect shadowRect, float kernelSize);

You should pass Color and FloatRect as const references.

> Index: WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp
> ===================================================================

> +static inline void drawPathShadow(GraphicsContext* context, GraphicsContextPrivate* gcp, bool fillShadow, bool strokeShadow)
> +{
> +
> +        // calculate the kernel size according to the HTML5 specification

Huh? HTML5 has something to say about filters? Oh, canvas. You should say
canvas (but this isn't just called for canvas).


> Index: WebCore/platform/graphics/cairo/ImageCairo.cpp
> ===================================================================

> +    // Draw the shadow
> +#if ENABLE(FILTERS)
> +    IntSize shadowSize;
> +    int shadowBlur;
> +    Color shadowColor;
> +    if (context->getShadow(shadowSize, shadowBlur, shadowColor)) {
> +        // calculate the kernel size according to the HTML5 specification
> +        float kernelSize = (shadowBlur < 8 ? shadowBlur / 2.f : sqrt(shadowBlur * 2.f));
> +        int blurRadius = ceil(kernelSize);
> +        IntSize shadowBufferSize(dstRect.width() + blurRadius * 2, dstRect.height() + blurRadius * 2);
> +        FloatRect shadowRect = FloatRect(dstRect.location(), shadowBufferSize);
> +        shadowRect.move(shadowSize.width() - kernelSize, shadowSize.height() - kernelSize);
> +        shadowColor = colorWithOverrideAlpha(shadowColor.rgb(), (shadowColor.alpha() *  context->getAlpha()) / 255.f);
> +
> +        //draw shadow into a new ImageBuffer
> +        OwnPtr<ImageBuffer> shadowBuffer = ImageBuffer::create(shadowBufferSize);
> +        cairo_t* shadowContext = shadowBuffer->context()->platformContext();
> +        cairo_set_source(shadowContext, pattern);
> +        cairo_translate(shadowContext, -dstRect.x(), -dstRect.y());
> +        cairo_rectangle(shadowContext, 0, 0, dstRect.width(), dstRect.height());
> +        cairo_fill(shadowContext);
> +
> +        context->createPlatformShadow(shadowBuffer.release(), shadowColor, shadowRect, kernelSize);

Can you share this code?

> Index: WebCore/platform/graphics/filters/GraphicsContextFilter.h
> ===================================================================

> +    class GraphicsContextFilter : public Filter {

The name of this class is confusing. There's an SVGFilter, but that ultimately
renders into a GraphicsContext. What is special about this one? Should this
instead be some kind of helper class that makes it easy to apply some filter to
drawing into a GraphicsContext?

> +    public:
> +        static PassRefPtr<GraphicsContextFilter> create();
> +
> +        bool effectBoundingBoxMode() { return false; }

This method should be const, and I prefer to see 'virtual' in all subclasses.

> +        void calculateEffectSubRegion(FilterEffect*) { }
> +        FloatRect filterRegion() { return FloatRect(); }

This should be const (in the base class and all subclasses).

> +        FloatRect sourceImageRect() { return FloatRect(); }

This should be const (in the base class and all subclasses).

It looks OK, but I'd like to hear some thoughts about GraphicsContextFilter.

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