Bug 43792 - Accelerated transitions do not suspend/resume properly.
Summary: Accelerated transitions do not suspend/resume properly.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 43733
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-10 08:08 PDT by Chris Marrin
Modified: 2010-09-24 19:52 PDT (History)
2 users (show)

See Also:


Attachments
Partial patch. Transitions pause and resume, but sometimes at end of transition, flashes back to start for an instant (14.00 KB, patch)
2010-09-09 16:00 PDT, Chris Marrin
no flags Details | Formatted Diff | Diff
Main patch: unify transitions and animations in GraphicsLayer, and pause transitions. (53.50 KB, patch)
2010-09-23 18:09 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (18.27 KB, patch)
2010-09-24 17:58 PDT, Simon Fraser (smfr)
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2010-08-10 08:08:30 PDT
If you call AnimationController::suspendAnimations() while an accelerated transition is running, it does not suspend. When you call resumeAnimations() after the transition has finished, it will perform the transition again from the point at which suspend was called and then snap back to the start state briefly at the end.

This can be tested when https://bugs.webkit.org/show_bug.cgi?id=43733 is landed

rdar://problem/7616086
Comment 1 Chris Marrin 2010-09-09 16:00:59 PDT
Created attachment 67108 [details]
Partial patch. Transitions pause and resume, but sometimes at end of transition, flashes back to start for an instant

This patch is just a checkpoint. The basic bug is fixed, but there is sometimes a flash back to the start frame at the end of the transition.
Comment 2 Simon Fraser (smfr) 2010-09-23 18:09:42 PDT
Created attachment 68634 [details]
Main patch: unify transitions and animations in GraphicsLayer, and pause transitions.
Comment 3 Simon Fraser (smfr) 2010-09-23 18:10:37 PDT
Comment on attachment 68634 [details]
Main patch: unify transitions and animations in GraphicsLayer, and pause transitions.

As Chris noted, with these changes, unpaused transitions can jump at the end. I'll fix that in a followup.
Comment 4 mitz 2010-09-23 20:20:09 PDT
Comment on attachment 68634 [details]
Main patch: unify transitions and animations in GraphicsLayer, and pause transitions.

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
Comment 5 Simon Fraser (smfr) 2010-09-23 21:17:58 PDT
Comment on attachment 68634 [details]
Main patch: unify transitions and animations in GraphicsLayer, and pause transitions.

http://trac.webkit.org/changeset/68233

Keeping bug open to fix flash at end of resumed transition.
Comment 6 Simon Fraser (smfr) 2010-09-24 17:58:21 PDT
Created attachment 68795 [details]
Patch
Comment 7 Simon Fraser (smfr) 2010-09-24 19:52:31 PDT
http://trac.webkit.org/changeset/68323