Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Fix backface-visibility handling

RESOLVED FIXED in mozilla10

Status

()

Core
Layout
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

Tracking

unspecified
mozilla10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 566116 [details] [diff] [review]
Check the Z scale of the inverse instead

This is visibly broken on http://romaxa.bolshe.net/css3d/morphing/morphing-cubes.html
Attachment #566116 - Flags: review?(tterribe)
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.
Attachment #566116 - Flags: review?(tterribe) → review+
(Assignee)

Comment 2

6 years ago
(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.
(Assignee)

Comment 3

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/15aadd79c480
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/15aadd79c480
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10

Updated

6 years ago
Assignee: nobody → matt.woodrow
You need to log in before you can comment on or make changes to this bug.