[webkit-reviews] review granted: [Bug 43792] Accelerated transitions do not suspend/resume properly. : [Attachment 68634] Main patch: unify transitions and animations in GraphicsLayer, and pause transitions.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 23 20:20:08 PDT 2010


mitz at webkit.org has granted Simon Fraser (smfr) <simon.fraser at apple.com>'s
request for review:
Bug 43792: Accelerated transitions do not suspend/resume properly.
https://bugs.webkit.org/show_bug.cgi?id=43792

Attachment 68634: Main patch: unify transitions and animations in
GraphicsLayer, and pause transitions.
https://bugs.webkit.org/attachment.cgi?id=68634&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=68634&action=review

> WebCore/platform/graphics/GraphicsLayer.cpp:252
> +    return String::format("-|transition%d-", property);

Can you use %c instead?

> WebCore/platform/graphics/GraphicsLayer.h:295
> +    virtual void removeAnimation(const String& /*animationName*/)  { }

Extra space before the brace.

> WebCore/platform/graphics/mac/GraphicsLayerCA.h:275
> +    void setCAAnimationOnLayer(CAPropertyAnimation*, AnimatedPropertyID
property, const String& animationName, int index, double timeOffset);
> +    bool removeCAAnimationFromLayer(AnimatedPropertyID property, const
String& animationName, int index);
> +    void pauseCAAnimationOnLayer(AnimatedPropertyID property, const String&
animationName, int index, double timeOffset);

I don’t know why you named the AnimatedPropertyID parameter in these
declarations.

> WebCore/platform/graphics/mac/GraphicsLayerCA.h:340
> +	   LayerPropertyAnimation(CAPropertyAnimation* caAnim, const String&
animationName, AnimatedPropertyID property, int index, double timeOffset)

Could rename caAnim to caPropertyAnimation.

> WebCore/platform/graphics/mac/GraphicsLayerCA.mm:549
> +    NSString* animationID = animationIdentifier;

Star on the wrong side.

> WebCore/platform/graphics/mac/GraphicsLayerCA.mm:550
> +    CAAnimation* anim = [fromLayer animationForKey:animationID];

Ditto.

> WebCore/rendering/RenderLayerBacking.cpp:1238
> +// This is used for the 'freeze' api, for testing only.

API


More information about the webkit-reviews mailing list