Bug 85808 - [chromium] Non-preserves-3d requires explicit orthographic transform when computing screen-space transform
Summary: [chromium] Non-preserves-3d requires explicit orthographic transform when com...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shawn Singh
URL:
Keywords:
Depends on: 84195
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-07 10:34 PDT by Shawn Singh
Modified: 2012-08-06 10:43 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.35 KB, patch)
2012-08-02 10:30 PDT, Shawn Singh
no flags Details | Formatted Diff | Diff
Patch for landing (1.15 KB, patch)
2012-08-06 09:40 PDT, Shawn Singh
no flags Details | Formatted Diff | Diff
Patch for landing (9.35 KB, patch)
2012-08-06 09:44 PDT, Shawn Singh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shawn Singh 2012-05-07 10:34:07 PDT
Consider the following case:
   parent layer A has a perspective projection
   descendant surface B does not preserve 3d, thus flattening its children
   children of B have 3-d transforms

In this case, children of B should be flattened to surface B, before the perspective projection is applied.
However, in the current code, this flattening is accidentally skipped when computing the screen-space transform, and therefore it computes incorrectly.

Thanks to danakj@ for finding this problem. =)

The fix is simple, when there is a surface that does not preserve 3d, and it does not have a perspective projection already, then it should be using an orthographic projection instead.  Because transforms accumulate over the entire hierarchy, this orthographic projection actually needs to be made explicit, so that children are correctly flattened before ancestor transforms do anything else.
Comment 1 Shawn Singh 2012-05-07 16:52:23 PDT
It turns out that adding the orthographic portion of the screen-space transform (which I'm pretty sure is correct) breaks one layout test that seems to be malformed in the first place.  The fix for that is in bug 84195.
Comment 2 Shawn Singh 2012-08-02 10:18:39 PDT
So I finally have a working fix.  Patch coming in a moment, no obvious regressions from unit tests and layout tests on mac.

Instead of using orthographic projection, it works more correctly to use the "flattening" operation, which is similar to an orthographic projection, but has some blatant differences, too:

- A canonical orthographic matrix post-multiplied to a transform would effectively set the matrix's 3rd column to (0, 0, 0, 0).   -- yes, 3rd element is zero here.

- The flatten operation effectively sets the matrix's 3rd row AND 3rd colum to (0, 0, 1, 0).

I tried for a while to swallow why exactly the flatten operation is more correct, but I can't justify it to myself 100% yet.  The best I could come up with is that x and y values behave exactly like an orthographic projection, while the z component of the matrix is overridden so that it preserves the z value that is input to this transform.

I experimented using an orthographic projection instead of flatten operation, andin that case layer sorting seems to break, for example, on http://www.webkit.org/blog-files/3d-transforms/transform-style.html

I narrowed it down to the difference in the value of m33() for these two transforms.  when m33() is zero for the orthographic projection, the layer sorting seems to break.  When it's 1 (flattening operation), the layer sorting works.  So my best guess is that the flatten operation is necessary for our layer sorter, because it preserves the z-values of points on the subtree that is being flattened.  At the same time, the transform remains "correct" for x and y, only because we know that inputs and outputs are 2d, even though 3d things can happen to them.

Any thoughts?
Comment 3 Shawn Singh 2012-08-02 10:30:29 PDT
Created attachment 156117 [details]
Patch
Comment 4 Vangelis Kokkevis 2012-08-02 13:07:38 PDT
(In reply to comment #2)
> 
> I experimented using an orthographic projection instead of flatten operation, andin that case layer sorting seems to break, for example, on http://www.webkit.org/blog-files/3d-transforms/transform-style.html
> 
> I narrowed it down to the difference in the value of m33() for these two transforms.  when m33() is zero for the orthographic projection, the layer sorting seems to break.  When it's 1 (flattening operation), the layer sorting works.  So my best guess is that the flatten operation is necessary for our layer sorter, because it preserves the z-values of points on the subtree that is being flattened.  At the same time, the transform remains "correct" for x and y, only because we know that inputs and outputs are 2d, even though 3d things can happen to them.
> 
> Any thoughts?

Interesting. If preserves 3D isn't set shouldn't we be skipping sorting of sublayers altogether?
Comment 5 Adrienne Walker 2012-08-02 14:14:23 PDT
Comment on attachment 156117 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156117&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCMathUtil.cpp:371
> +    //  - For z values, the new transform overrides any effect that the transform had on
> +    //    z, and instead it preserves the z value for any points that are transformed.

Can you explain why z values are preserved? Shouldn't everything flatten to z=0 on the surface?
Comment 6 Shawn Singh 2012-08-02 14:54:54 PDT
(In reply to comment #5)
> (From update of attachment 156117 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156117&action=review
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCMathUtil.cpp:371
> > +    //  - For z values, the new transform overrides any effect that the transform had on
> > +    //    z, and instead it preserves the z value for any points that are transformed.
> 
> Can you explain why z values are preserved? Shouldn't everything flatten to z=0 on the surface?

Are you asking why they should be preserved?  Ian and I believe that it is an intentional hack that allows us to use the transform for two purposes simultaneously:  x,y,w would be used for rendering, and they are correctly orthographically projected, and z would be used for sorting; whatever z-value the subtree would have produced is preserved by having this flattening here.

Or, if you're asking how this transform actually preserves z, it's because of the 3rd row being set to 0, 0, 1, 0:

So  if we say  p' = flattenedA * p

then   p'.z =  0*p.x + 0*p.y + 1*p.z + 0*p.w 
so p'.z = p.z.

Suppose we have additional matrices that were post-multiplied (i.e. the subtree has more transforms applied after the sublayer flattened).... like p' = flattenedA * B * C * D * p

then  p'.z =  z-value of (B * C * D * p).

i.e. it preserves whatever z-value is given to flattenedA.  It seems to me that is (almost) exactly what we want for layer sorting.
Comment 7 Adrienne Walker 2012-08-02 14:59:09 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 156117 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=156117&action=review
> > 
> > > Source/WebCore/platform/graphics/chromium/cc/CCMathUtil.cpp:371
> > > +    //  - For z values, the new transform overrides any effect that the transform had on
> > > +    //    z, and instead it preserves the z value for any points that are transformed.
> > 
> > Can you explain why z values are preserved? Shouldn't everything flatten to z=0 on the surface?
> 
> Are you asking why they should be preserved?  Ian and I believe that it is an intentional hack that allows us to use the transform for two purposes simultaneously:  x,y,w would be used for rendering, and they are correctly orthographically projected, and z would be used for sorting; whatever z-value the subtree would have produced is preserved by having this flattening here.

Yeah, I'm asking why.

When do layers in non-preserves-3d contexts need to be sorted?
Comment 8 Shawn Singh 2012-08-02 15:12:50 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (From update of attachment 156117 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=156117&action=review
> > > 
> > > > Source/WebCore/platform/graphics/chromium/cc/CCMathUtil.cpp:371
> > > > +    //  - For z values, the new transform overrides any effect that the transform had on
> > > > +    //    z, and instead it preserves the z value for any points that are transformed.
> > > 
> > > Can you explain why z values are preserved? Shouldn't everything flatten to z=0 on the surface?
> > 
> > Are you asking why they should be preserved?  Ian and I believe that it is an intentional hack that allows us to use the transform for two purposes simultaneously:  x,y,w would be used for rendering, and they are correctly orthographically projected, and z would be used for sorting; whatever z-value the subtree would have produced is preserved by having this flattening here.
> 
> Yeah, I'm asking why.
> 
> When do layers in non-preserves-3d contexts need to be sorted?

Yeah, good question... this is also to answer Comment #4.

Actually the situation you guys are thinking of is not the situation we would be dealing with here.

The situation we're considering here is:  (1) an ancestor layer has preserve-3d==false.  Then, a bunch of layers in the subtree are in a 3d-rendering context, and need to be sorted with each other.

The transform these layers use to compute their z stuff for sorting is the drawTransform... and the drawTransform would include the ancestor's transform as part of the drawTransform.  That is where z-values would get lost if we used a pure orthographic projection -- at the ancestor.

By doing this flattening operation instead, we don't loose the z-values that we needed for correct sorting.

Does that answer your question?  =)
Comment 9 Adrienne Walker 2012-08-02 16:11:12 PDT
(In reply to comment #8)
> (In reply to comment #7)
> >
> > When do layers in non-preserves-3d contexts need to be sorted?
> 
> Yeah, good question... this is also to answer Comment #4.
> 
> Actually the situation you guys are thinking of is not the situation we would be dealing with here.
> 
> The situation we're considering here is:  (1) an ancestor layer has preserve-3d==false.  Then, a bunch of layers in the subtree are in a 3d-rendering context, and need to be sorted with each other.
> 
> The transform these layers use to compute their z stuff for sorting is the drawTransform... and the drawTransform would include the ancestor's transform as part of the drawTransform.  That is where z-values would get lost if we used a pure orthographic projection -- at the ancestor.

Ok, so keeping m33=1 and not forcing z=0 is only for the sublayer matrix flattening.  The screen space transform doesn't need this, but it's not problematic to have it.
Comment 10 Shawn Singh 2012-08-02 17:25:42 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > >
> > > When do layers in non-preserves-3d contexts need to be sorted?
> > 
> > Yeah, good question... this is also to answer Comment #4.
> > 
> > Actually the situation you guys are thinking of is not the situation we would be dealing with here.
> > 
> > The situation we're considering here is:  (1) an ancestor layer has preserve-3d==false.  Then, a bunch of layers in the subtree are in a 3d-rendering context, and need to be sorted with each other.
> > 
> > The transform these layers use to compute their z stuff for sorting is the drawTransform... and the drawTransform would include the ancestor's transform as part of the drawTransform.  That is where z-values would get lost if we used a pure orthographic projection -- at the ancestor.
> 
> Ok, so keeping m33=1 and not forcing z=0 is only for the sublayer matrix flattening.  The screen space transform doesn't need this, but it's not problematic to have it.


The screen space transform does need it.  That was the whole point of this bug.

Dana had a lovely repro for this, I have it locally on my machine.  How about I convert it into a layout test for one more "layer" of coverage (pun definitely intended)?  This was related to the bug you reported, where the android apps page was clipping the 3d layer when resizing the window.  http://code.google.com/p/chromium/issues/detail?id=121743
Comment 11 Adrienne Walker 2012-08-02 17:48:29 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #7)
> > > >
> > > > When do layers in non-preserves-3d contexts need to be sorted?
> > > 
> > > Yeah, good question... this is also to answer Comment #4.
> > > 
> > > Actually the situation you guys are thinking of is not the situation we would be dealing with here.
> > > 
> > > The situation we're considering here is:  (1) an ancestor layer has preserve-3d==false.  Then, a bunch of layers in the subtree are in a 3d-rendering context, and need to be sorted with each other.
> > > 
> > > The transform these layers use to compute their z stuff for sorting is the drawTransform... and the drawTransform would include the ancestor's transform as part of the drawTransform.  That is where z-values would get lost if we used a pure orthographic projection -- at the ancestor.
> > 
> > Ok, so keeping m33=1 and not forcing z=0 is only for the sublayer matrix flattening.  The screen space transform doesn't need this, but it's not problematic to have it.
>  
> The screen space transform does need it.  That was the whole point of this bug.

I was asking about m33=1 vs. m33=0, not about whether flattening needs to be applied.  Just trying to understand the math.
Comment 12 Shawn Singh 2012-08-02 22:15:57 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > (In reply to comment #7)
> > > > >
> > > > > When do layers in non-preserves-3d contexts need to be sorted?
> > > > 
> > > > Yeah, good question... this is also to answer Comment #4.
> > > > 
> > > > Actually the situation you guys are thinking of is not the situation we would be dealing with here.
> > > > 
> > > > The situation we're considering here is:  (1) an ancestor layer has preserve-3d==false.  Then, a bunch of layers in the subtree are in a 3d-rendering context, and need to be sorted with each other.
> > > > 
> > > > The transform these layers use to compute their z stuff for sorting is the drawTransform... and the drawTransform would include the ancestor's transform as part of the drawTransform.  That is where z-values would get lost if we used a pure orthographic projection -- at the ancestor.
> > > 
> > > Ok, so keeping m33=1 and not forcing z=0 is only for the sublayer matrix flattening.  The screen space transform doesn't need this, but it's not problematic to have it.
> >  
> > The screen space transform does need it.  That was the whole point of this bug.
> 
> I was asking about m33=1 vs. m33=0, not about whether flattening needs to be applied.  Just trying to understand the math.


Oh I see what you're asking - I think you're right, the screen space transform doesn't need it.  but I'll try it out and make sure.
Comment 13 Adrienne Walker 2012-08-03 09:21:22 PDT
Comment on attachment 156117 [details]
Patch

R=me.  Ok, I think I'm convinced by this.  Thanks for the explanation about why the sublayer matrix can't have m33=0.
Comment 14 Shawn Singh 2012-08-06 09:40:03 PDT
Created attachment 156710 [details]
Patch for landing
Comment 15 WebKit Review Bot 2012-08-06 09:41:14 PDT
Comment on attachment 156710 [details]
Patch for landing

Rejecting attachment 156710 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
1 cwd: /mnt/git/webkit-commit-queue/

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 FAILED at 3.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/ChangeLog.rej
patching file Source/WebKit/chromium/ChangeLog
Hunk #1 FAILED at 3.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/chromium/ChangeLog.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/13443446
Comment 16 Shawn Singh 2012-08-06 09:43:44 PDT
> > I was asking about m33=1 vs. m33=0, not about whether flattening needs to be applied.  Just trying to understand the math.
> 
> 
> Oh I see what you're asking - I think you're right, the screen space transform doesn't need it.  but I'll try it out and make sure.


So, it looks like the screenSpaceTransform does actually require the m33==1 part, too.  The reason is that the inverse screen-space transform is being used by CCOcclusionTracker, with a project operation.  The projection would be incorrect if the z-value is not correctly preserved.

So I guess that's additional (and probably more important) explanation about why we need to do this special flatten operation, rather than the pure orthographic projection.

Nothing changes in the patch based on this new point, it's just about our conceptual understanding.  Thanks for reviewing!
Comment 17 Shawn Singh 2012-08-06 09:44:23 PDT
Created attachment 156713 [details]
Patch for landing
Comment 18 WebKit Review Bot 2012-08-06 10:43:18 PDT
Comment on attachment 156713 [details]
Patch for landing

Clearing flags on attachment: 156713

Committed r124782: <http://trac.webkit.org/changeset/124782>
Comment 19 WebKit Review Bot 2012-08-06 10:43:22 PDT
All reviewed patches have been landed.  Closing bug.