Closed Bug 718809 Opened 12 years ago Closed 12 years ago

getBoundingClientRect returns incorrect results for certain 3D transforms

Categories

(Core :: DOM: CSS Object Model, defect)

Product:
Core
Component:
DOM: CSS Object Model
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12

People

(Reporter: ayg, Assigned: mattwoodrow)

References

Details

(Whiteboard: [parity-webkit])

Attachments

(1 file, 1 obsolete file)

Use TransformBounds instead
2.75 KB, patch
roc
: review+
Details | Diff | Splinter Review 
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.
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.
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.
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.
Assignee: nobody → matt.woodrow
Attached patch Use TransformBounds instead (obsolete) — — Splinter Review 
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.
Attachment #589756 - Flags: review?(roc)
Comment on attachment 589756 [details] [diff] [review]
Use TransformBounds instead

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

Test?
Attachment #589756 - Flags: review?(roc) → review+
I can get Aryeh's test imported over in bug 647323, in case that's helpful
Added testcase for this.
Attachment #589756 - Attachment is obsolete: true
Attachment #590021 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/975ec4955ef9
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: