Bug 84195 – Improve W3C conformance of backface visibility

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Shawn Singh]

Recently it seems like the W3C spec changed, or at least was clarified, and chromium does not exactly obey the latest spec.   http://dev.w3.org/csswg/css3-transforms/#backface-visibility-property   Specifically rule #1 defines specific behavior when there is a transformed element that does not have a 3d rendering context (i.e. it is not actually a 3d layer).

I have attached a mock set of tests.  Chrome and Safari both behave correctly when the inner-most layer is kept as 3d.  (ignore layer sorting).  However, for the 3rd case, I think chrome's behavior actually fits the spec, even though Safari has a different result.

More importantly, however, chrome might fail these semantics when a hierarchy of renderSurfaces are involved.  Currently, hierarchies of renderSurfaces are quite rare, so this bug is not a high priority.

[Shawn Singh]

Created attachment 140778 [details]
Patch

[Shawn Singh]

I think I implemented test cases correctly according to the latest W3C spec (and this patch makes chromium work according to the spec).  However, Safari does not pass a few of the cases, specifically backface-visibility-hierarchical-transform.html  and  backface-visibility-no3d.html.

Simon, I think you were already aware of this bug in a different bug report, but I couldn't find that bug number.

Do I need to do anything specific to mark these tests as expecting to fail on Safari pixel tests?


Otherwise this patch is ready for review - thanks in advance.

[Adrienne Walker]

(In reply to comment #2)

> Do I need to do anything specific to mark these tests as expecting to fail on Safari pixel tests?

In general, you can update LayoutTests/platform/mac/test_expectations.txt to add IMAGE (or FAIL) expectations for Safari tests.

[Shawn Singh]

Created attachment 140794 [details]
Patch with safari test expectations

[Shawn Singh]

Comment on attachment 140794 [details]
Patch with safari test expectations

Removing r=? because I forgot to check unit tests.  One test needs to be updated related to animations and prepainting.

[WebKit Review Bot]

Comment on attachment 140794 [details]
Patch with safari test expectations

Attachment 140794 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12649537

New failing tests:
compositing/backface-visibility/backface-visibility-3d.html
compositing/backface-visibility/backface-visibility-non3d.html
CCLayerTreeHostCommonTest.verifyBackFaceCulling

[WebKit Review Bot]

Created attachment 140811 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Shawn Singh]

Created attachment 142358 [details]
Fixed patch, and added unit tests

[Shawn Singh]

(In reply to comment #8)
> Created an attachment (id=142358) [details]
> Fixed patch, and added unit tests

(1) fixed the layout test breakage from previous patch, by removing <br> tags that had different spacing on different platforms

(2) Added logic that deals with render surfaces backface visibility correctly

(3) fixed one existing unit test

(4) added 2 more unit tests

(5) Tested this patch on mac layout tests and unit tests, no regressions.

I feel this patch now provides pretty good coverage of W3C backface visibility semantics for chromium.

[Shawn Singh]

Created attachment 142368 [details]
Exact same patch, just rebased to tip of tree

[WebKit Review Bot]

Comment on attachment 142368 [details]
Exact same patch, just rebased to tip of tree

Attachment 142368 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12723181

New failing tests:
compositing/backface-visibility/backface-visibility-3d.html
compositing/backface-visibility/backface-visibility-non3d.html

[WebKit Review Bot]

Created attachment 142377 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Shawn Singh]

Created attachment 142589 [details]
Fixed test baselines again, should work this time

[Shawn Singh]

Created attachment 143082 [details]
PaUpdate to patch due to r117645

Enne, can you please review this? Thanks very much. I don't think the style errors have anything to do with this patch.

[WebKit Review Bot]

Attachment 143082 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/comp..." exit_code: 1
LayoutTests/platform/mac/test_expectations.txt:161:  fast/inline/continuation-outlines-with-layers.html is also in a Skipped file.  [test/expectations] [5]
LayoutTests/platform/mac/test_expectations.txt:170:  tables/mozilla_expected_failures/collapsing_borders/bug41262-5.html is also in a Skipped file.  [test/expectations] [5]
LayoutTests/platform/mac/test_expectations.txt:219:  ietestcenter/css3/valuesandunits/units-000.htm is also in a Skipped file.  [test/expectations] [5]
Total errors found: 3 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adrienne Walker]

Comment on attachment 143082 [details]
PaUpdate to patch due to r117645

View in context: https://bugs.webkit.org/attachment.cgi?id=143082&action=review

