[webkit-reviews] review denied: [Bug 49907] Better result passing in filter primitives : [Attachment 75679] patch 3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 6 22:47:29 PST 2010


Dirk Schulze <krit at webkit.org> has denied Zoltan Herczeg
<zherczeg at webkit.org>'s request for review:
Bug 49907: Better result passing in filter primitives
https://bugs.webkit.org/show_bug.cgi?id=49907

Attachment 75679: patch 3
https://bugs.webkit.org/attachment.cgi?id=75679&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75679&action=review

The patch looks great, still some notes from me.

> WebCore/platform/graphics/filters/FEBlend.cpp:92
> +    if (hasResult())
> +	   return;

You should mention this in the changelog, that you do not create the same
filter effect result twice.

> WebCore/platform/graphics/filters/FETurbulence.cpp:329
> +    if (!absolutePaintRect().width() || !absolutePaintRect().height())

do we allow negative width or height? If not you could use isEmpty() here.

> WebCore/platform/graphics/filters/FilterEffect.cpp:130
> +    int xOrigin = rect.x();
> +    int xDest = 0;
> +    if (xOrigin < 0) {
> +	   xDest = -xOrigin;
> +	   xOrigin = 0;
> +    }
> +    int xEnd = rect.right();
> +    if (xEnd > m_absolutePaintRect.width())
> +	   xEnd = m_absolutePaintRect.width();
> +
> +    int yOrigin = rect.y();
> +    int yDest = 0;
> +    if (yOrigin < 0) {
> +	   yDest = -yOrigin;
> +	   yOrigin = 0;
> +    }
> +    int yEnd = rect.bottom();
> +    if (yEnd > m_absolutePaintRect.height())
> +	   yEnd = m_absolutePaintRect.height();

I wounder if we could use the intersect functionality of FloatRect instead
doing it ourself here.


More information about the webkit-reviews mailing list