RESOLVED FIXED 198091
Layer flashing and poor perf during scrolling of message list on gmail.com and hotmail.com - overlap testing needs to constrained to clipping scopes
https://bugs.webkit.org/show_bug.cgi?id=198091
Summary Layer flashing and poor perf during scrolling of message list on gmail.com an...
Simon Fraser (smfr)
Reported 2019-05-21 15:50:06 PDT
Layer flashing and poor perf during scrolling of message list on gmail.com and hotmail.com - overlap testing needs to constrained to clipping scopes
Attachments
Patch (64.62 KB, patch)
2019-05-21 16:27 PDT, Simon Fraser (smfr)
koivisto: review+
Simon Fraser (smfr)
Comment 1 2019-05-21 16:27:32 PDT
Simon Fraser (smfr)
Comment 2 2019-05-21 16:28:21 PDT
Antti Koivisto
Comment 3 2019-05-21 16:53:21 PDT
Comment on attachment 370354 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370354&action=review > Source/WebCore/rendering/LayerOverlapMap.cpp:180 > + auto* clippingScope = findClippingScopeForLayers(enclosingClippingLayers); > + if (!clippingScope) > + return false; Does this happen and what does it mean if it does? Could we assert here instead? > Source/WebCore/rendering/LayerOverlapMap.cpp:246 > +OverlapMapContainer::ClippingScope* OverlapMapContainer::findClippingScopeForLayers(const Vector<LayerOverlapMap::LayerAndBounds>& enclosingClippingLayers) const > +{ > + ASSERT(enclosingClippingLayers.size()); > + ASSERT(enclosingClippingLayers[0].layer.isRenderViewLayer()); > + > + const auto* currScope = &m_rootScope; > + for (unsigned i = 1; i < enclosingClippingLayers.size(); ++i) { > + auto& scopeLayerAndBounds = enclosingClippingLayers[i]; > + auto* childScope = currScope->childWithLayer(scopeLayerAndBounds.layer); > + if (!childScope) > + return nullptr; > + > + currScope = childScope; > + } > + > + return const_cast<ClippingScope*>(currScope); As discussed, maybe this can be replaced with a side hashmap? That might make the code easier to follow and avoid passing the full enclosingClippingLayers stack. > Source/WebCore/rendering/RenderLayerCompositor.cpp:1811 > + for (const auto* currLayer = layer.parent(); currLayer; currLayer = currLayer->parent()) { Not a fan of the 'currFoo' naming.
Simon Fraser (smfr)
Comment 4 2019-05-21 17:13:08 PDT
Note You need to log in before you can comment on or make changes to this bug.