[webkit-reviews] review granted: [Bug 78155] Implement hardware animation of CSS filters : [Attachment 126393] Patch with review changes, crash fixes and tests for more cases

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 9 17:10:01 PST 2012


Dean Jackson <dino at apple.com> has granted Chris Marrin <cmarrin at apple.com>'s
request for review:
Bug 78155: Implement hardware animation of CSS filters
https://bugs.webkit.org/show_bug.cgi?id=78155

Attachment 126393: Patch with review changes, crash fixes and tests for more
cases
https://bugs.webkit.org/attachment.cgi?id=126393&action=review

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=126393&action=review


very minor change requests. awesome!

> Source/WebCore/ChangeLog:17
> +	   Reviewed by NOBODY (OOPS!).

Needs newline.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1906
> +    if (listIndex < 0)
> +	   return false;
> +	   
> +    const FilterOperations* operations = (listIndex >= 0) ?
static_cast<const FilterAnimationValue*>(valueList.at(listIndex))->value() : 0;


No longer need the ternary operation.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2220
> +    // We toss the last tfArray value because it has to one shorter than the
others.

Nits: might as well say "timing function" rather than "tfArray value". And "has
to *be* one".

> Source/WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.mm:441
> +	   double t = 0;

Nit: for consistency, you use amount above and below, but t here.


More information about the webkit-reviews mailing list