[Webkit-unassigned] [Bug 48212] SVG FELighting performance issues

[bugzilla-daemon at webkit.org]

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


Dirk Schulze <krit at webkit.org> changed:

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




--- Comment #3 from Dirk Schulze <krit at webkit.org>  2010-10-27 01:02:35 PST ---
(From update of attachment 71991)
View in context: https://bugs.webkit.org/attachment.cgi?id=71991&action=review

Have you measured the performance with ByteArray*? I couldn't see a difference between usigned char* and ByteArray* on feGaussianBlur with shark. Of course you save some address calculations on using unsigned char* (pixels[0] = ... pixels[1]), but would still interest me you see a difference.
The changes on FloatPoint3D are ok.

> WebCore/platform/graphics/filters/FELighting.cpp:95
> +inline void FELighting::LightingData::topRight(int offset, IntPair& intPair)
>  {
> -    return static_cast<int>(pixels->get(offset - widthMultipliedByPixelSize + cPixelSize + cAlphaChannelOffset));
> +    unsigned char* pixel = pixels + offset;
> +    int left = static_cast<int>(pixel[-cPixelSize + cAlphaChannelOffset]);
> +    int center = static_cast<int>(pixel[cAlphaChannelOffset]);
> +    pixel += widthMultipliedByPixelSize;
> +    int bottomLeft = static_cast<int>(pixel[-cPixelSize + cAlphaChannelOffset]);
> +    int bottom = static_cast<int>(pixel[cAlphaChannelOffset]);
> +    intPair.normalX = -(left << 1) + (center << 1) - bottomLeft + bottom;
> +    intPair.normalY = -left - (center << 1) + bottomLeft + (bottom << 1);
>  }

Is there no way to share more of the code with the other edge functions?

> WebCore/platform/graphics/filters/FELighting.cpp:196
> +            if (m_specularExponent == 1.0f)

== 1

> WebCore/platform/graphics/filters/FELighting.cpp:205
> +        normalVector.setZ(1.0f);

just ...setZ(1);

> WebCore/platform/graphics/filters/FELighting.cpp:214
> +            if (m_specularExponent == 1.0f)

== 1

> WebCore/platform/graphics/filters/FELighting.cpp:-128
> -    if (data.lightStrength > 1.0f)
> -        data.lightStrength = 1.0f;
> -    if (data.lightStrength < 0.0f)
> -        data.lightStrength = 0.0f;

1 and 0

> WebCore/platform/graphics/filters/FELighting.cpp:237
> +    IntPair intPair;
> +    int offset;

Please initialize these values. And declare them on the first use.

> WebCore/platform/graphics/filters/FELighting.cpp:258
> +    setPixel(offset, data, paintingData, 0, 0, -2.0f / 3.0f, -2.0f / 3.0f, intPair);

Normally the compiler should optimize it, but can you add a static const for -2 / 3.f ? This is reused multiple times. The same for some other constant calculations, a name is also easier to understand.

> WebCore/platform/graphics/filters/FELighting.cpp:346
>      RefPtr<ImageData> srcImageData(in->resultImage()->getUnmultipliedImageData(effectDrawingRect));

change srcImageData(...) to srcImageData = ... please

> WebCore/platform/graphics/filters/FELighting.h:57
> +    struct IntPair {
> +        int normalX;
> +        int normalY;
> +    };

Do you really see a noticeable performance win compared to IntPoint?

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