Closed Bug 1395476 Opened 3 years ago Closed 3 years ago

CSS nested 3d transform not displayed when filter also applied

Categories

(Core :: Web Painting, defect, P3)

54 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: i.atent.dead, Assigned: miko)

Details

Attachments

(3 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170628075643

Steps to reproduce:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Build 20170628075643

Set up a nested 3d transformation on some elements.
Add a filter to a parent element.

See https://codepen.io/anon/pen/QMzOzV

Possibly related to #1392191


Actual results:

Second level transformed element is not displayed.


Expected results:

Both elements should be displayed, with the filter applied.
Component: Untriaged → Layout
Product: Firefox → Core
Priority: -- → P3
Thanks for the bug report!  mozregression says this regressed in this range:
===
Last good revision: 11dc79e232110ba6de5179e46dfbda77b52a88c3 (2015-09-18)
First bad revision: 4313752f69956ae248bd4e7ff3913c8dd4252698 (2015-09-19)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=11dc79e232110ba6de5179e46dfbda77b52a88c3&tochange=4313752f69956ae248bd4e7ff3913c8dd4252698
===
--> regression from bug 1097464

(Not a recent regression, so wontfix for 57 which is only accepting low-risk/extremely-high-priority fixes at this point)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Here's the testcase taken from codepen, for archival purposes (in case the codepen disappears or something)
(Miko, this bug seems related to your area of expertise - maybe you'd be interested in taking this at some point?)
Component: Layout → Layout: Web Painting
I was able to reduce to testcase even more, and this seems pretty bad. I am not very familiar with the filter code, but I'll have a look.
Assignee: nobody → mikokm
Status: NEW → ASSIGNED
Attached file testcase-reduced.html
The problem here might be related to bug 1381753, but with BasicLayerManager. The trigger seems to be nested transform-style: preserve-3d combined with filter, but I am guessing this can also happen if a mask is used.

Both test cases work with WebRender enabled, so most likely this is a problem with layers.
The problem is that FrameLayerBuilder::RecomputeVisibilityForItems() gets called for display item (in this case nsDisplayFilter) containing nested nsDisplayTransforms. The GetBounds() call for the inner nsDisplayTransform returns an empty rectangle, which is intersected with the child item bounds when setting their visible rect. The empty visible rect causes the child items to get culled later in FrameLayerBuilder::PaintItems().
As discussed on irc, an alternative to this patch would be to teach RecomputeVisibility about preserve-3d items, so that we propagate the visible rect from the preserve-3d root down to the leaves, and then do normal RecomputeVisibility on the children of that.

That's a lot of work, and this work is purely an optimization, so we didn't think it was necessary for a very rare corner case.

We can revisit this if painting performance of this ever shows up as a problem.
Comment on attachment 8926216 [details]
Bug 1395476 - Do not call mStoredList.RecomputeVisibility for 3d transforms

https://reviewboard.mozilla.org/r/197470/#review202732

::: layout/painting/nsDisplayList.cpp:8458
(Diff revision 1)
>  {
> +  // nsDisplayTransform::GetBounds() returns an empty rect in nested 3d context.
> +  // Calling mStoredList.RecomputeVisibility below for such transform causes the
> +  // child display items to end up with empty visible rect.
> +  // We avoid this by bailing out always if we are dealing with a 3d context.
> +  if (!mFrame->Extend3DContext()) {

Shouldn't this be the opposite, bail out if Extend3DContext is true?

Maybe:

if (mFrame->Extend3DContext() || mFrame->Combines3DTransformWithAncestors()) {
 return true;
}

As currently written it will early return for a normal (non-preserve-3d) transform, which isn't what we want.
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> Shouldn't this be the opposite, bail out if Extend3DContext is true?
You are totally right. This is now fixed.

I also added the new reftest to layout/reftests/transform-3d/reftest.list.
Comment on attachment 8926216 [details]
Bug 1395476 - Do not call mStoredList.RecomputeVisibility for 3d transforms

https://reviewboard.mozilla.org/r/197470/#review202974
Attachment #8926216 - Flags: review?(matt.woodrow) → review+
Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/250063fc6ce2
Do not call mStoredList.RecomputeVisibility for 3d transforms r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/250063fc6ce2
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.