To reproduce start "QtTestBrowser -graphicsbased" and visit the following page: http://mrdoob.com/projects/chromeexperiments/google-gravity/. The problem seems to be related to TextureMapper / GraphicsLayerTextureMapper creation. Either we end up having multiple root layers, or a root layer without a TextureMapper (possibly due to a race condition). I will investigate further and post my findings.
Note to No'am and Huang: assert(textureMapper) in GraphicsLayerTextureMapper::updateBackingStore is what's failing after the change.
minimal example to reproduce the problem: <html><body> <div style="-webkit-transform: translateZ(0px);">hello</div> </body></html>
Created attachment 176272 [details] Backtrace
After I read code again, I understand GraphicsLayerTextureMapper::updateBackingStore has a problem. GraphicsLayer::FlushCompositingState can be called by RenderLayerBacking. It means this method can be called before creating TextureMapper. So TextureMapperLayer::flushCompositingState() checks and if textureMapper does not exist, do nothing. void TextureMapperLayer::flushCompositingState(GraphicsLayerTextureMapper* graphicsLayer, TextureMapper* textureMapper, int options) { if (!textureMapper) return; ... } But GraphicsLayerTextureMapper::updateBackingStore expects textureMapper always exists. Bug 103366 can fix this bug, but I think it is right that this bug fixes hitting assertion and then reviews Bug 103366. If we change code like following code, this bug can be fixed. In the detail, we must keep m_needsDisplay until GraphicsLayerTextureMapper::prepareBackingStore() is completed, so m_needsDisplay should be removed in GraphicsLayerTextureMapper::didFlushCompositingState(). I'm glad anyone to fix it. If you want, I can do it of course. @@ -404,8 +410,6 @@ void GraphicsLayerTextureMapper::didFlushCompositingState() { updateBackingStore(); m_changeMask = 0; - m_needsDisplay = false; - m_needsDisplayRect = IntRect(); } void GraphicsLayerTextureMapper::didFlushCompositingStateRecursive() @@ -430,6 +434,10 @@ void GraphicsLayerTextureMapper::updateBackingStore() void GraphicsLayerTextureMapper::prepareBackingStore() { + TextureMapper* textureMapper = m_layer->textureMapper(); + if (!textureMapper) + return; + if (!shouldHaveBackingStore()) { m_backingStore.clear(); return; @@ -441,9 +449,6 @@ void GraphicsLayerTextureMapper::prepareBackingStore() if (dirtyRect.isEmpty()) return; - TextureMapper* textureMapper = m_layer->textureMapper(); - ASSERT(textureMapper); - if (!m_backingStore) m_backingStore = TextureMapperTiledBackingStore::create();
> I'm glad anyone to fix it. If you want, I can do it of course. Yes, if you could fix this before posting more refactor patches it would be great :)
(In reply to comment #5) > > I'm glad anyone to fix it. If you want, I can do it of course. > Yes, if you could fix this before posting more refactor patches it would be great :) My explanation can not fix this bug yet. "QtTestBrowser -graphicsbased" can not draw minimal text in #c2. I'm also investigating why.
Created attachment 176391 [details] Patch : GraphicsLayerTextureMapper::updateBackingStore() early returns until TextureMapper is set.
Created attachment 176392 [details] Patch : TextureMapperLayerClientQt::setTextureMapper() must call flushCompositingStateForThisLayerOnly() of the root layer.
Comment on attachment 176391 [details] Patch : GraphicsLayerTextureMapper::updateBackingStore() early returns until TextureMapper is set. View in context: https://bugs.webkit.org/attachment.cgi?id=176391&action=review LGTM with changelog rewordings. > Source/WebCore/ChangeLog:8 > + GraphicsLayerTextureMapper::updateBackingStore() early returns until TextureMapper is set. early returns until -> should return early before > Source/WebCore/ChangeLog:12 > + TextureMapperLayer::flushCompositingState() checks and early returns if returns early > Source/WebCore/ChangeLog:13 > + TextureMapper does not exist. does not exist -> was not created > Source/WebCore/ChangeLog:17 > + However, GraphicsLayerTextureMapper::updateBackingStore() expects that TextureMapper > + always exists. This patch makes updateBackingStore() early return like > + TextureMapperLayer::flushCompositingState(). updateBackingStore should also return early when TextureMapper was not created.
Comment on attachment 176392 [details] Patch : TextureMapperLayerClientQt::setTextureMapper() must call flushCompositingStateForThisLayerOnly() of the root layer. View in context: https://bugs.webkit.org/attachment.cgi?id=176392&action=review LGTM with ChangeLog rewording. > Source/WebKit/qt/ChangeLog:16 > + Currently, PageClientQGraphicsWidget::setRootGraphicsLayer does not flush layer > + states of the root layer after setting TextureMapper while > + PageClientQWidget::setRootGraphicsLayer() flushes them of the root layer. If > + not flushing states, descendant layers can not reach the root layer. It means > + descendant layers can not reach the TextureMapper that the root layer has. > + This is already the behavior for PageClientQWidget, which should be the case for PageClientQGraphicsWidget as well.
Thanks Huang and No'am! We're trying to cut a release of the Qt port and branch this weekend. I appreciate how quickly you responded. Thanks a lot! :) We can test the patches during the course of today also and land them with the suggested changes if they work, unless somebody is quicker :)
LGTM :-) I tried both patches, and they seem to reliably resolve the issue. Thank you very much for looking into it so quickly!
Comment on attachment 176391 [details] Patch : GraphicsLayerTextureMapper::updateBackingStore() early returns until TextureMapper is set. Committed r135986.
Comment on attachment 176392 [details] Patch : TextureMapperLayerClientQt::setTextureMapper() must call flushCompositingStateForThisLayerOnly() of the root layer. Committed r135987.
All reviewed patches have been landed. Closing bug.
Thanks for review and manual patch and good advice!!
> We're trying to cut a release of the Qt port and branch this weekend. I appreciate how quickly you responded. Thanks a lot! :) No problem Simon, let me know if anything in coordinated graphics is blocking your release and I'll respond as quickly as I can.