1081185 - CSS 3D transform clipped wrongly

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

Attachment: bugclipping.html

Steps To Reproduce:
1. Open attached
   --- observe pink trapezoid
2. Reload (F5)
   --- observe pink trapezoid

Actual Results:
The pink trapezoid parts clipped with white trapezoid

Expected Results:
Not clipped, No white trapezoid

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9350909a3401
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0 ID:20140719062420
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24a69de91baa
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0 ID:20140719065519
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9350909a3401&tochange=24a69de91baa

Regressed by: Bug 1022612

Comment 1

[Alice0775 White]

Attachment: bug.png

Comment 2

[Alice0775 White]

[Tracking Requested - why for this release]:
rendering error due to regression of Bug 1022612 Part 20.


In local build
Last Good: 58d7c85b017e
First Bad: 2b0d786121d4

Regressed by:
	2b0d786121d4	Robert O'Callahan — Bug 1022612. Part 20: Do the business. r=mattwoodrowchecking. r=mattwoodrow

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This fixes it and seems logical, though I can't say I understand this code very well.

Comment 4

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8508409 [details] [diff] [review]
fix

Review of attachment 8508409 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! Hope that didn't take too long to find.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Some notes from my debugging here:
The basic problem is that we were computing an invalid visible rect for the transformed DIV in the reftest:
     BackgroundColor p=0x79bcf45c f=0x79cf87f8(Block(div)(1)) bounds(0,0,7200,7200) visible(3308,0,3892,7200) componentAlpha(0,0,0,0) clip(0,0,7200,7200)  uniform (opaque 0,0,7200,7200) (rgba 1.000000,0.000000,1.000000,1.000000) layer=0x79b1dc50

The computed transform was
$32 = {_11 = 1, _12 = 0, _13 = 0, _14 = 0,
       _21 = 0.599999964, _22 = 0.599999964, _23 = -1, _24 = 0.00999999978,
       _31 = 0, _32 = 1, _33 = 0, _34 = 0,
       _41 = -35.9999962, _42 = 84, _43 = 60, _44 = 0.400000006}
and its inverse
$44 = {_11 = 1, _12 = 0, _13 = 0, _14 = 0,
       _21 = 0, _22 = 0, _23 = 1, _24 = 0, 
       _31 = 0.599999964, _32 = -0.400000006, _33 = -0.599999964, _34 = 0.00999999978,
       _41 = -1.66893005e-06, _42 = 60, _43 = -120, _44 = 1}

x==3308 is about 55px; you can easily see that transforming (55px,0px,0px) by the transform gives you an x value of about 20px, which is well to the right of the left edge of the window. Using larger x or y values obviously moves the point further to the right.

When I dug into ProjectRectBounds we had that situation where the w values for different corners have different signs. I noticed and fixed the obvious bug with the ordering of the points, and that fixed the reftest, though I have to admit I don't fully understand this code.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/37e954c9b929

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/37e954c9b929

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

34 and 35 are marked as affected and this looks safe enough to uplift. Do you agree?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8508409 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: 1022612 (actually that bug didn't cause the problem, just exposed it)
[User impact if declined]: misrendering of some pages using CSS 3d transforms
[Describe test coverage new/current, TBPL]: quite good reftest coverage
[Risks and why]: very low risk
[String/UUID change made/needed]: none

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8508409 [details] [diff] [review]
fix

Beta+
Aurora+

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9449bb444443
https://hg.mozilla.org/releases/mozilla-beta/rev/acd99fc02446

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/acd99fc02446

Comment 13

[Brogan Zumwalt [Inactive]]

Issue verified fixed on Flame 2.1 and Flame 2.2

Actual Results: Pink trapezoid is not clipped, no white trapezoid is visible.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104001202
Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5
Gecko: 388b03efe92d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104040207
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 54d05732f29b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0