incomplete rendering with 3d-transform

RESOLVED FIXED in mozilla33

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: gal, Assigned: mattwoodrow)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
mozilla33
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
A div is drawn here with z position 10. It rotates around and we render it incompletely (disappears, flickering) whenever it intersects with the view plane. This might be a clipping issue.
(Reporter)

Comment 1

4 years ago
mattwoodrow: wonder if that’s a regression
mattwoodrow: I can take a look later today
mattwoodrow: The fact that it dissapears in chunks that are aligned to the non-transformed space is interesting
mattwoodrow: sounds like we’re clipping/adjusting the visible region of the pre-transform layer incorrectly
Assignee: nobody → matt.woodrow
Keywords: regressionwindow-wanted
(Assignee)

Comment 2

4 years ago
This is a bug with nsDisplayTransform::ComputeVisibility. We're taking the visible area of the window and converting it into the coordinate space of the children and getting really wrong results.

These sort of bugs were meant to be fixed when we added gfx3DMatrix:UntransformBounds, but I guess that still isn't sufficient.

Comment 3

4 years ago
Created attachment 8448466 [details]
non animation html

it appears/disappears when resize width of browser

Comment 4

4 years ago
I can reproduce in Firefox10 on windows7. so, I think this is not a regression.
(Assignee)

Comment 5

4 years ago
Thanks Alice!

Quite a few bugs here actually.

I've constructed a standalone testcase (using eigen) for the incorrect visibility calculations and sent it to bjacob to see if he can help me figure out where we're going wrong. I think it's because we're failing to account for the 4th component (perspective/w) of the transformed vectors.

We're also failing to respect all the preserve-3d properties, since we generate multiple frames for the #o frame and get confused.
(Assignee)

Comment 6

4 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
 
> We're also failing to respect all the preserve-3d properties, since we
> generate multiple frames for the #o frame and get confused.

