Bug 137265 – [TexMap] Avoid constructing Vector when setting new children of TextureMapperLayer

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

[TexMap] Avoid constructing Vector when setting new children of TextureMapperLayer

[Zan Dobersek]

Created attachment 238959 [details]
Patch

[Brent Fulgham]

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?

[Zan Dobersek]

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.

[Zan Dobersek]

Created attachment 244079 [details]
Patch

[Brent Fulgham]

Comment on attachment 244079 [details]
Patch

Thank you for revising this! r=me.

[Zan Dobersek]

Committed r178034: <http://trac.webkit.org/changeset/178034>

[Csaba Osztrogonác]

(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*>&'