[Webkit-unassigned] [Bug 28362] SVG Filter feComposite implementation is missing

[bugzilla-daemon at webkit.org]

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


Nikolas Zimmermann <zimmermann at kde.org> changed:

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




--- Comment #2 from Nikolas Zimmermann <zimmermann at kde.org>  2009-08-18 11:52:41 PDT ---
(From update of attachment 34934)
r- for various issues:


> +#if !PLATFORM(CG) && !PLATFORM(SKIA)
> +void inline operatorIn(const PassRefPtr<CanvasPixelArray>& srcPixelArrayA, PassRefPtr<CanvasPixelArray>& srcPixelArrayB)
s/void inline/inline void/
Pass CanvasPixelArray pointers here.

>  {
> +    for (unsigned pixelOffset = 0; pixelOffset < srcPixelArrayA->length(); pixelOffset++) {
s/pixelOffset++/++pixelOffset/

> +        unsigned pixelByteOffset = pixelOffset * 4;
> +        unsigned char alphaA = srcPixelArrayA->get(pixelByteOffset + 3);
> +        unsigned char alphaB = srcPixelArrayB->get(pixelByteOffset + 3);
> +
> +        unsigned char resultB = (alphaA * alphaB) / 255;
> +        srcPixelArrayB->set(pixelByteOffset + 3, resultB);
> +    }
> +}
> +#endif

It should be made more clear why CG/Skia support clipToImageBuffer, where the
rest doesn't.

> + void inline arithmetic(const PassRefPtr<CanvasPixelArray>& srcPixelArrayA, PassRefPtr<CanvasPixelArray>& srcPixelArrayB,
s/void inline/inline void/
Pass CanvasPixelArray pointers here.

> +    for (unsigned pixelOffset = 0; pixelOffset < srcPixelArrayA->length(); pixelOffset++) {
s/pixelOffset++/++pixelOffset/

> +        unsigned pixelByteOffset = pixelOffset * 4;
> +        for (unsigned channel = 0; channel < 4; channel++) {
s/channel++/++channel/

> +#if PLATFORM(CG) || PLATFORM(SKIA)
> +                filterContext->save();
> +                filterContext->clipToImageBuffer(calculateDrawingRect(m_in2->subRegion()), m_in2->resultImage());
> +                filterContext->drawImage(m_in->resultImage()->image(), calculateDrawingRect(m_in->subRegion()));
> +                filterContext->restore();
> +#else
> +                IntRect effectADrawingRect = calculateDrawingIntRect(m_in2->subRegion());
> +                PassRefPtr<CanvasPixelArray> srcPixelArrayA(m_in2->resultImage()->getPremultipliedImageData(effectADrawingRect)->data());
> +
> +                IntRect effectBDrawingRect = calculateDrawingIntRect(m_in->subRegion());
> +                PassRefPtr<ImageData> imageData(m_in->resultImage()->getPremultipliedImageData(effectBDrawingRect));
> +                PassRefPtr<CanvasPixelArray> srcPixelArrayB(imageData->data());
> +
> +                operatorIn(srcPixelArrayA, srcPixelArrayB);
> +                resultImage()->putPremultipliedImageData(imageData.get(), IntRect(IntPoint(), resultImage()->size()), IntPoint());
> +#endif

This code should be encapsulated in a helper function, to avoid spreading
CG/Skia defines.
Don't use "PassRefPtr<CanvasPixelArray> = ...". As the name indicates, it
should only be used when passing around RefPtrs, which is not the case here.
Also imageData->data() returns a plain pointer, no need for you to ref/deref
it.

There's also a style issue in the FECOMPOSITE_OPERATOR_ARITHMETIC case, you're
indenting too much there.

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