Last Comment Bug 718809 - getBoundingClientRect returns incorrect results for certain 3D transforms
: getBoundingClientRect returns incorrect results for certain 3D transforms
Status: RESOLVED FIXED
[parity-webkit]
:
Product: Core
Classification: Components
Component: DOM: CSS Object Model (show other bugs)
: Trunk
: x86 Linux
: -- minor (vote)
: mozilla12
Assigned To: Matt Woodrow (:mattwoodrow)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 591718
  Show dependency treegraph
 
Reported: 2012-01-17 13:19 PST by Aryeh Gregor (:ayg) (next working March 28-April 26)
Modified: 2012-01-23 11:53 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use TransformBounds instead (909 bytes, patch)
2012-01-18 19:39 PST, Matt Woodrow (:mattwoodrow)
roc: review+
Details | Diff | Splinter Review
Use TransformBounds instead (2.75 KB, patch)
2012-01-19 15:33 PST, Matt Woodrow (:mattwoodrow)
roc: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-01-17 13:19:24 PST
Fairly minimal test-case:

data:text/html,<!DOCTYPE html>
<body style=margin:0>
<div style="background:blue;height:50px;width:100px;
-moz-transform: matrix3d(
1, 0,  0, 0,
0, 1,  0, 0,
0, 1,  1, 0,
0, 0, 10, 1);-moz-transform-origin:0 0"></div>
<script>
var rect = document.querySelector("div").getBoundingClientRect();
document.body.appendChild(document.createTextNode(
rect.top + " " + rect.right + " " + rect.bottom + " " + rect.left));
</script>

Actual output (Firefox 12.0a1 2012-01-12): -10 100 40 0
Expected output (found in Chrome 17 dev): 0 100 50 0

The effect of the transform is to add the z-component to the y-component, then add 10px to the z-component.  The rectangle starts with its corners at
  (0,0,0), (100,0,0), (100,50,0), (0,50,0),
and the effect should be to add 10 to the z-component of each.  This shouldn't affect the rendering, and indeed it doesn't.  But the bounding client rect reported by Gecko (12.0a1 2012-01-12) has top at -10 and bottom at 40, instead of top at 0 and bottom at 50 as expected.  Chrome 17 correctly reports the top at 0 and the bottom at 50.  Changing the off-diagonal 1 or 10 in the matrix to a 0 makes the bug vanish, so this is pretty much minimal.

I observed this by fuzz-testing with some random matrices for CSS 3D Transform tests I'm working on.  There might also be other bugs causing the Gecko failures I'm seeing, but this is the first reduction I got.

I didn't test on IE10, since I don't have access to it.  It might be useful for someone to try that.
Comment 1 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-01-18 08:11:32 PST
Tests I'm working on that uncovered the bug:

http://aryeh.name/tmp/css-test/contributors/aryehgregor/incoming/3d-transforms.html

(As the tmp/ in the URL indicates, this is not necessarily a stable page.   A stabler version can be found at <http://hg.csswg.org/test/file/tip/contributors/aryehgregor/incoming/3d-transforms.html>, but it doesn't run directly in the browser, you can only view the source.)

I'm now fuzz-testing with the identity matrix with every possible choice of one or two entries replaced with 10.  This produced incorrect getBoundingClientRect() in Firefox 12.0a1 (2012-01-17) for the following matrices:

  matrix3d(1, 0, 10, 0,  0, 1, 0, 0,   10, 0, 1, 0,  0, 0, 0, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 10, 0,  10, 0, 1, 0,  0, 0, 0, 1)
  matrix3d(1, 0, 10, 0,  0, 1, 0, 0,   0, 10, 1, 0,  0, 0, 0, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 10, 0,  0, 10, 1, 0,  0, 0, 0, 1)
  matrix3d(1, 0, 10, 0,  0, 1, 0, 0,   0, 0, 1, 10,  0, 0, 0, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 10, 0,  0, 0, 1, 10,  0, 0, 0, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 0, 0,   10, 0, 1, 0,  0, 0, 10, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 0, 0,   0, 10, 1, 0,  0, 0, 10, 1)
  matrix3d(1, 0, 0, 0,   0, 1, 0, 0,   0, 0, 1, 10,  0, 0, 10, 1)

I don't know if this is one bug or multiple bugs, but I observe by inspection that all of these matrices will map a point (x, y, 0, 1) to (x, y, z, 1) for some z, so they should have the same effect on getBoundingClientRect() as the identity matrix, and they don't.

WebKit (Chrome 17 dev) is correct on all of these, AFAICT.  It fails for anything with the fourth, eighth, or sixteenth entry nonzero -- it needs the bottom row to be (0, 0, x, 1), apparently.  As noted, I wasn't able to easily test on IE10.
Comment 2 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-01-18 09:56:04 PST
Also all the following rotations:

  rotateX(22.5deg)
  rotateY(22.5deg)
  rotate3d(1, 0, 0, 22.5deg)
  rotate3d(0, 1, 0, 22.5deg)
  rotate3d(1, -1, 0, 22.5deg)
  rotate3d(3, -17.2, 0.14, 22.5deg)

Plus the same six with various other angles I tried (45deg, 86.451deg, 90deg, 270deg, 452deg, -1rad, 1rad, 256grad).  In all cases, WebKit gives correct values.  Gecko seems to render right in the case I looked at (I didn't check all those permutations!), but gives incorrect getBoundingClientRect() results.

I'll stop posting examples for now -- there should be enough to look at, and I don't want to spam.
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-01-18 17:36:38 PST
This is because of the way gfx3DMatrix::ProjectPoint works.

  // Define a ray of the form P + Ut where t is a real number
  // w is assumed to always be 1 when transforming 3d points with our
  // 4x4 matrix.
  // p is our click point, q is another point on the same ray.
  // 
  // Note: since the transformation is a general projective transformation and is not
  // necessarily affine, we can't just take a unit vector u, back-transform it, and use
  // it as unit vector on the back-transformed ray. Instead, we really must take two points
  // on the ray and back-transform them.

Maybe we need a different sort of projection for getBoundingClientRect? That would seem strange since that's what our rendering code seems to do.

Over to Matt.
Comment 4 Matt Woodrow (:mattwoodrow) 2012-01-18 19:39:20 PST
Created attachment 589756 [details] [diff] [review]
Use TransformBounds instead

This is an incorrect usage of ProjectBounds. I probably should try add a better comment for it's usage.

It's designed for when a rect has been transformed via a 3d transform, and then had the z information thrown away.

It finds the value of '?' where matrix.TransformPoint({x, y, ?}) = {x', y', 0} and then returns {x', y'}.

So given a point of {0, 0}, it finds ? to be -10, and then {0, 0, -10} transforms into {0, -10, 0}, which is the output we see.
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-01-18 19:49:01 PST
Comment on attachment 589756 [details] [diff] [review]
Use TransformBounds instead

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

Test?
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2012-01-18 19:49:11 PST
And thanks for the quick fix!
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2012-01-19 01:44:26 PST
I can get Aryeh's test imported over in bug 647323, in case that's helpful
Comment 8 Matt Woodrow (:mattwoodrow) 2012-01-19 15:33:17 PST
Created attachment 590021 [details] [diff] [review]
Use TransformBounds instead

Added testcase for this.
Comment 9 Matt Woodrow (:mattwoodrow) 2012-01-22 17:09:31 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/975ec4955ef9
Comment 10 Ed Morley [:emorley] 2012-01-23 11:53:11 PST
https://hg.mozilla.org/mozilla-central/rev/975ec4955ef9

Note You need to log in before you can comment on or make changes to this bug.