Closed Bug 968555 Opened 6 years ago Closed 6 years ago

backface-visibility should not create a stacking context

Categories

(Core :: Web Painting, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: dbaron, Assigned: mattwoodrow)

References

(Depends on 3 open bugs)

Details

(Keywords: css3)

Attachments

(3 files)

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.
Flags: needinfo?(matt.woodrow)
Priority: -- → P4
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.
Flags: needinfo?(matt.woodrow)
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?
Flags: needinfo?(matt.woodrow)
Attached file 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.
Flags: needinfo?(matt.woodrow)
This ended up being easier than I thought.

This should preserve the behaviour for the given testcase, but without requiring a stacking context.
Attachment #8380967 - Flags: review?(dbaron)
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
Attachment #8380967 - Flags: review?(dbaron) → review+
Let's make this bug about just the backface-visibility half of the change; I'll file a separate bug on perspective.
Summary: perspective should create a stacking context; backface-visibility should not create a stacking context → backface-visibility should not create a stacking context
I filed bug 976364 for perspective creating a stacking context, and bug 976365 for creating a containing block for abs and fixed pos elements.
Keywords: css3
Attachment #8381026 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/88723262c85c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee: dbaron → matt.woodrow
Depends on: 1030124
Depends on: 1038652
Depends on: 1185376
Depends on: 1201471
No longer depends on: 1201471
Component: Layout: View Rendering → Layout: Web Painting
Depends on: 1494429
You need to log in before you can comment on or make changes to this bug.