1494110 - Unnecessary invalidation of elements with an SVG mask when scrolling

Status: NEW

(Core :: SVG, defect, P3)

Description

[Jonathan Watt [:jwatt]]

As detailed in bug 1494092 comment 2, the code at:

https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/layout/svg/nsSVGIntegrationUtils.cpp#334-341

invalidates the entire frame bounds for frames with inactive SVG mask layers when the page is scrolled. Stack:

  nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects
  mozilla::FrameLayerBuilder::AddPaintedDisplayItem
  mozilla::PaintedLayerDataNode::PopAllPaintedLayerData
  mozilla::PaintedLayerDataNode::Finish
  mozilla::PaintedLayerDataNode::FinishChildrenIntersecting
  mozilla::PaintedLayerDataTree::FinishPotentiallyIntersectingNodes
  mozilla::PaintedLayerDataTree::AddingOwnLayer
  mozilla::ContainerState::ProcessDisplayItems
  mozilla::FrameLayerBuilder::BuildContainerLayerFor
  nsDisplayList::BuildLayers
  nsDisplayList::PaintRoot
  nsLayoutUtils::PaintFrame
  mozilla::PresShell::Paint
  nsViewManager::ProcessPendingUpdatesPaint
  nsViewManager::ProcessPendingUpdatesForView
  nsViewManager::ProcessPendingUpdates
  nsRefreshDriver::Tick

The only reason that:

  layout/reftests/invalidation/scroll-inactive-layers.html
  layout/reftests/invalidation/scroll-inactive-layers-2.html

don't currently failing is because of a bug in the `if` block above that (bug 1494092 comment 2 for the details of that).

Comment 1

[Miko Mynttinen]

I recently looked at the DLBI code for filters and masks, and spotted some other possible issues:

- nsDisplaySVGEffectGeometry::mBBox and nsDisplaySVGEffectGeometry::mUserSpaceOffset are not moved in MoveBy()[1].

- nsDisplayMaskGeometry::mDestRects are not moved in MoveBy()[2].

- nsDisplayFilterGeometry does not handle changing opacity, only nsDisplayMaskGeometry does[3]. This could be moved to nsDisplaySVGEffectGeometry to avoid duplicating code.

- nsDisplayFilterGeometry uses nsImageGeometryMixin, but the display item type specifies TYPE_RENDERS_NO_IMAGES [4].

[1]: https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/layout/painting/nsDisplayListInvalidation.cpp#138
[2]: https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/layout/painting/nsDisplayListInvalidation.cpp#148
[3]: https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/layout/painting/nsDisplayListInvalidation.cpp#149
[4]: https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/layout/painting/nsDisplayItemTypesList.h#59

Disclaimer: I am not very familiar with SVG invalidation, so it might be that some or all of these are as intended.

Comment 2

[Jonathan Watt [:jwatt]]

Note that the current patch for bug 1482403 may mask this issue or change behavior by making ShouldUseNewItem return true for SVG effects. We'd like to remove that though!