Closed Bug 1214212 Opened 4 years ago Closed 4 years ago

Crash in mozilla::layers::InstallLayerClipPreserves3D when loading the THREE.CSS3DRenderer demo

Categories

(Core :: Graphics: Layers, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox42 --- unaffected
firefox43 --- unaffected
firefox44 + unaffected
firefox45 --- unaffected
firefox46 --- fixed
b2g-v2.5 --- unaffected
b2g-master --- fixed

People

(Reporter: RyanVM, Assigned: sinker)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(3 files, 7 obsolete files)

+++ This bug was initially created as a clone of Bug #1031866 +++

Tested on both Win10 and Linux. We reliably crash with Nightly on this url:

http://mrdoob.com/lab/javascript/threejs/css3d/

Here is the stack:
https://crash-stats.mozilla.com/report/index/8750eadf-8ab4-428e-973b-6fc882151013

Also horrible rendering issues prior to crashing. Regressed by bug 1097464. As expected, this doesn't reproduce on Aurora43 since bug 1097464 was backed out from there.
Flags: needinfo?(tlee)
Thinker, do we back this out, or is there a fix we can have quickly?
Assignee: nobody → tlee
It seems a dup of Bug 1211360.  There is a patch.  Could you check it?
Flags: needinfo?(tlee)
I don't get crash issue on linux.  So, I need more time to study crash problem.
Still crashes with the patch from bug 1211360 applied.
Ryan, could you check this patch to make sure it stopping the crash?
Still crashes.
--
Attachment #8674797 [details] [diff] - Attachment is obsolete: true
Attachment #8674797 - Attachment is obsolete: true
Ryan, this is a more complete solution.  I can not reproduce the bug on my desktop anymore, so if it does still not cure your problem, please try a debug build and give me a log.  Thank you!
Comment on attachment 8675253 [details] [diff] [review]
Remove clips from separator transform items, v2

Robert, the problem here is elements participating a 3D rendering context having clips too.  It run into the MOZ_CRASH() we had put in InstallLayerClipPreserves3D(), clips of layers could not be applied if the layer's effective transform can not be draw in 2D (not CanDraw2D()), it can happen only for preserves 3D cases.

My solution is making every child item of a transformed, and extending 3D, item as a child of an additional separator transform item with ID transform.  If there is any clips on children, an intermediate surface will be created for the layer of it's containing separator item, and the clips would be applied on the intermediate surface without running into MOZ_CRASH().
Attachment #8675253 - Flags: feedback?(roc)
I forget to mentioned, children that is transformed and participating the same 3D rendering context will not be a child of a separator item.  They will be direct children of the transformed, and extending 3D, item.  There are two cases for this type of children:
 1. the frame of the child item is extending 3D too, or
 2. the frame of the child item is a leaf; participating the context, but flattening.
They can not have clips.  So, we can make them as direct children of the parent item, and be direct children on layer tree too.
Comment on attachment 8675253 [details] [diff] [review]
Remove clips from separator transform items, v2

Appears to have fixed the crash, but there's still visual glitches when the page first loads and when you click and drag the mouse to rotate the cube.
Attached image screenshot after initial page load (obsolete) —
This is what things look like immediately after the page loads with attachment  8675253 [details] [diff] [review] applied on m-c tip (rev bb5c1f7cc078).
Attached image screenshot after rotating the cube (obsolete) —
This shows the artifacting the occurs when dragging the mouse to rotate the cube around.
No warnings/asserts in a debug build. Seems like some kind of invalidation issue - if I resize the window, the cube renders normally until moved again.
Do you mean it not fix glitches even applied the patch of Bug 1211360?
I need to correct comment #16, the right patch is on bug 1210784.
Depends on: 1210784
With this patch and the patches from bug 1211360 and bug 1210784 applied, things are definitely heading in the right direction. No crashing and invalidation issues AFAICT.

That said, there are still some issues remaining as shown in the attached screenshot and the one coming next. The side of the cube showing burning fire appears to partially stop drawing after a couple seconds. In this screenshot, the left side continued to animate while the right side was frozen.
Attachment #8675319 - Attachment is obsolete: true
Attachment #8675321 - Attachment is obsolete: true
This screenshot shows two different issues: the blue side of the cube should be drawn consistently (the left side is the correct rendering). Also, the black rectangle of the left side of the screenshot starts as the full height of the viewport, then drops to the size shown in the screenshot, before finally fully disappearing. Another possible invalidation issue I guess since they tend to go away when resizing the window.
Ryan, thank you for your feedback!  I have updated bug 1210784.  Both issues you mentioned are gone on my box.
The latest patch definitely fixed the issues on the cube. However, I'm still seeing intermittent black bars on the sides during initial page load that eventually invalidate and go away given enough time.
Attachment #8675440 - Attachment is obsolete: true
Attachment #8675441 - Attachment is obsolete: true
Attached patch layersdump.diffSplinter Review
Ryan, I have updated bug 1210784.  If it is still not normal, apply this patch to get layer tree, I may find out what the root cause is with log.
Comment on attachment 8675253 [details] [diff] [review]
Remove clips from separator transform items, v2

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

::: layout/generic/nsFrame.cpp
@@ +1929,5 @@
> +    nsDisplayTransform *sepIdItem =
> +      new (aBuilder) nsDisplayTransform(aBuilder, aFrame, aSource,
> +                                        aDirtyRect, Matrix4x4(), aIndex);
> +    sepIdItem->SetNoExtendContext();
> +   aTarget->AppendToTop(sepIdItem);

Fix indent
Attachment #8675253 - Flags: feedback?(roc) → feedback+
--
Attachment #8675253 [details] [diff] - Attachment is obsolete: true
Attachment #8676755 - Flags: review?(roc)
Attachment #8675253 - Attachment is obsolete: true
Comment on attachment 8676755 [details] [diff] [review]
Remove clips from separator transform items, v3

please checkin this patch without closing the bug.
Attachment #8676755 - Flags: checkin+
Keywords: leave-open
https://treeherder.mozilla.org/logviewer.html#?job_id=12857609&repo=try

This reftest (layout/reftests/bugs/745934-1.html) a little bit different in color for every pixel.  Do you think we can just make it in fuzzy?
Flags: needinfo?(roc)
The test looks pretty different in the Mulet log:
https://treeherder.mozilla.org/logviewer.html#?job_id=16072311&repo=mozilla-inbound
906199-1.html is also quite different.
Flags: needinfo?(roc)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #29)
> The test looks pretty different in the Mulet log:
> https://treeherder.mozilla.org/logviewer.html#?job_id=16072311&repo=mozilla-
> inbound
> 906199-1.html is also quite different.

These are already fixed on my local patch.  For separator transforms are not force active, some cases layers are not created as expected.  But, after fix this problem, comment #28 is happened.
--
Attachment #8676755 [details] [diff] - Attachment is obsolete: true
Attachment #8677884 - Flags: review?(roc)
Attachment #8676755 - Attachment is obsolete: true
Minor changes
 - Make nsDisplayTransform items being separator to be LAYER_ACTIVE_FORCE.
 - nsDisplayTransform::GetBounds()returns bounds of children (or visible overflow rect) for items being separator.
 - fuzzing reftests

https://treeherder.mozilla.org/#/jobs?repo=try&revision=72a37b5eea7d
The patch that landed here took care of the crashes. The glitchy rendering appears to be another manifestation of bug 1210784, so let's just re-purpose this bug and close it out.
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: leave-opencrash
Resolution: --- → FIXED
Summary: Glitchy rendering and crash in mozilla::layers::InstallLayerClipPreserves3D → Crash in mozilla::layers::InstallLayerClipPreserves3D when loading the THREE.CSS3DRenderer demo
Target Milestone: --- → mozilla44
No longer depends on: 1210784
Blocks: 1224433
Tracked since it's a crash, even though it is already fixed.
Backed out from Aurora44 so that bug 1097464 could be backed out cleanly. It remains fixed on Fx45+ where bug 1097464 is still landed.

https://hg.mozilla.org/releases/mozilla-aurora/rev/b1716d3ede0d
Blocks: 1240783
This along with the change that caused this regression were backed out from Firefox 45.

https://hg.mozilla.org/releases/mozilla-aurora/rev/64ec448f156d
You need to log in before you can comment on or make changes to this bug.