Move layer-tree traverse from TextureMapperLaye to GraphicsLayerTextureMapper. It is because of two reasons. 1. It is a part of implementing actor model. After implementing actor model, each GraphicsLayerTextureMapper needs to send a layer state message to TextureMapperLayer. It means the outside instance of TextureMapperLayer should perform recursive traverse. 2. LayerTreeRenderer needs to syncronize a layer state of TextureMapperLayer when receiving the SetCompositingLayerState message. See Bug 103171.
Created attachment 176176 [details] Patch
Now GraphicsLayerTextureMapper::flushCompositingStateForThisLayerOnly() synchronize a layer state and animation on this layer only. Currently, GraphicsLayerTextureMapper::flushCompositingStateForThisLayerOnly() performs synchronizing the layer state of the given layer and the animations of children as well as the given layer. However, there is no requirement that flushCompositingStateForThisLayerOnly() synchronizes all animations in layer tree. All clients that call flushCompositingStateForThisLayerOnly() uses GraphicsLayer in the same way. rootLayer->flushCompositingStateForThisLayerOnly() nonCompositingLayer->flushCompositingStateForThisLayerOnly() frameView->flushCompositingStateIncludingSubframes() It means that flushCompositingStateForThisLayerOnly() needs to synchronize the animation of only the given layer. Currently, we traverse 3 times to sync animation redundantly.
Created attachment 176200 [details] Patch
Comment on attachment 176200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176200&action=review > Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:434 > + return; Now we can call GraphicsLayerTextureMapper::flushCompositingState() before setting TextureMapper to the root TextureMapperLayer. GraphicsLayerTextureMapper::flushCompositingState() can sync layer states except for completing prepareBackingStore(). prepareBackingStore() can not be perform without TextureMapper, but it is safe because we keep m_needsDisplay and united m_needsDisplayRect until prepareBackingStore() completes after setting TextureMapper to the root layer.
Created attachment 178471 [details] Patch
Comment on attachment 178471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178471&action=review You're also removing the SyncOptions thingy... it's ok but you need to specific in the changelog why/how this is ok. > Source/WebCore/ChangeLog:11 > + urrently, CoordinatedGraphicsLayer performs layer tree traversal by > + itself, but GraphicsLayerTextureMapper delegates the tree traversal to > + TextureMapperLayer. This patch decides the tree traversal is the role of > + GraphicsLayer. Perform the layer-tree traversal in GraphicsLayerTextureMapper. > Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:420 > + toGraphicsLayerTextureMapper(maskLayer())->flushCompositingState(rect); you don't need toGraphicsLayerTextureMapper
Created attachment 178479 [details] Patch
(In reply to comment #6) > (From update of attachment 178471 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178471&action=review Thank you for review! > You're also removing the SyncOptions thingy... it's ok but you need to specific in the changelog why/how this is ok. Oops, I missed. > > Source/WebCore/ChangeLog:11 > > + urrently, CoordinatedGraphicsLayer performs layer tree traversal by > > + itself, but GraphicsLayerTextureMapper delegates the tree traversal to > > + TextureMapperLayer. This patch decides the tree traversal is the role of > > + GraphicsLayer. > > Perform the layer-tree traversal in GraphicsLayerTextureMapper. I rephrased based on your help :) > > > Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:420 > > + toGraphicsLayerTextureMapper(maskLayer())->flushCompositingState(rect); > > you don't need toGraphicsLayerTextureMapper Done.
mrobinson, noamr and I talked about GTK+ AC regresstion in IRC. Bug 103046 made the problem, and this bug fixes the problem. I did not understand how Bug 103046 can cause the problem yet.
(In reply to comment #9) > mrobinson, noamr and I talked about GTK+ AC regresstion in IRC. > > Bug 103046 made the problem, and this bug fixes the problem. > > I did not understand how Bug 103046 can cause the problem yet. I filed Bug 104640 to fix the bug.
Created attachment 178739 [details] Patch
(In reply to comment #11) > Created an attachment (id=178739) [details] > Patch Fix Bug 104640 the first and refactor this bug the second.
Comment on attachment 178739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178739&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:367 > + flushCompositingStateSelf(graphicsLayer); > + syncAnimations(); IS this needed? can't we have one function, that calls syncAnimations in the end?
Created attachment 178934 [details] Patch
(In reply to comment #13) > (From update of attachment 178739 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178739&action=review > > > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:367 > > + flushCompositingStateSelf(graphicsLayer); > > + syncAnimations(); > > IS this needed? can't we have one function, that calls syncAnimations in the end? Good suggestion. I changed :)
Comment on attachment 178934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178934&action=review I'm OK with this. I'd rather someone else cq+ if things are stable enough for this. > Source/WebCore/ChangeLog:19 > + In addition, this patch removes SyncOptions enum. Currently, when using > + ComputationsOnly, the layer-tree traversal synchronizes only animations > + without layer states. However, there is no client that calls > + flushCompositingState() for the purpose. Even we already have the API > + for the purpose: applyAnimationsRecursively(). So from now on, we have > + only flushCompositingStateForThisLayerOnly() API in TextureMapperLayer > + for brevity. Just say: Also removed the SyncOptions enum, which is redundant since no client calls it with ComputationsOnly.
Created attachment 179367 [details] Patch
(In reply to comment #16) > (From update of attachment 178934 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178934&action=review > > I'm OK with this. I'd rather someone else cq+ if things are stable enough for this. I think it is stable. Now a day, I tested much :) > Just say: > Also removed the SyncOptions enum, which is redundant since no client calls it with ComputationsOnly. Thank you for rephrasing!
Comment on attachment 179367 [details] Patch Clearing flags on attachment: 179367 Committed r137867: <http://trac.webkit.org/changeset/137867>
All reviewed patches have been landed. Closing bug.