WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
72753
[WK2][Qt] Move Accelerated Composite animations to UIProcess
https://bugs.webkit.org/show_bug.cgi?id=72753
Summary
[WK2][Qt] Move Accelerated Composite animations to UIProcess
Igor Trindade Oliveira
Reported
2011-11-18 13:30:29 PST
SSIA
Attachments
Patch
(19.93 KB, patch)
2011-11-18 13:39 PST
,
Igor Trindade Oliveira
noam
: review-
Details
Formatted Diff
Diff
Patch
(19.32 KB, patch)
2011-11-18 15:57 PST
,
Igor Trindade Oliveira
no flags
Details
Formatted Diff
Diff
Patch
(19.77 KB, patch)
2011-11-18 16:34 PST
,
Igor Trindade Oliveira
no flags
Details
Formatted Diff
Diff
Patch
(20.73 KB, patch)
2011-11-21 06:32 PST
,
Igor Trindade Oliveira
noam
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Igor Trindade Oliveira
Comment 1
2011-11-18 13:39:43 PST
Created
attachment 115864
[details]
Patch Proposed patch version 1.
Noam Rosenthal
Comment 2
2011-11-18 15:14:59 PST
Comment on
attachment 115864
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=115864&action=review
Awesome! See comments, they are mainly nitpicks.
> Source/WebCore/ChangeLog:6 > + Add helper method to synchronize animations.
... in TextureMapper.
> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:987 > +void TextureMapperNode::syncAnimationsRecursively(GraphicsLayerTextureMapper* graphicsLayer)
The argument might not be necessary. See next comment.
> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:994 > + Vector<GraphicsLayer*> children = graphicsLayer->children();
You probably want to use the TextureMapperNode's children and not the graphicsLayer's children, otherwise you might be syncing animations for items that aren't in the synchronized tree yet.
> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:1000 > + }
early return.
> Source/WebKit2/UIProcess/LayerTreeHostProxy.h:129 > + void updateSceneGraphUpdateTimerFired(Timer<LayerTreeHostProxy>*);
I believe the convention is didFireViewportUpdateTimer
> Source/WebKit2/UIProcess/LayerTreeHostProxy.h:130 > + Timer<LayerTreeHostProxy> m_sceneGraphUpdateTimer;
Let's use viewport instead of scenegraph
> Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:283 > +#if PLATFORM(QT)
Why QT only? This seems cross-platform. Nobody else uses WebGraphicsLayer yet so don't worry :)
> Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:306 > +#if PLATFORM(QT)
ditto
> Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:318 > +#if PLATFORM(QT)
ditto
Igor Trindade Oliveira
Comment 3
2011-11-18 15:57:58 PST
Created
attachment 115897
[details]
Patch Patch v2. Implement all suggestions proposed by Noam.
Igor Trindade Oliveira
Comment 4
2011-11-18 16:34:24 PST
Created
attachment 115907
[details]
Patch Patch v2. Improve Changelog
Noam Rosenthal
Comment 5
2011-11-18 16:57:20 PST
Comment on
attachment 115907
[details]
Patch Awsomeness.
WebKit Review Bot
Comment 6
2011-11-18 18:23:49 PST
Comment on
attachment 115907
[details]
Patch Clearing flags on attachment: 115907 Committed
r100834
: <
http://trac.webkit.org/changeset/100834
>
WebKit Review Bot
Comment 7
2011-11-18 18:23:53 PST
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 8
2011-11-19 05:33:50 PST
Many tests started to crash after this patch:
http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r100834%20%2815660%29/results.html
Unfortunately it made run-webkit-test exit before finishing. :( Could you check it please as soon as possible?
Igor Trindade Oliveira
Comment 9
2011-11-19 05:42:32 PST
Ok Ossy, i am investigating right now (In reply to
comment #8
)
> Many tests started to crash after this patch: >
http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r100834%20%2815660%29/results.html
> > Unfortunately it made run-webkit-test exit before finishing. :( > > Could you check it please as soon as possible?
Csaba Osztrogonác
Comment 10
2011-11-21 02:42:59 PST
Rolled out by
http://trac.webkit.org/changeset/100859
Igor Trindade Oliveira
Comment 11
2011-11-21 06:32:34 PST
Created
attachment 116076
[details]
Patch Proposed patch. This patch fixes the crashes.
Noam Rosenthal
Comment 12
2011-11-21 07:02:18 PST
Comment on
attachment 116076
[details]
Patch Cool, Please commit yourself when you can watch the bots :)
WebKit Review Bot
Comment 13
2011-11-21 08:42:14 PST
Comment on
attachment 116076
[details]
Patch
Attachment 116076
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/10363332
New failing tests: inspector/extensions/extensions-events.html
Noam Rosenthal
Comment 14
2011-11-27 10:14:02 PST
Igor, should we close this bug?
Igor Trindade Oliveira
Comment 15
2011-11-27 10:36:52 PST
Committed
r100922
:
http://trac.webkit.org/changeset/100922
(In reply to
comment #14
)
> Igor, should we close this bug?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug