Bug 36219

Summary: [Qt] GraphicsLayer: Pausing and resuming of animations don't work as expected
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: Layout and RenderingAssignee: Noam Rosenthal <noam>
Status: CLOSED FIXED    
Severity: Normal CC: commit-queue, hausmann, kenneth
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 36191    
Attachments:
Description Flags
Fix for how GraphicsLayer deals with animation states
none
Fix for how GraphicsLayer deals with animation states none

Noam Rosenthal
Reported 2010-03-17 07:09:26 PDT
LayoutTests/animations/play-state.html fails because of this bug.
Attachments
Fix for how GraphicsLayer deals with animation states (6.90 KB, patch)
2010-03-17 09:28 PDT, Kim Grönholm 		no flags
DetailsFormatted DiffDiff
Fix for how GraphicsLayer deals with animation states (6.94 KB, patch)
2010-03-18 06:28 PDT, Kim Grönholm 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kim Grönholm
Comment 1 2010-03-17 09:28:21 PDT
Created attachment 50915 [details] Fix for how GraphicsLayer deals with animation states
Noam Rosenthal
Comment 2 2010-03-17 09:29:55 PDT
LGTM (we worked on this together :))
Kim Grönholm
Comment 3 2010-03-18 06:28:59 PDT
Created attachment 51021 [details] Fix for how GraphicsLayer deals with animation states Updated the patch because it caused a compiler warning.
Simon Hausmann
Comment 4 2010-03-18 07:27:07 PDT
How can this patch fix a layout test if this code isn't even executed during the DRT run? :) Note that DumpRenderTree doesn't use QGraphicsWebView but QWebView, so the accelerated compositing code path isn't taken AFAICS.
Noam Rosenthal
Comment 5 2010-03-18 07:30:50 PDT
We tested that layout-test manually. It's one of the tests that failed originally when we enabled compositing by default, which raises the question, how did the tests fail in the first place if DRT isn't called? :) in any case, it does fix the layout-test even if it's run manually and not through DRT.
Kenneth Rohde Christiansen
Comment 6 2010-03-18 07:32:26 PDT
(In reply to comment #5) > We tested that layout-test manually. > It's one of the tests that failed originally when we enabled compositing by > default, which raises the question, how did the tests fail in the first place > if DRT isn't called? :) > > in any case, it does fix the layout-test even if it's run manually and not > through DRT. That is actually a very good point! Strange that enabling AC by default broke 3 tests. I think it is about time we change DRT to use the QGraphicsWebView.
Simon Hausmann
Comment 7 2010-03-18 07:52:52 PDT
(In reply to comment #5) > We tested that layout-test manually. > It's one of the tests that failed originally when we enabled compositing by > default, which raises the question, how did the tests fail in the first place > if DRT isn't called? :) It sounds like there's a side-effect somewhere... > in any case, it does fix the layout-test even if it's run manually and not > through DRT. Okay, sounds good. Just wanted to check :)
Simon Hausmann
Comment 8 2010-03-18 07:55:52 PDT
(In reply to comment #6) > (In reply to comment #5) > > We tested that layout-test manually. > > It's one of the tests that failed originally when we enabled compositing by > > default, which raises the question, how did the tests fail in the first place > > if DRT isn't called? :) > > > > in any case, it does fix the layout-test even if it's run manually and not > > through DRT. > > That is actually a very good point! Strange that enabling AC by default broke 3 > tests. > > I think it is about time we change DRT to use the QGraphicsWebView. We should do that at the same time as changing QtLauncher's default then.
WebKit Commit Bot
Comment 9 2010-03-18 08:58:50 PDT
Comment on attachment 51021 [details] Fix for how GraphicsLayer deals with animation states Clearing flags on attachment: 51021 Committed r56163: <http://trac.webkit.org/changeset/56163>
WebKit Commit Bot
Comment 10 2010-03-18 08:58:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.