| Summary: | [TexMap] Avoid constructing Vector when setting new children of TextureMapperLayer | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
| Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarcelo, commit-queue, gyuyoung.kim, hw1008.kim, kondapallykalyan, luiz, mrobinson, noam, ossy | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Zan Dobersek
2014-09-30 14:06:38 PDT
Created attachment 238959 [details]
Patch
Comment on attachment 238959 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238959&action=review In general this seems like a good change, but I think it might be better if you simply modified the setChildren method to do this work. > Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:485 > + m_layer->addChild(toTextureMapperLayer(child)); Why not just encapsulate the operation of removing the children from the layer, and adding the new children, inside TextureMapperLayer? I.e., why couldn't setChildren just use the return value of "children()" as its argument? Comment on attachment 238959 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238959&action=review >> Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:485 >> + m_layer->addChild(toTextureMapperLayer(child)); > > Why not just encapsulate the operation of removing the children from the layer, and adding the new children, inside TextureMapperLayer? I.e., why couldn't setChildren just use the return value of "children()" as its argument? This requires GraphicsLayerTextureMapper.h to be included in TextureMapperLayer.cpp, but I think we can live with that. Created attachment 244079 [details]
Patch
Comment on attachment 244079 [details]
Patch
Thank you for revising this! r=me.
Committed r178034: <http://trac.webkit.org/changeset/178034> (In reply to comment #6) > Committed r178034: <http://trac.webkit.org/changeset/178034> It broke the EFL build: ../../Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp: In member function 'void WebKit::CoordinatedGraphicsScene::setLayerChildrenIfNeeded(WebCore::TextureMapperLayer*, const WebCore::CoordinatedGraphicsLayerState&)': ../../Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:218:32: error: no matching function for call to 'WebCore::TextureMapperLayer::setChildren(WTF::Vector<WebCore::TextureMapperLayer*>&)' ../../Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:218:32: note: candidate is: In file included from DerivedSources/WebKit2/include/WebCore/TextureMapperLayer.h:1:0, from ../../Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h:35, from ../../Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:24: ../../Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:81:10: note: void WebCore::TextureMapperLayer::setChildren(const WTF::Vector<WebCore::GraphicsLayer*>&) ../../Source/WebCore/platform/graphics/texmap/TextureMapperLayer.h:81:10: note: no known conversion for argument 1 from 'WTF::Vector<WebCore::TextureMapperLayer*>' to 'const WTF::Vector<WebCore::GraphicsLayer*>&' |