1328020 - 3d transform doesn't correctly alignes elements in space

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P2)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open attached "testcase 1"

AR:  Blue element is displayed "above" the gray element
ER:  Blue element should be placed below the gray element,
     just like on Nightly 2016-05-26 or GoogleChrome

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]: layout is broken

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=36614ec50c15a5c2f3085b133b5bf698996d6751&tochange=459802a227c2ba71cd742f8399a35ee68fc04d6a

Regressed by: Bug 1274673

Comment 3

[Miko Mynttinen]

(In reply to Alice0775 White from comment #2)
> [Tracking Requested - why for this release]: layout is broken
> 
> Regression window:
> https://hg.mozilla.org/integration/autoland/
> pushloghtml?fromchange=36614ec50c15a5c2f3085b133b5bf698996d6751&tochange=4598
> 02a227c2ba71cd742f8399a35ee68fc04d6a
> 
> Regressed by: Bug 1274673

Bug 1274673 introduced a completely new layer sorting algorithm, which seems to have broken this test case.

I'm guessing the cause for this behavior might be numerical inaccuracies or sorting of co-planar layers. I'll look into it in more detail shortly.

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Miko Mynttinen [:miko] from comment #3)
> (In reply to Alice0775 White from comment #2)
> > [Tracking Requested - why for this release]: layout is broken
> > 
> > Regression window:
> > https://hg.mozilla.org/integration/autoland/
> > pushloghtml?fromchange=36614ec50c15a5c2f3085b133b5bf698996d6751&tochange=4598
> > 02a227c2ba71cd742f8399a35ee68fc04d6a
> > 
> > Regressed by: Bug 1274673
> 
> Bug 1274673 introduced a completely new layer sorting algorithm, which seems
> to have broken this test case.
> 
> I'm guessing the cause for this behavior might be numerical inaccuracies or
> sorting of co-planar layers. I'll look into it in more detail shortly.

Great, thanks.  It'd be good not to have it get to developer edition, so we have until the end of next week.

Comment 5

[Marcia Knous [:marcia]]

Tracking 53+ for this layout regression.

Comment 6

[Miko Mynttinen]

The current layer sorting behavior with compositor backends that do not support arbitrary layer geometry (DirectX) is as follows:
1) The layers are sorted using BSP tree and split along the intersections if needed
2) If there were splits, the duplicate layers are filtered out

Unfortunately, it is impossible to render scenes with intersections (such as the test case in this bug) correctly with this approach, or with the previous sorting algorithm.

I see these as solutions:
1) Leave the current behavior as it is. This will cause some regressions with DirectX compositor when scenes have intersecting layers (most likely the rendering would be incorrect anyway). The results with this approach should usually be at least as good as with the legacy sorting.

2) Re-sort the layers using the legacy sorting algorithm if the BSP tree sorting detects splits. This should render scenes with intersections as they were before. The downside is a small performance hit because the layer sorting is now done twice.

3) Use only the legacy sorting algorithm with DirectX compositor.

This problem will eventually go away with the DirectX compositor triangle rendering (bug 1323791).

Comment 7

[Matt Woodrow (:mattwoodrow)]

Both the old code and the new-fallback code resolve intersections arbitrarily. Trying to pick one over the other based on specific websites doesn't seem productive as we'll always regress other pages in the process.

We should just work on getting the 'real' fix complete (bug 1323791), and do our best to make sure it ships in the same release as plane splitting.

Getting it done before the next merge might be tough, but it should be a fairly easy uplift early in the Aurora cycle.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Let's sort this out before we change back to the old way of failing and then call it another regression.  In the meantime, not tracking.

Comment 9

[Miko Mynttinen]

The changes in bug 1323791 landed, and this issue is now fixed in Nightly.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Based on comment 7, it sounds like this is a wontfix for 53?

Comment 11

[Miko Mynttinen]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> Based on comment 7, it sounds like this is a wontfix for 53?

Yes, this is correct.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Bug 1323791 is wontfix for 54, so IIUC, that makes this wontfix as well.