Summary: | [Qt] Animations jump when the page is suspended | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jocelyn Turcotte <jturcotte> | ||||||
Component: | Layout and Rendering | Assignee: | Noam Rosenthal <noam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | jturcotte, noam, ossy, rafael.lobo, webkit.review.bot | ||||||
Priority: | P3 | Keywords: | Qt | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 100769 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Jocelyn Turcotte
2012-10-29 07:52:03 PDT
Are you looking at this, or should I? I didn't investigate yet, I'll do if you don't. I'll take a look today. Created attachment 171318 [details]
Patch
Comment on attachment 171318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171318&action=review I have a few comments. :-) > Source/WebCore/platform/graphics/GraphicsLayerAnimation.cpp:265 > +void GraphicsLayerAnimation::pause(double pauseTime) I think domTime/documentTime would make more clear what kind of time unit we expect here. Isn't "pauseTime" is already implicit? > Source/WebCore/platform/graphics/GraphicsLayerAnimation.cpp:267 > // FIXME: should apply offset here. Do we need this comment? > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:811 > + m_lastAnimationStartTime = WTF::currentTime() - timeOffset; Is timeOffset the starting time for WebProcess? I still have to trouble to follow the logic of those offsets. :-( Meh, sorry for all the typos on last message. :-( Created attachment 171336 [details]
Patch
(In reply to comment #5) > I think domTime/documentTime would make more clear what kind of time unit we expect here. Isn't "pauseTime" is already implicit? Yes, it is :) > > Source/WebCore/platform/graphics/GraphicsLayerAnimation.cpp:267 > > // FIXME: should apply offset here. > > Do we need this comment? No, we don't :) > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:811 > > + m_lastAnimationStartTime = WTF::currentTime() - timeOffset; > > Is timeOffset the starting time for WebProcess? I still have to trouble to follow the logic of those offsets. :-( These offsets are for animation delay. For some reason we receive them here as "negative delay" - it's the actual delay, but that time has already passed. I renamed it to delayAsNegativeTimeOffset, any better suggestion? > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:811
> > > + m_lastAnimationStartTime = WTF::currentTime() - timeOffset;
> >
> > Is timeOffset the starting time for WebProcess? I still have to trouble to follow the logic of those offsets. :-(
>
> These offsets are for animation delay. For some reason we receive them here as "negative delay" - it's the actual delay, but that time has already passed. I renamed it to delayAsNegativeTimeOffset, any better suggestion?
Hmm, I think your suggestion sounds awesome! :-D
Comment on attachment 171336 [details] Patch Clearing flags on attachment: 171336 Committed r132907: <http://trac.webkit.org/changeset/132907> All reviewed patches have been landed. Closing bug. (In reply to comment #10) > (From update of attachment 171336 [details]) > Clearing flags on attachment: 171336 > > Committed r132907: <http://trac.webkit.org/changeset/132907> It caused a regression - https://bugs.webkit.org/show_bug.cgi?id=100769 Could you check it please? |