809073 - NudgeToIntegers methods on gfxMatrix and gfx3DMatrix are unused and useless

Status: RESOLVED INVALID

(Core :: Graphics, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Unused according to:
  http://mxr.mozilla.org/mozilla-central/ident?i=NudgeToInteger&filter=

Useless because geometrically it makes no sense to round to integers the coefficients of a matrix. Think about what that does to a rotation matrix. It would make sense for a translation matrix but then it would be an inefficient way of doing the same on the translation vector.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think you searched on the wrong identifier:
http://dxr.mozilla.org/search.cgi?tree=mozilla-central&string=nudgetointegers

(In reply to Benoit Jacob [:bjacob] from comment #0)
> Useless because geometrically it makes no sense to round to integers the
> coefficients of a matrix.

Don't think geometrically :-). These are useful when we have a computation which, in ideal arithmetic, would give integer coefficients in the matrix, but due to floating-point precision limits gives non-integer coefficients in the matrix. NudgeToIntegers corrects those errors when they occur, at the cost of potentially introducing a small error when those errors did not occur.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> I think you searched on the wrong identifier:
> http://dxr.mozilla.org/search.cgi?tree=mozilla-central&string=nudgetointegers

Ah! ok :-)

> 
> (In reply to Benoit Jacob [:bjacob] from comment #0)
> > Useless because geometrically it makes no sense to round to integers the
> > coefficients of a matrix.
> 
> Don't think geometrically :-). These are useful when we have a computation
> which, in ideal arithmetic, would give integer coefficients in the matrix,
> but due to floating-point precision limits gives non-integer coefficients in
> the matrix. NudgeToIntegers corrects those errors when they occur, at the
> cost of potentially introducing a small error when those errors did not
> occur.

I still don't understand how this is useful on anything else than a translation or scaling matrix, but if we know in the code that a matrix is such a special matrix, presumably we shouldn't represent that transformation as a matrix in the first place?

Again, this will completely destroy the rotation component of a matrix, and turn it into something that isn't a rotation anymore.

For example take a 2D rotation about the origin, with angle pi/4

The matrix is

    0.707  -0.707
    0.707   0.707

If we apply NudgeToIntegers to this matrix, we get
    1      -1
    1       1

Which means that we have multiplied it by 1.414 i.e. we have applied a scaling with factor 1.414.

In what case is this desirable and doesn't this show highly visible undesirable effects?

For example here's a caller:
http://dxr.mozilla.org/mozilla-central/layout/svg/nsSVGPathGeometryFrame.cpp.html#l604

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Benoit Jacob [:bjacob] from comment #2)
> I still don't understand how this is useful on anything else than a
> translation or scaling matrix, but if we know in the code that a matrix is
> such a special matrix, presumably we shouldn't represent that transformation
> as a matrix in the first place?

The problem is we don't know in the code when this will happen.

Problems like this occur, for example, when we end up doing (the equivalent of) A*B*C (spread out over several functions) where in some situations A happens to equal B-inverse; ideally we'd always get exactly C out, but that's too hard. If C had integer components we at least want those components to be exact in the result.

> Again, this will completely destroy the rotation component of a matrix, and
> turn it into something that isn't a rotation anymore.
> 
> For example take a 2D rotation about the origin, with angle pi/4
> 
> The matrix is
> 
>     0.707  -0.707
>     0.707   0.707
> 
> If we apply NudgeToIntegers to this matrix, we get
>     1      -1
>     1       1

No we won't. 0.707 isn't close to 1 so it won't be touched. Check the definition of NudgeToInteger in Tools.h.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> No we won't. 0.707 isn't close to 1 so it won't be touched. Check the
> definition of NudgeToInteger in Tools.h.

Oh, got it -- I had misread the function as rounding all coefficients.

This is still rather destructive and successive rotation + nudgetointeger steps could transform a matrix that initially was a rotation into a matrix that isn't, for example a 2D rotation with angle 1e-3 is

   1 - 5e-7   -1e-3
   1e-3        1 - 5e-7

So if we apply NudgeToInteger to it, we get

   1          -1e-3
   1e-3        1

Which is no longer a rotation. Applying this sufficiently many times (suppose you have an animation where at each step you apply a small rotation and nudge) we would depart exponentially from being a rotation.

But this problem exists already without nudging due to float arithmetic having finite precision --- NudgeToIntegers only makes it worse by allowing it to happen with larger values (1e-6 vs 1.2e-7). At this point let's RESOLVE INVALID.