The shader is always recompiled before paint. Add code to cache the shader at least until next paint.
Created attachment 174817 [details] Patch V1
Comment on attachment 174817 [details] Patch V1 View in context: https://bugs.webkit.org/attachment.cgi?id=174817&action=review Great progress, see comments. > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp:152 > + WebCustomFilterProgramProxy* customFilterProgramProxy = static_cast<WebCustomFilterProgramProxy*>(program->platformCompiledProgram()->client()); This seems a bit messy... I am sure we have enough information in LayerTreeCoordinator to know which shaders we have to send. > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp:159 > + if (!customFilterProgramProxy->wasSentToUIProcess()) { I would prefer if we sent all the new programs to the Ui process in LayerTreeCoordinator rather than here with an if condition. > Source/WebKit2/Shared/CoordinatedGraphics/WebCustomFilterProgramProxy.h:75 > + bool wasSentToUIProcess() const { return m_wasSentToUIProcess; } Sent you went through the trouble of creating this class, maybe it should also do its own encoding/decoding rather than leave it to CoordinatedGraphicsArgumentCoders. > Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:292 > + injectCachedCustomFilterPrograms(const_cast<FilterOperations&>(filters)); Would be cleaner to create a local copy and inject to it rather than const_cast
(In reply to comment #2) > (From update of attachment 174817 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174817&action=review > > Great progress, see comments. Thanks Noam, your comments make sense. I will integrate the feedback and upload again.
Created attachment 174832 [details] Patch V2 Integrated the feedback.
Comment on attachment 174832 [details] Patch V2 View in context: https://bugs.webkit.org/attachment.cgi?id=174832&action=review Very nice! Some pre-landing nits below... > Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp:270 > + int programId = 0; > + if (!decoder->decode(programId)) programID > Source/WebKit2/Shared/CoordinatedGraphics/WebCustomFilterOperation.h:50 > + int programId() const { return m_programId; } programID() and m_programID (WebKit style) > Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:313 > + } > + WebCustomFilterOperation* customOperation = static_cast<WebCustomFilterOperation*>(operation); Add line break > Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:319 > + if (iter == m_customFilterPrograms.end()) { > + ASSERT_NOT_REACHED(); > + continue; > + } ASSERT(iter != m_customFilterPrograms.end()) > Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:338 > + if (iter == m_customFilterPrograms.end()) { > + ASSERT_NOT_REACHED(); > + return; > + } Ditto
btw we should probably call checkCustomFilterProgramProxies for filter operations inside an animation, otherwise those might not be rendered correctly.
(In reply to comment #6) > btw we should probably call checkCustomFilterProgramProxies for filter operations inside an animation, otherwise those might not be rendered correctly. Hm.. didn't know the filter animations are accelerated on Texture Mapper. Will do.
Created attachment 174835 [details] Patch V3 I've added the animations fix, so I've set the CQ+ flag again. @Noam: is there an automatic way to test the composited animations?
(In reply to comment #8) > @Noam: is there an automatic way to test the composited animations? Unfortunately not really quite yet, we would need good support for taking several pixel snapshots for the same page and that's tricky.
Comment on attachment 174835 [details] Patch V3 Clearing flags on attachment: 174835 Committed r135056: <http://trac.webkit.org/changeset/135056>
All reviewed patches have been landed. Closing bug.