RESOLVED FIXED 137265
[TexMap] Avoid constructing Vector when setting new children of TextureMapperLayer
https://bugs.webkit.org/show_bug.cgi?id=137265
Summary [TexMap] Avoid constructing Vector when setting new children of TextureMapper...
Zan Dobersek
Reported 2014-09-30 14:06:38 PDT
[TexMap] Avoid constructing Vector when setting new children of TextureMapperLayer
Attachments
Patch (4.66 KB, patch)
2014-09-30 14:11 PDT, Zan Dobersek
no flags
Patch (4.65 KB, patch)
2015-01-06 11:27 PST, Zan Dobersek
bfulgham: review+
Zan Dobersek
Comment 1 2014-09-30 14:11:01 PDT
Brent Fulgham
Comment 2 2014-10-31 11:05:03 PDT
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 3 2015-01-06 11:21:14 PST
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
Comment 4 2015-01-06 11:27:47 PST
Brent Fulgham
Comment 5 2015-01-06 14:23:02 PST
Comment on attachment 244079 [details] Patch Thank you for revising this! r=me.
Zan Dobersek
Comment 6 2015-01-07 05:14:25 PST
Csaba Osztrogonác
Comment 7 2015-01-07 07:41:19 PST
(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*>&'
Note You need to log in before you can comment on or make changes to this bug.