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-
Patch (19.32 KB, patch)
2011-11-18 15:57 PST, Igor Trindade Oliveira
no flags
Patch (19.77 KB, patch)
2011-11-18 16:34 PST, Igor Trindade Oliveira
no flags
Patch (20.73 KB, patch)
2011-11-21 06:32 PST, Igor Trindade Oliveira
noam: review+
webkit.review.bot: commit-queue-
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
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.