[chromium] TransformationMatrix::to2dTransform seems unnecessary in LayerRendererChromium

• Component: WebCore Misc.
• Status: RESOLVED WONTFIX

[Shawn Singh]

Pasting from comment 9 on https://bugs.webkit.org/show_bug.cgi?id=80806
> The "3d part" of the transform doesn't affect the final x and y components of the quad in device space, only the z component.  In other words, for all values of quad:
> 
> transform.to2dTransform().mapQuad(quad) == transform.mapQuad(quad)

At first I believed that removing the 3d stuff in the transform will remove any intrinsic perspective that was also in it.  It turns out the 3d perspective properties are still retained on the bottom row of the matrix.  So even though the function is called to2dTransform(), it is actually still a 3d transform.  So I feel the function name is severely misleading.

As it stands, the function seems to be a gigantic no-op then, isn't it?  why do we need to do this operation?  It doesn't make anything faster; using 2d quads and points will already bypass the 3rd row and 3rd column, and everything else (inversion in particular) still operates on a 4x4 matrix.

We can mark this as WontFix, I'd like to be convinced first about why its actually necessary before doing that.  Otherwise, I think we should remove it entirely.

[Adrienne Walker]

(In reply to comment #0)
> Pasting from comment 9 on https://bugs.webkit.org/show_bug.cgi?id=80806
> > The "3d part" of the transform doesn't affect the final x and y components of the quad in device space, only the z component.  In other words, for all values of quad:
> > 
> > transform.to2dTransform().mapQuad(quad) == transform.mapQuad(quad)
> 
> At first I believed that removing the 3d stuff in the transform will remove any intrinsic perspective that was also in it.  It turns out the 3d perspective properties are still retained on the bottom row of the matrix.  So even though the function is called to2dTransform(), it is actually still a 3d transform.  So I feel the function name is severely misleading.

Can you clarify what you mean by "perspective"?

> As it stands, the function seems to be a gigantic no-op then, isn't it?  why do we need to do this operation?  It doesn't make anything faster; using 2d quads and points will already bypass the 3rd row and 3rd column, and everything else (inversion in particular) still operates on a 4x4 matrix.

The function is not a no-op.  For some values of transform,

transform.to2dtransform().inverse().mapQuad(quad) != transform.inverse().mapQuad(quad)

> We can mark this as WontFix, I'd like to be convinced first about why its actually necessary before doing that.  Otherwise, I think we should remove it entirely.

This is WontFix.

[Adrienne Walker]

(In reply to comment #1)

> > As it stands, the function seems to be a gigantic no-op then, isn't it?  why do we need to do this operation?  It doesn't make anything faster; using 2d quads and points will already bypass the 3rd row and 3rd column, and everything else (inversion in particular) still operates on a 4x4 matrix.
> 
> The function is not a no-op.  For some values of transform,
> 
> transform.to2dtransform().inverse().mapQuad(quad) != transform.inverse().mapQuad(quad)

Haha, I realize this is exactly the same as the first thing you quote from me.  I think maybe I had some thought that not dropping the 3d transform part of the matrix would result in a different value when you projected back since you'd flattened that point to 2d and inflated in 2d.

Tests would certainly be helpful at this point in the conversation.

[Shawn Singh]

(In reply to comment #2)
> (In reply to comment #1)
> 
> Haha, I realize this is exactly the same as the first thing you quote from me.  I think maybe I had some thought that not dropping the 3d transform part of the matrix would result in a different value when you projected back since you'd flattened that point to 2d and inflated in 2d.
> 
> Tests would certainly be helpful at this point in the conversation.

Well, there might be a correctness difference on the inverse.   So the question I see is:

is it true that  transform.to2dTransform().inverse().mapPoint() == transform.inverse().projectQuad() ??

And, assuming the answer is "no they are not equal", then we should ask which one is correct to use for our code.

As per offline discussion with Enne, I'll toy with some example tests on this, and report back answering these questions.

[Adrienne Walker]

Based on a simple example, I think my intuition is correct in that inverse.mapQuad != to2DTransform.inverse.mapQuad.  This is because in the process of taking the inverse, the components in the 3rd column of the original matrix end up affecting components other than z in the final matrix.

I think I have also convinced myself that using to2DTransform + inverse.mapQuad is equivalent to just using inverse.projectQuad.

My inclination is to leave this code as-is, given that transform2d + mapQuad is cheaper than projectQuad because it ignores transforming the z-component of every point.

[Shawn Singh]

Spoke offline with David.  He mentioned that the primary reason it was used was because otherwise many of the matrices being used were not invertible.  He also said it could be worth looking into why that is the case, since it might point to a different issue that should be fixed.

That, combined with Enne's responses, definitely means this should be WontFix.

Thanks for your time David and Enne =)