Bug 40841

Summary:

[Qt] Node that have both an opacity and a transform animation on them seem not to fire

Product:

WebKit

Reporter:

Sam Magnuson <smagnuso>

Component:

Layout and Rendering

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ademar, alessandro.portale, ariya.hidayat, commit-queue, eric, jarred, kling, laszlo.gombos, noam

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch (with testcase)	none
Test case tar'd up.	none
Rediff against trunk	none
Patch	eric: review-

Sam Magnuson

Reported 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.

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Sam Magnuson

Comment 1 2010-06-18 09:31:06 PDT

Created attachment 59122 [details] Proposed patch (with testcase)

Sam Magnuson

Comment 2 2010-06-18 09:32:21 PDT

Created attachment 59123 [details] Test case tar'd up.

Noam Rosenthal

Comment 3 2010-06-18 17:38:34 PDT

Ah, that explains a few things I was seeing :) Good patch.

Sam Magnuson

Comment 4 2010-06-22 16:12:49 PDT

Created attachment 59438 [details] Rediff against trunk

Ariya Hidayat

Comment 5 2010-08-08 12:57:58 PDT

Sam, why the patch does not include the test?

Kenneth Rohde Christiansen

Comment 6 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()

Eric Seidel (no email)

Comment 7 2010-10-14 11:35:05 PDT

Are there plans to update this patch?

Sam Magnuson

Comment 8 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.

Sam Magnuson

Comment 9 2010-10-21 16:50:17 PDT

Created attachment 71509 [details] Patch

Noam Rosenthal

Comment 10 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?

WebKit Commit Bot

Comment 11 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

Eric Seidel (no email)

Comment 12 2010-12-14 15:14:22 PST

Comment on attachment 71509 [details] Patch An updated patch is needed which includes -expected.txt files.

Andreas Kling

Comment 13 2011-05-17 09:58:45 PDT

*** Bug 59198 has been marked as a duplicate of this bug. ***

Andreas Kling

Comment 14 2011-05-17 10:12:28 PDT

I'll generate the missing -expected.txt and land this.

Andreas Kling

Comment 15 2011-05-17 10:17:30 PDT

Committed r86680: <http://trac.webkit.org/changeset/86680>

Ademar Reis

Comment 16 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>

Note You need to log in before you can comment on or make changes to this bug.