1377090 - Unify gfxMatrix with gfx::Matrix

Status: RESOLVED FIXED

(Core :: Graphics, enhancement)

Description

[Kartikaya Gupta (email:[e-mail])]

Following up from bug 1376782, we can unify gfxMatrix with gfx::Matrix, and reduce gfxMatrix to a lowly typedef.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=40d65f91578f7157862a80f2ca8e8bd51a61f95e

Note that these patches build on top of the ones in bug 1376782.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1377090 - Make the Matrix class templated so we can instantiate it with a double type.

This extracts a BaseMatrix template of which Matrix is now a particular
specialization. The BaseMatrix allows us to reuse the same code for
floats and doubles, much like the other "base" classes (BasePoint,
BaseRect, etc.).

Review commit: https://reviewboard.mozilla.org/r/153252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153252/

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1377090 - Move NudgeToInteger(double) into Tools.h for reuse.

Review commit: https://reviewboard.mozilla.org/r/153254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153254/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1377090 - Further generify the Matrix class and define MatrixDouble.

To allow MatrixDouble to be a drop-in replacement for gfxMatrix, it
needs to accept the "double" versions of Point, Rect, and Size. This
patch does that by adding some extra typedefs inside BaseMatrix to
abstract over that.

It also moves some function implementations into the .h file as they
don't need specialization. I left some function implementations in
the Matrix.cpp file:
- Rotation, because it is specialized for Float and Double, since it uses
  sinf/cosf vs sin/cos in the two implementations.
- The Matrix4x4 multiplication operator overload, because if I put it
  inside the BaseMatrix class declaration Matrix4x4 isn't defined yet
  and the compiler doesn't like it.

Review commit: https://reviewboard.mozilla.org/r/153256/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153256/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1377090 - Turn gfxMatrix into a typedef for MatrixDouble.

Most of this patch is updating a few places that use gfxMatrix to use
the equivalent-but-differently-named functions on MatrixDouble:
- Translate/Rotate/Scale get turned into PreTranslate/PreRotate/PreScale
- Transform(Point) gets turned into TransformPoint(Point)
- gfxMatrix::TransformBounds(gfxRect) gets turned into
  gfxRect::TransformBoundsBy(gfxMatrix).
- gfxMatrix::Transform(gfxRect) gets turned into
  gfxRect::TransformBy(gfxMatrix).
The last two functions are added in this patch as convenience wrappers
to gfxRect instead of Matrix.h because we don't want Matrix.h to "know"
about gfxRect (to avoid adding gecko dependencies on Moz2D). Once we
turn gfxRect into a typedef for RectDouble these will be eliminated
anyway.

Review commit: https://reviewboard.mozilla.org/r/153258/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153258/

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8882137 [details]
Bug 1377090 - Make the Matrix class templated so we can instantiate it with a double type.

https://reviewboard.mozilla.org/r/153252/#review158838

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 8882138 [details]
Bug 1377090 - Move NudgeToInteger(double) into Tools.h for reuse.

https://reviewboard.mozilla.org/r/153254/#review158842

::: gfx/thebes/gfxMatrix.cpp:146
(Diff revision 1)
> -}
> -
>  gfxMatrix&
>  gfxMatrix::NudgeToIntegers(void)
>  {
> -    NudgeToInteger(&_11);
> +    mozilla::gfx::NudgeToInteger(&_11);

nit: Since is a cpp file can we just do using namespace mozilla::gfx;?

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 8882139 [details]
Bug 1377090 - Further generify the Matrix class and define MatrixDouble.

https://reviewboard.mozilla.org/r/153256/#review158844

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8882138 [details]
Bug 1377090 - Move NudgeToInteger(double) into Tools.h for reuse.

https://reviewboard.mozilla.org/r/153254/#review158842

> nit: Since is a cpp file can we just do using namespace mozilla::gfx;?

We could, but this file gets deleted in part 4 anyway so there's not much point.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Rebased to m-c tip, still looking ok:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e71f82d9ac671c6996103da468c5236c3b87f547

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8882140 [details]
Bug 1377090 - Turn gfxMatrix into a typedef for MatrixDouble.

https://reviewboard.mozilla.org/r/153258/#review159590

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8882137 [details]
Bug 1377090 - Make the Matrix class templated so we can instantiate it with a double type.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153252/diff/1-2/

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8882138 [details]
Bug 1377090 - Move NudgeToInteger(double) into Tools.h for reuse.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153254/diff/1-2/

Comment 14

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8882139 [details]
Bug 1377090 - Further generify the Matrix class and define MatrixDouble.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153256/diff/1-2/

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8882140 [details]
Bug 1377090 - Turn gfxMatrix into a typedef for MatrixDouble.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153258/diff/1-2/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aecdc7b7c830
Make the Matrix class templated so we can instantiate it with a double type. r=bas
https://hg.mozilla.org/integration/autoland/rev/e6f68aad8e3d
Move NudgeToInteger(double) into Tools.h for reuse. r=bas
https://hg.mozilla.org/integration/autoland/rev/5468507e9a0a
Further generify the Matrix class and define MatrixDouble. r=bas
https://hg.mozilla.org/integration/autoland/rev/e21a77cab740
Turn gfxMatrix into a typedef for MatrixDouble. r=jrmuizel

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/aecdc7b7c830
https://hg.mozilla.org/mozilla-central/rev/e6f68aad8e3d
https://hg.mozilla.org/mozilla-central/rev/5468507e9a0a
https://hg.mozilla.org/mozilla-central/rev/e21a77cab740