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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 12 15:22:12 PST 2009


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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43089|review?                     |review+
               Flag|                            |




--- Comment #9 from Simon Fraser (smfr) <simon.fraser at apple.com>  2009-11-12 15:22:12 PST ---
(From update of attachment 43089)
> Index: WebCore/GNUmakefile.am
> ===================================================================
> --- WebCore/GNUmakefile.am	(revision 50863)
> +++ WebCore/GNUmakefile.am	(working copy)
> @@ -2625,6 +2625,8 @@ webcore_sources += \
>  	WebCore/platform/graphics/filters/Filter.h \
>  	WebCore/platform/graphics/filters/FilterEffect.cpp \
>  	WebCore/platform/graphics/filters/FilterEffect.h \
> +	WebCore/platform/graphics/filters/GraphicsContextFilter.cpp \
> +	WebCore/platform/graphics/filters/GraphicsContextFilter.h \

Stale file names.

Should you add ImageBufferFilter to the xcode project as well? It seems
generally useful.

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


> +        Color fillColor = colorWithOverrideAlpha(context->fillColor().rgb(), context->fillColor().alpha() / 255.f * gcp->state.globalAlpha);

If alpha() returns a float no need to make the 255 literal a float; the
compiler will promote it.

> +static inline void drawPathShadow(GraphicsContext* context, GraphicsContextPrivate* gcp, bool fillShadow, bool strokeShadow)
> +{
> +#if ENABLE(FILTERS)
> +    IntSize shadowSize;
> +    int shadowBlur;
> +    Color shadowColor;
> +    if (context->getShadow(shadowSize, shadowBlur, shadowColor)) {
> +        //calculate filter values
> +        cairo_t* cr = context->platformContext();
> +        cairo_path_t* path = cairo_copy_path(cr);
> +        double x0, x1, y0, y1;
> +        cairo_stroke_extents(cr, &x0, &y0, &x1, &y1);
> +        FloatRect rect(x0, y0, x1 - x0, y1 - y0);
> +
> +        IntSize shadowBufferSize;
> +        FloatRect shadowRect;
> +        float kernelSize (0.0);
> +        GraphicsContext::calculateShadowBufferDimensions(shadowBufferSize, shadowRect, kernelSize, rect, shadowSize, shadowBlur);
> +
> +        // create suitably-sized ImageBuffer to hold the shadow
> +        OwnPtr<ImageBuffer> shadowBuffer = ImageBuffer::create(shadowBufferSize);
> +
> +        //draw shadow into a new ImageBuffer
> +        cairo_t* shadowContext = shadowBuffer->context()->platformContext();
> +        copyContextProperties(cr, shadowContext);
> +        cairo_translate(shadowContext, -rect.x() + kernelSize, -rect.y() + kernelSize);
> +        cairo_new_path(shadowContext);
> +        cairo_append_path(shadowContext, path);
> +
> +        if (fillShadow)
> +            setPlatformFill(context, shadowContext, gcp);
> +        if (strokeShadow)
> +            setPlatformStroke(context, shadowContext, gcp);
> +
> +        context->createPlatformShadow(shadowBuffer.release(), shadowColor, shadowRect, kernelSize);
> +    }

This would be cleaner with an early return.

> -void GraphicsContext::setPlatformShadow(IntSize const&, int, Color const&, ColorSpace)
> +void GraphicsContext::setPlatformShadow(IntSize const& size, int, Color const&, ColorSpace)
>  {
> -    notImplemented();
> +    // Cairo doesn't support shadows natively, they are drawn manually in the draw*
> +    // functions
> +
> +    if (m_common->state.shadowsIgnoreTransforms) {
> +        // Meaning that this graphics context is associated with a CanvasRenderingContext

For now, but what if this changes? We may decide that shadows ignore CSS
transforms at some point.

> Index: WebCore/platform/graphics/cairo/ImageCairo.cpp
> ===================================================================
> --- WebCore/platform/graphics/cairo/ImageCairo.cpp	(revision 50863)
> +++ WebCore/platform/graphics/cairo/ImageCairo.cpp	(working copy)
> @@ -132,6 +132,30 @@ void BitmapImage::draw(GraphicsContext* 
>      cairo_matrix_t matrix = { scaleX, 0, 0, scaleY, srcRect.x(), srcRect.y() };
>      cairo_pattern_set_matrix(pattern, &matrix);
>  
> +    // Draw the shadow
> +#if ENABLE(FILTERS)
> +    IntSize shadowSize;
> +    int shadowBlur;
> +    Color shadowColor;
> +    if (context->getShadow(shadowSize, shadowBlur, shadowColor)) {
> +        IntSize shadowBufferSize;
> +        FloatRect shadowRect;
> +        float kernelSize (0.0);
> +        GraphicsContext::calculateShadowBufferDimensions(shadowBufferSize, shadowRect, kernelSize, dstRect, shadowSize, shadowBlur);
> +        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);
> +    }
> +#endif
> +
>      // Draw the image.
>      cairo_translate(cr, dstRect.x(), dstRect.y());
>      cairo_set_source(cr, pattern);
> Index: WebCore/platform/graphics/filters/Filter.h
> ===================================================================

>          // SVG specific
> +        virtual void calculateEffectSubRegion(FilterEffect*) const = 0;

It's not clear why this can be const. Does it change state in |this|, or the
filter effect?

> +        virtual bool effectBoundingBoxMode() const = 0;

I can't figure out what this means from the signature. Bool return? Not your
fault, but it seems like this needs an enum for the return value.

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

> +namespace WebCore {
> +
> +    class ImageBufferFilter : public Filter {

We don't generally indent for namespaces.

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


More information about the webkit-unassigned mailing list