Closed Bug 1216832 Opened 4 years ago Closed 4 years ago

Hang near mozilla::layers::ClientMultiTiledLayerBuffer::PaintThebes with transform-style, will-change, layout.event-regions.enabled

Categories

(Core :: Layout, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox44 --- affected
firefox46 + fixed
firefox47 --- fixed

People

(Reporter: jruderman, Assigned: mattwoodrow)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: hang, testcase, Whiteboard: gfx-noted)

Attachments

(4 files)

With:
  user_pref("layout.event-regions.enabled", true);

the testcase causes a hang on Mac, along with 10+ GB of memory use. I do not have e10s enabled.
Attached file hang sample
Sounds fun.
Anything obvious?
Flags: needinfo?(matt.woodrow)
Whiteboard: gfx-noted
This is because we override the visible region for frames with preserve-3d to be the overflow area.

We're treating the entire massive div as visible, and trying to allocate tiles for it is killing us.
Component: Graphics: Layers → Layout
Flags: needinfo?(matt.woodrow)
Assignee: nobody → matt.woodrow
I don't entirely understand the way this used to work (though I'm pretty sure I wrote most of it).

The new code does this:

When we hit the root of a preserve-3d tree, store the current dirty rect.

For each frame that extends a preserve-3d tree, make sure we set the NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag on any descendants that are also part of the preserve-3d tree to make sure we build display items for them.

For each frame that has preserve-3d ancestors, use the dirty rect stored for the root and untransform it by the current accumulated transform.

This should give us correct visible regions for leaves, and for non-transformed content attached to the middle of a preserve-3d tree.

It might give us 'incorrect' (2d representations of areas that are only meaningful in 3d) visible regions for intermediate transform layers in the tree, but we should be ignoring these already for layers that are marked as preserving 3d.

It fixes this reftest, and passes all reftests in the transform-3d folder. I wouldn't be in a hurry to uplift this though, as it's pretty scary.
Attachment #8717742 - Flags: review?(roc)
Comment on attachment 8717742 [details] [diff] [review]
Handle visible regions with preserve-3d better

Review of attachment 8717742 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

You should be able to write a test here, a crashtest that will blow up before this fix.
Attachment #8717742 - Flags: review?(roc) → review+
Attachment #8718214 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/31da53652020
https://hg.mozilla.org/mozilla-central/rev/642a97a7cf34
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Blocks: 1224433
Comment on attachment 8717742 [details] [diff] [review]
Handle visible regions with preserve-3d better

Approval Request Comment
[Feature/regressing bug #]: Not a regression, but is necessary for bug 1224433 to work.
[User impact if declined]: Massive memory usage on some 3d webpages.
[Describe test coverage new/current, TreeHerder]: Tested manually, crashtest added, been on m-c for a few weeks.
[Risks and why]: Medium risk. This changes the way we compute visible regions for preserve-3d content to be sane instead of a wild overestimate. It's a decent sized change, but it's a big improvement in all ways.
[String/UUID change made/needed]: None.
Attachment #8717742 - Flags: approval-mozilla-aurora?
Marking affected for 46 so sheriffs will see the uplift request
Comment on attachment 8717742 [details] [diff] [review]
Handle visible regions with preserve-3d better

Should help with preserve-3d regressions in 46. Has automated test coverage
Please uplift to aurora.
Attachment #8717742 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
has problems uplifting to aurora 

grafting 328455:31da53652020 "Bug 1216832 - Handle preserve-3d visible regions during display list building by always transforming from the preserve-3d root each time. r=roc"
merging layout/base/nsDisplayList.cpp
merging layout/base/nsDisplayList.h
merging layout/generic/nsFrame.cpp
warning: conflicts while merging layout/base/nsDisplayList.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Tomcats-MacBook-Pro-2:mozilla-central Tomcat$
Flags: needinfo?(matt.woodrow)
Adding tracking to this since we want this feature to be in 46
Depends on: 1253190
You need to log in before you can comment on or make changes to this bug.