Bug 40841 - [Qt] Node that have both an opacity and a transform animation on them seem not to fire
Summary: [Qt] Node that have both an opacity and a transform animation on them seem no...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: Qt, QtTriaged
: 59198 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-06-18 09:08 PDT by Sam Magnuson
Modified: 2011-05-17 11:58 PDT (History)
9 users (show)

See Also:


Attachments
Proposed patch (with testcase) (10.35 KB, patch)
2010-06-18 09:31 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Test case tar'd up. (1.13 KB, application/x-gzip)
2010-06-18 09:32 PDT, Sam Magnuson
no flags Details
Rediff against trunk (4.56 KB, patch)
2010-06-22 16:12 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Patch (8.74 KB, patch)
2010-10-21 16:50 PDT, Sam Magnuson
eric: review-
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 09:08:07 PDT
I have attached a manual test case that reproduces the problem on my page as well as a proposed fix. The timing is not correct either, but I will submit that as a separate bug once I determine what is going on there.
Comment 1 Sam Magnuson 2010-06-18 09:31:06 PDT
Created attachment 59122 [details]
Proposed patch (with testcase)
Comment 2 Sam Magnuson 2010-06-18 09:32:21 PDT
Created attachment 59123 [details]
Test case tar'd up.
Comment 3 Noam Rosenthal 2010-06-18 17:38:34 PDT
Ah, that explains a few things I was seeing :)
Good patch.
Comment 4 Sam Magnuson 2010-06-22 16:12:49 PDT
Created attachment 59438 [details]
Rediff against trunk
Comment 5 Ariya Hidayat 2010-08-08 12:57:58 PDT
Sam, why the patch does not include the test?
Comment 6 Kenneth Rohde Christiansen 2010-08-14 07:35:26 PDT
Comment on attachment 59438 [details]
Rediff against trunk

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:634
 +              else if (!m_state.drawsContent && m_layer->drawsContent())
Is this an important differencial?

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1194
 +      virtual AnimatedPropertyID getAnimatedProperty() const = 0;
We normally do not use get. Maybe just call it animatedProperty()
Comment 7 Eric Seidel 2010-10-14 11:35:05 PDT
Are there plans to update this patch?
Comment 8 Sam Magnuson 2010-10-21 16:25:26 PDT
(In reply to comment #6)
> (From update of attachment 59438 [details])
> WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:634
>  +              else if (!m_state.drawsContent && m_layer->drawsContent())
> Is this an important differencial?
> 

For the duration of this function, yes. It sync's the two at the end of the function but this conditional just notices that the state has changed and what action to take.

> WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1194
>  +      virtual AnimatedPropertyID getAnimatedProperty() const = 0;
> We normally do not use get. Maybe just call it animatedProperty()

Fair enough, will rename.
Comment 9 Sam Magnuson 2010-10-21 16:50:17 PDT
Created attachment 71509 [details]
Patch
Comment 10 Noam Rosenthal 2010-10-23 01:19:07 PDT
Sam, this looks like a good patch - would you like it committed or are you waiting for another patch?
Comment 11 WebKit Commit Bot 2010-10-24 10:01:47 PDT
Comment on attachment 71509 [details]
Patch

Rejecting patch 71509 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2
Last 500 characters of output:
ests/websocket/tests/workers ......
http/tests/workers .....
http/tests/xhtmlmp .
http/tests/xmlhttprequest ...........................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers .........
547.40s total testing time

21631 test cases (99%) succeeded
1 test case (<1%) was new
11 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/4615058
Comment 12 Eric Seidel 2010-12-14 15:14:22 PST
Comment on attachment 71509 [details]
Patch

An updated patch is needed which includes -expected.txt files.
Comment 13 Andreas Kling 2011-05-17 09:58:45 PDT
*** Bug 59198 has been marked as a duplicate of this bug. ***
Comment 14 Andreas Kling 2011-05-17 10:12:28 PDT
I'll generate the missing -expected.txt and land this.
Comment 15 Andreas Kling 2011-05-17 10:17:30 PDT
Committed r86680: <http://trac.webkit.org/changeset/86680>
Comment 16 Ademar Reis 2011-05-17 11:57:47 PDT
Revision r86680 cherry-picked into qtwebkit-2.2 with commit 4495597 <http://gitorious.org/webkit/qtwebkit/commit/4495597>