Actually, this is fine, it's just a really weird testcase. We've got transform-style:preserve-3d, overflow:hidden, and perspective: 536.929px; all on the same frame (#o).

overflow:hidden disables preserve-3d (http://www.w3.org/TR/css-transforms-1/#propdef-transform-style), and perspective is only applied to child elements so doesn't really make sense in the middle of a preserve-3d chain.
(Assignee)

Comment 7

4 years ago
Created attachment 8451973 [details] [diff] [review]
Only include un-transformed points that are above the w=0 plane

Roc, this has problems with nsLayoutUtils::TransformPoints which assumes that if one of the points can be transformed then all of them can. This isn't true when we're going backwards through a projective transform.

I've left the code as-is for now, but it has the potential to get some crazy numbers for points that can't be mapped to any value in the destination coordinate space.
Attachment #8451973 - Flags: review?(bjacob)
Flags: needinfo?(roc)
(Assignee)

Comment 8

4 years ago
I see that the nsLayoutUtils::TransformPoints interface matches the GeometryUtils one proposed by cssom-view, we might need a change to spec too then.
Here is a quick explanation of what is going on here geometrically.

For the sake of getting a simple example that still reflects the core issue at hand, let us work in 1 dimension. So our space is a line, with a single coordinate, "x". Let us use uppercase letters for homogeneous coordinates: (X, W). I choose the letter W for the last homogeneous coord so that we feel on familiar grounds here. Let us choose as usual W=0 for the projective infinity, so that as usual the correspondence between the affine x coordinate and projective (X, W) coordinates is given by
t
    x = X / W.

Note that the projective 1D space, being the set of all lines through the origin in the (X, W) plane, is a circle with opposite points identified, which is again a circle.

Another way to see that the projective 1D space is a circle, is to see it as the line with one "point at infinity" adjoined, and the two "ends" of the line glued together at that point.

In our affine 1D world, which is a line, a "rect" is just an interval, [a .. b]. So we are used to representing rects by just giving the two endpoints. We are used, then, to transforming rects by just transforming endpoints. No more information than the resulting two points, is needed to encode the transformed rect.

In our projective 1D world, which is a circle, a "rect" is still going to be an interval, i.e. a set of points contained in between two endpoints on the circle. But it is no longer true that a rect is uniquely determined by its two endpoints. Indeed, on a circle, there are two ways to go from one point to another: clockwise or counter-clockwise.

So our whole approach of transforming rects by just transforming the endpoints and constructing the result rect from those transformed endpoints, breaks down. In order to properly transform rects under projective transformations, we would need to carry some extra information to tell which side is inside.

The approach of the patch here is to cut down the domain of homogeneous coordinates: instead of allowing arbitrary (X, W) in the entire plane, we now restrict to the W >= 0 half-plane.

This allows to remove the above-described ambiguity as follows. Take two points in the projective 1D space, say (X1, W1) and (X2, W2). Normalize these coords so that they have norm 1, so we are looking at points on the unit circle in the (X, W) plane. Normally, there is still an ambiguity as (X1, W1) and (-X1, -W1) both satisy this condition and represent the same point in projective space. But that's where our W>=0 restriction kicks in: it removes one of these two candidates. Thus each point in projective space is now represented by only exactly one (X, W) homogeneous coordinate, except for the point at infinity (W=0).

Concretely, we've now represented the projective 1D space as a half-circle with its two endpoints glued. This gives us a well-defined choice of what's the "rect" delimited by two endpoints: just go in the direction that doesn't go through the end-points of the half-circle.

What, then, if a projective transform is expressed by a matrix that acts on (X, W) in a way that does not preserve the W>=0 condition? The approach of the patch at hand there is then to just clamp the edge to stop before W becomes negative.

Actually, this patch does something more hacky, less meaningful than just described. Ideally, it would stop at exactly W=0 and deal properly with points-at-infinity thus obtained. But instead, because it is a bunch of code that is not designed to handle points-at-infinity gracefully (i.e. that code expects to be able to represent the result rect in affine coordinates), it has to stop not at W=0 but "just before": at a very small positive W like W=0.000001 or some such.

That's the ugly, arbitrary, fragile part. I'm going to r+ because it fixes a concrete bug already and Matt said it's what Chromium does. But please, file a follow-up bug about that. That follow-up bug should at least be about properly clamping at W=0 and dealing with the resulting infinities (e.g. the transformed rect could be a half-plane or a wedge between two half-lines). Ideally though, we would find a way to deal with arbitrary projective transforms.
Comment on attachment 8451973 [details] [diff] [review]
Only include un-transformed points that are above the w=0 plane

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

Haven't carefully checked all the math, just the parts that looked involved enough to have room to let typos slip in. Basically, I'm trusting that we have enough reftests for any gross wrong behavior to be caught... let me know if you would like more careful reviewing of specific parts.

::: gfx/layers/apz/src/APZCTreeManager.cpp
@@ +1103,5 @@
>  
>    AsyncPanZoomController* result = nullptr;
>    // This walks the tree in depth-first, reverse order, so that it encounters
>    // APZCs front-to-back on the screen.
> +  if (hitTestPointForChildLayers.AbovePerspectivePlane()) {

I would say HasPositiveWCoord() to be very specific and down-to-earth here.

AbovePerspectivePlane has two issues: it's a 3D "hyperplane" not a 2D plane, and it's called "at infinity" not "perspective" afaik.

::: gfx/thebes/gfx3DMatrix.cpp
@@ +774,5 @@
> +  // the positive side of the w=0 plane as possible.
> +
> +  // Since we know what we want the w component to be, we can rearrange the
> +  // interpolation equation and solve for t.
> +  float w = 0.00001f;

Please file that follow-up bug as described above, and reference it from this comment. This probably deserves a FIXME since it could plausibly be a cause of bugs in the future --- either excessive clipping or loss of precision.
Attachment #8451973 - Flags: review?(bjacob) → review+
(Assignee)

Comment 11

4 years ago
Awesome explanation Benoit, thanks for adding that.
(Assignee)

Updated

4 years ago
Blocks: 1035611
(Assignee)

Updated

4 years ago
Keywords: regressionwindow-wanted
(Assignee)

Comment 12

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/15b924cf6eab
Thanks bjacob!

We don't really want to deal with rects that transform to unbounded shapes. I'm pretty sure that is never going to be useful, it's always going to be confusing, and we don't want our geometric data types to have to handle those cases. So I think we need to always choose the projection that excludes points-at-infinity.

I think this means (but I could be wrong) that in nsLayoutUtils::TransformPoints, we should return an error if any two of the transformed points have differently-signed W values or if any one of the transformed points has a zero W value. I.e. if they're all negative or all positive I guess we're OK. Is that right?
Flags: needinfo?(roc)
https://hg.mozilla.org/mozilla-central/rev/15b924cf6eab
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Assignee)

Comment 15

4 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #13)
> Thanks bjacob!
> 
> We don't really want to deal with rects that transform to unbounded shapes.
> I'm pretty sure that is never going to be useful, it's always going to be
> confusing, and we don't want our geometric data types to have to handle
> those cases. So I think we need to always choose the projection that
> excludes points-at-infinity.
> 
> I think this means (but I could be wrong) that in
> nsLayoutUtils::TransformPoints, we should return an error if any two of the
> transformed points have differently-signed W values or if any one of the
> transformed points has a zero W value. I.e. if they're all negative or all
> positive I guess we're OK. Is that right?

That's one option, and probably is correct re the current spec.

I'm not sure what the actual uses cases for this interface are, but the way gecko uses it internally this wouldn't be useful behaviour. Since our input rect is usually the window size, it's quite common to have some of the input points not exist in the transformed plane. We only ever need the rectangle (not a quad) and we can restrict it to the bounds of the child element so we get around it that way.

Updated

4 years ago
Depends on: 1036657
Blocks: 1064293

Updated

3 years ago
Depends on: 1102218

Updated

3 years ago
Depends on: 1123315

Updated

3 years ago
Depends on: 1131961

Updated

2 years ago
Depends on: 1187199

Updated

2 years ago
Depends on: 1217012
You need to log in before you can comment on or make changes to this bug.