Bug 40846 - [Qt] GraphicsLayerQt delay seems unnecessary.
Summary: [Qt] GraphicsLayerQt delay seems unnecessary.
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
: 40778 (view as bug list)
Depends on:
Blocks: 38744
  Show dependency treegraph
 
Reported: 2010-06-18 11:01 PDT by Sam Magnuson
Modified: 2011-04-18 19:48 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch (with testcase) (8.34 KB, patch)
2010-06-18 11:15 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Rediff against trunk (2.57 KB, patch)
2010-06-22 16:13 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Magnuson 2010-06-18 11:01:14 PDT
I had noticed that if I had a delay in my animation it would get offset strangely. Further investigation makes me believe that the animation takes care of firing after the delay on its own and GraphicsLayerQt.cpp doesn't need to delay the animation's start. Is there a test case that shows its required? The attached test case seems to indicate otherwise.
Comment 1 Sam Magnuson 2010-06-18 11:15:00 PDT
Created attachment 59137 [details]
Proposed patch (with testcase)
Comment 2 Sam Magnuson 2010-06-18 11:15:43 PDT
To actually use that testcase I will need bug #40841 to be applied as well as the items will remain transparent (and the timing won't be seen)
Comment 3 Noam Rosenthal 2010-06-18 17:40:40 PDT
Can you make sure it works with LayoutTests/animations/fill-mode-transform.html?
It's an existing test case that has delay and uses AC.
Comment 4 Sam Magnuson 2010-06-21 09:59:58 PDT
(In reply to comment #3)
> Can you make sure it works with LayoutTests/animations/fill-mode-transform.html?
> It's an existing test case that has delay and uses AC.

Thanks for the pointer to that test, however, it fails to pass. When I take out my change it continues to fail in the same way - so I'm not thinking my change is causing the failure. Has this test passed completely before?
Comment 5 Noam Rosenthal 2010-06-21 10:42:53 PDT
As long as your change doesn't create a regression wrt delays, I have no problems with it. I'll look further into that test, I think it's unstable.
Comment 6 Sam Magnuson 2010-06-21 10:53:05 PDT
(In reply to comment #5)
> As long as your change doesn't create a regression wrt delays, I have no problems with it. I'll look further into that test, I think it's unstable.

I'm not seeing any worse behaviour, and in the test that I included in here it makes the graphicsview behave the same as when disabled.
Comment 7 Sam Magnuson 2010-06-22 16:13:20 PDT
Created attachment 59439 [details]
Rediff against trunk
Comment 8 Noam Rosenthal 2010-06-25 13:20:21 PDT
*** Bug 40778 has been marked as a duplicate of this bug. ***
Comment 9 WebKit Commit Bot 2010-06-28 08:47:45 PDT
Comment on attachment 59439 [details]
Rediff against trunk

Clearing flags on attachment: 59439

Committed r62010: <http://trac.webkit.org/changeset/62010>
Comment 10 WebKit Commit Bot 2010-06-28 08:47:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Hausmann 2010-07-01 01:25:51 PDT
Revision r62010 cherry-picked into qtwebkit-2.0 with commit 45fa2b8b4d6906275f948b93076c38f9539427b0