Closed
Bug 968555
Opened 10 years ago
Closed 10 years ago
backface-visibility should not create a stacking context
Categories
(Core :: Web Painting, defect, P4)
Core
Web Painting
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: dbaron, Assigned: mattwoodrow)
References
(Depends on 2 open bugs, Regressed 1 open bug)
Details
(Keywords: css3)
Attachments
(3 files)
343 bytes,
text/html
|
Details | |
8.91 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
1.50 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(matt.woodrow)
Updated•10 years ago
|
Priority: -- → P4
Assignee | ||
Comment 1•10 years ago
|
||
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)
Reporter | ||
Comment 2•10 years ago
|
||
I posted about perspective: http://lists.w3.org/Archives/Public/public-fx/2014JanMar/0070.html
Reporter | ||
Comment 3•10 years ago
|
||
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)
Assignee | ||
Comment 4•10 years ago
|
||
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)
Assignee | ||
Comment 5•10 years ago
|
||
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)
Reporter | ||
Comment 6•10 years ago
|
||
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+
Reporter | ||
Comment 7•10 years ago
|
||
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
Reporter | ||
Comment 8•10 years ago
|
||
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
Reporter | ||
Comment 9•10 years ago
|
||
Attachment #8381026 -
Flags: review?(matt.woodrow)
Assignee | ||
Updated•10 years ago
|
Attachment #8381026 -
Flags: review?(matt.woodrow) → review+
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/88723262c85c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Reporter | ||
Comment 11•10 years ago
|
||
landed tests: https://hg.mozilla.org/integration/mozilla-inbound/rev/543382e6f367
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/543382e6f367
Assignee: nobody → dbaron
Reporter | ||
Updated•10 years ago
|
Assignee: dbaron → matt.woodrow
Updated•5 years ago
|
Component: Layout: View Rendering → Layout: Web Painting
You need to log in
before you can comment on or make changes to this bug.
Description
•