[webkit-reviews] review denied: [Bug 101801] [Texmap][CSS Shaders] Reuse the precompiled shader for custom filters in TextureMapperGL : [Attachment 174817] Patch V1
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Nov 17 09:48:11 PST 2012
Noam Rosenthal <noam at webkit.org> has denied Alexandru Chiculita
<achicu at adobe.com>'s request for review:
Bug 101801: [Texmap][CSS Shaders] Reuse the precompiled shader for custom
filters in TextureMapperGL
https://bugs.webkit.org/show_bug.cgi?id=101801
Attachment 174817: Patch V1
https://bugs.webkit.org/attachment.cgi?id=174817&action=review
------- Additional Comments from Noam Rosenthal <noam at webkit.org>
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()->c
lient());
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
More information about the webkit-reviews
mailing list