968555 - backface-visibility should not create a stacking context

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P4)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

According to http://dev.w3.org/csswg/css-transforms/ :

 * backface-visibility shouldn't trigger creation of a stacking context
 * perspective should trigger creation of a stacking context

The code we use to determine whether transforms trigger a stacking context does the opposite on both counts, as I pointed out in bug 964885 comment 3.  (See nsIFrame::BuildDisplayListForChild isStackingContext -> isVisuallyAtomic -> nsIFrame::IsTransformed -> nsStyleDisplay::HasTransform(nsIFrame*) -> nsStyleDisplay::HasTransformStyle()).

I'm inclined to agree with the spec on backface-visibility -- it doesn't seem like the sort of thing that should trigger a stacking context, though maybe I'm missing some deeper implication it has.

I don't have much of an opinion on whether perspective ought to trigger a stacking context, since I don't really understand it right now.  Does it make sense that it ought to?

I'm also curious about whether IsTransformed / HasTransform / HasTransformStyle are really doing the right thing for the bulk of their callers.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Heh, I disagree with both of these :)

If backface-visiblity doesn't establish a stacking context, then we only hide the backface of that element, and not of descendants. That seems pretty different to the rest of transforms.

I also don't see the point of making perspective establish a stacking context. It doesn't really do anything except pass the perspective onto children that are actually transformed.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I posted about perspective:
http://lists.w3.org/Archives/Public/public-fx/2014JanMar/0070.html

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It seems to me that backface-visibility only does anything interesting when it's applied to an element that has a 3-D transform, and elements with a transform already establish a stacking context.  So why should backface-visibility have any additional effects?  Why isn't transform creating a stacking context good enough?

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Testcase

The attached testcase shows an example of a frame that doesn't have any 'transform' (or other stacking context properties), but still expects backface-visibility:hidden to work.

This is a massively reduced version of the original bug that prompted me to make it a stacking context.

So we don't actually need a stacking context here, but we do need to make sure that display items from a backface-visibility:hidden frame are layerized separately to any other content so we can cull them and only them.

I think we could drop backface-visibility from 'IsTransform' (which stops it being a stacking context) and instead make  'WrapPreserve3DList' aware that it needs to look for backface hidden display items to wrap.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't create a stacking context for backface-visibility:hidden

This ended up being easier than I thought.

This should preserve the behaviour for the given testcase, but without requiring a stacking context.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8380967 [details] [diff] [review]
Don't create a stacking context for backface-visibility:hidden

>[mq]: backface-hidden

Please include the commit message when posting patches for review.

nsStyleStruct.h:

Maybe rename HasBackfaceHidden to IsBackfaceHidden or BackfaceIsHidden,
since there's not a thing that it "has", really?

>   /* Returns whether the element has the -moz-transform property
>-   * or a related property. */
>+   * or a related property that requires a transform stacking context  */

I'm not convinced that this definition is what we want HasTransformStyle
to be in the long run.  e.g., I wouldn't want to add perspective here
just because it requires a stacking context.

Maybe just leave the comment alone for now?


nsIFrame.h:

>+   * Returns whether this frame has a parent that Preserves3DChildren(), has its own
>+   * transform (or hidden backface) to be combined with the parent and
>+   * if it be drawn with preserve-3d. Returns false if the frame is clipped.

First line runs past 80 chars.

the "and if it be drawn with preserve-3d" doesn't really make sense;
please reword more grammatically.

I'm also not sure what the existing "Returns false if the frame is clipped."
means.  Maybe remove it?  Whether the *parent* frame is clipped is
already covered by whether the parent Preserves3DChildren().

nsDisplayTransform.cpp:

In both GetDeltaToTransformOrigin and GetDeltaToPerspectiveOrigin you have:

>+  if (aFrame->StyleDisplay()->HasBackfaceHidden() &&
>+      !aFrame->IsTransformed()) {
>+    return gfxPoint3D();
>+  }

It seems you really only need to check !aFrame->IsTransformed(); you can
drop the HasBackfaceHidden() check (given the assertion you have on the
previous line).


I'm not sure how to verify that this is the total of what required
having the mBackfaceVisibility check in HasTransform, so I'm going to
trust you on that bit.


r=dbaron

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Let's make this bug about just the backface-visibility half of the change; I'll file a separate bug on perspective.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 976364 for perspective creating a stacking context, and bug 976365 for creating a containing block for abs and fixed pos elements.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2 (tests): Test that 'backface-visibility' does not cause creation of a stacking context.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88723262c85c

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

landed tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/543382e6f367

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/543382e6f367