R=me.  I love all the comments and named functions in CCLayerTreeHostCommon.  Thanks for all the tests and for taking the time to make the rectangles in the layout tests axis aligned.

> Source/WebCore/ChangeLog:3
> +        [chromium] Improve W3C conformance of backface-visibility

I know that all the code changes are Chromium-specific, but if these new tests fail on any other platform it might be unexpected to see them in a [chromium] change.  So, I'd prefer that you land this whole patch without the [chromium] tag.

[Shawn Singh]

OK, I will fix that when I land very soon =)

[Shawn Singh]

Committed r118117: <http://trac.webkit.org/changeset/118117>

[Sergio Villar Senin]

Are you sure that the 3rd test case is correct? It's true that backface visibility is computed differently if there is not preserve-3d but you still have to take into account the whole set of transforms, and the lime div is backfacing (because its parent is rotated 100º), so knowing that the backface-visibility is hidden I see no reason why it should be visible.

[Shawn Singh]

(In reply to comment #19)
> Are you sure that the 3rd test case is correct? It's true that backface visibility is computed differently if there is not preserve-3d but you still have to take into account the whole set of transforms, and the lime div is backfacing (because its parent is rotated 100º), so knowing that the backface-visibility is hidden I see no reason why it should be visible.

Hi, can you please clarify the 3rd test case of which .html layout test?   Then I can take a look at what you're asking about.

In the meantime I can say that, according to the spec, when the layer is not in an existing 3d rendering context, backface visibility is supposed to be determined by the layer's own local transform.  (Rule #1 in section 12 of http://dev.w3.org/csswg/css3-transforms/#backface-visibility-property)  In our code that transform is layer->transform().

[Shawn Singh]

(In reply to comment #20)
> (In reply to comment #19)
> > Are you sure that the 3rd test case is correct? It's true that backface visibility is computed differently if there is not preserve-3d but you still have to take into account the whole set of transforms, and the lime div is backfacing (because its parent is rotated 100º), so knowing that the backface-visibility is hidden I see no reason why it should be visible.
> 
> Hi, can you please clarify the 3rd test case of which .html layout test?   Then I can take a look at what you're asking about.
> 
> In the meantime I can say that, according to the spec, when the layer is not in an existing 3d rendering context, backface visibility is supposed to be determined by the layer's own local transform.  (Rule #1 in section 12 of http://dev.w3.org/csswg/css3-transforms/#backface-visibility-property)  In our code that transform is layer->transform().

to clarify, the conceptual interpretation of all this, is that the layer should *not* actually consider any transforms from its ancestor layers.

[Simon Fraser (smfr)]

> the layer should *not* actually consider any transforms from its ancestor layers
if not in a 3D rendering context, right.

[Sergio Villar Senin]

(In reply to comment #22)
> > the layer should *not* actually consider any transforms from its ancestor layers
> if not in a 3D rendering context, right.

but just for backface culling right?

[Shawn Singh]

(In reply to comment #23)
> (In reply to comment #22)
> > > the layer should *not* actually consider any transforms from its ancestor layers
> > if not in a 3D rendering context, right.
> 
> but just for backface culling right?

Yes, I believe we're only talking about backface visibility (backface culling) right now.  But if you still think there is an issue about the test cases, it would be very helpful if you can point out exactly which file / test case, and we can discuss more concretely =)

[Sergio Villar Senin]

(In reply to comment #24)
> (In reply to comment #23)
> > (In reply to comment #22)
> > > > the layer should *not* actually consider any transforms from its ancestor layers
> > > if not in a 3D rendering context, right.
> > 
> > but just for backface culling right?
> 
> Yes, I believe we're only talking about backface visibility (backface culling) right now.  But if you still think there is an issue about the test cases, it would be very helpful if you can point out exactly which file / test case, and we can discuss more concretely =)

Sorry for the noise then, the test makes perfect sense now. It was just a bit weird for me that the transformations where inherited for rendering purpouses but now for backface culling (when !preserve-3d), but you're right that the standard mentions it clearly.

PS: I was talking about the backface-visibility-non3d.html test case, I thought I had mentioned it before but I hadn't

[Sergio Villar Senin]

(In reply to comment #25)
> Sorry for the noise then, the test makes perfect sense now. It was just a bit weird for me that the transformations where inherited for rendering purpouses but now for backface culling (when !preserve-3d), but you're right that the 

s/but now for/but NOT for