693520 - Fix backface-visibility handling

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Check the Z scale of the inverse instead

This is visibly broken on http://romaxa.bolshe.net/css3d/morphing/morphing-cubes.html

Comment 1

[Timothy B. Terriberry (:derf)]

Comment on attachment 566116 [details] [diff] [review]
Check the Z scale of the inverse instead

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

r+ with a couple minor suggestions.

::: gfx/thebes/gfx3DMatrix.cpp
@@ +836,5 @@
> +  gfxFloat det = Determinant();
> +  float _33 = _12*_24*_41 - _14*_22*_41 +
> +              _14*_21*_42 - _11*_24*_42 -
> +              _12*_21*_44 + _11*_22*_44;
> +  return (_33 < 0 && det >= 0) || (_33 >= 0 && det < 0);

I'm a little confused by the usage of >= 0 here, as that will make things pass the wouldn't have passed the Inverse()._33 < 0 test. But, I'd propose a much simpler test: _33*det < 0. It will have the same sign as _33/det, even if the magnitude is completely wrong.

::: gfx/thebes/gfx3DMatrix.h
@@ +298,5 @@
>    }
>  
>    gfx3DMatrix& Transpose();
>    gfx3DMatrix Transposed() const;
> + 

Whitespace-only change.

::: layout/base/nsDisplayList.cpp
@@ +2549,5 @@
>  
> +/* If the matrix is singular, or a hidden backface is shown, the frame won't be visible or hit. */
> +static bool IsFrameVisible(nsIFrame* aFrame, const gfx3DMatrix& aMatrix) 
> +{
> +  if (aMatrix.IsSingular()) {

It's a little sad that we can't combine this test with the backface test, since they both compute the determinant, and that's the bulk of the work here.

Comment 2

[Matt Woodrow (:mattwoodrow)]

(In reply to Timothy B. Terriberry (:derf) from comment #1)
> 
> It's a little sad that we can't combine this test with the backface test,
> since they both compute the determinant, and that's the bulk of the work
> here.

I think that if matrix maths ever starts showing up in profiles, then we can implement a dirty flag and start caching calculations like this.

Comment 3

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/15aadd79c480

Comment 4

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/15aadd79c480