Matrix3: removed .scale(), .rotate(), and .translate()

[WestLangley]

See the discussion in #24731.

We rotate objects in three.js -- not matrices.

I added these methods to emulate a CSS-like API. I do not think these methods are used, and I can no longer justify their existence.

[WestLangley]

Oops. SVGLoader() uses these methods extensively...

[Mugen87]

I'm not in favor of removing such basic math methods at this point of the library's maturity level. I think we should close this PR and #24731 since these methods were introduced for specific (valid) use cases.

[WestLangley]

It appears SVGLoader found a use for them.

@yomboprime You appear to understand these methods. Are you able to help suggest proper descriptions for the docs?

How do the methods relate to CSS transforms?

[yomboprime]

It appears SVGLoader found a use for them.

@yomboprime You appear to understand these methods. Are you able to help suggest proper descriptions for the docs?

How do the methods relate to CSS transforms?

A matrix of 3x3 elements, as used here, is an affine transform in 2D. That is, it is equivalent to a rotation, scale and translation in 2D. The translation is the vector ( elements[ 2 ], elements[ 5 ] ).

scale: Multiplies the matrix scale by the parameters, leaving the translation intact.
rotate: Constructs a rotation matrix with angle theta and scale 1, leaving the translation intact.
translate: This method adds a translation ( tx, ty ) that is represented in the frame (space) of the current transform. i.e. ( tx, ty ) is 'local'. To have it global, The method should just make elements[ 2 ] += tx; elements[ 5 ] += ty;.

I've not used CSS 2D transforms but I guess they use the matrices this way.

[yomboprime]

I think the problem with #24731 is that Matrix3 can be interpreted as an affine transform in 2D (as these 3 methods do), or a scale + rotation in 3D, without translation. Matrix4 is an affine transform in 3D.

[yomboprime]

By the way, I initially had my own methods in SVGLoader, and @mrdoob suggested to use Matrix3 instead.

[WestLangley]

Thanks @yomboprime. ... Reverse engineering what I did in #11863...

.scale( x, y ): premultiplies this matrix by the scale matrix

x 0 0
0 y 0
0 0 1

.rotate( theta ): premultiplies this matrix by the rotation matrix

 c  s  0
-s  c  0
 0  0  1

.translate( x, y ): premultiplies this matrix by the translation matrix

1 0 x
0 1 y
0 0 1

[WestLangley]

By the way, I initially had my own methods in SVGLoader, and @mrdoob suggested to use Matrix3 instead.

Interesting... So I am trying to decide if that is where they belong... Are SVG transform conventions the same as in CSS? The nomenclature sure is CSS-like.

Why is the rotation sign flipped, as compared to Matrix4.makeRotationZ()?

[yomboprime]

By the way, I initially had my own methods in SVGLoader, and @mrdoob suggested to use Matrix3 instead.

Interesting... So I am trying to decide if that is where they belong... Are these methods CSS-specific? The nomenclature sure is CSS-like.

They are not CSS specific, they are general matrix operations. Of course if Matrix3 is not used as a 2D transform anywhere else in the code base, they could be moved to SVGLoader. I can take care of that and test if you wish.

Edit: I guess the texture matrices are 2D, so the 3 methods can be used with them.

Why is the rotation sign flipped, as compared to Matrix4.makeRotationZ()?

Probably because it is a premultiplication? I mean, perhaps the sign gets inverted.

[WestLangley]

In SVGLoader() you had to negate the angle to get the method to work correctly for your use case:

three.js/examples/jsm/loaders/SVGLoader.js

Line 1485 in ecd0bf6

angle = - array[ 0 ] * Math.PI / 180;

For the use case in the three.js example, the method works correctly.

three.js/examples/webgl_materials_texture_rotation.html

Line 134 in ecd0bf6

.rotate( API.rotation ) // I don't understand how rotation can preceed scale, but it seems to be required...

//

These are not general math methods. They are utility methods for a specific use case.

[yomboprime]

In SVGLoader() you had to negate the angle to get the method to work correctly for your use case:

Take also in consideration that the Y axis in SVG is inverted w.r.t. the Threejs one:

three.js/examples/webgl_loader_svg.html

Line 168 in ecd0bf6

group.scale.y *= - 1;

That could explain why I did negate the angle.

[yomboprime]

That could explain why I did negate the angle.

I've seen that the angle in SVG is clockwise, so that explains the angle negation. Did not remember that.

[mrdoob]

Sorry for the delay...

I would also like to keep these methods. Seems like the issue in #24731 is with translation()?

@eXponenta do you have any suggestions on how these methods should behave so they make sense to you?

[eXponenta]

@mrdoob So, i think need keep texture-specific implementation as special TextureMatrix, or SCGMatrix for avoid confusing, and change behaviour on to classic matrix operations.

For us not make sence anymore how computations is doing now, we use a manual implementation, but counterintuitively for application supporting and we always marks matrix3 usage as unsafe with link to issue.

I not know how many peoples use a Matrix3 for regular matrix computation on 2D space, but looks like issue unpopular and our case is statistically insignificant.

[yomboprime]

Maybe renaming the 3 methods to adhere and operate as the Matrix4 nomenclature. Perhaps including "2D".

[mrdoob]

That sounds reasonable to me...

@WestLangley @eXponenta what do you think?

[yomboprime]

That could explain why I did negate the angle.

I've seen that the angle in SVG is clockwise, so that explains the angle negation. Did not remember that.

About the angle negation, I think it is rather because the Y axis is inverted in SVG (the model scale.y is negated in the example: group.scale.y *= - 1;)

[WestLangley]

There seems to be support for 2D matrix transforms, so I propose adding the following methods which parallel the similarly-named Matrix4 methods:

// for 2D Transforms

makeTranslation( x, y ) {

	this.set(
		1, 0, x,
		0, 1, y,
		0, 0, 1
	);

	return this;

}

makeRotation( theta ) {

	// clockwise (but I think counter-clockwise is a better convention for three.js)

	const c = Math.cos( theta );
	const s = Math.sin( theta );

	this.set(
		c, s, 0,
		- s, c, 0,
		0, 0, 1
	);

	return this;

}

makeScale( x, y ) {

	this.set(
		x, 0, 0,
		0, y, 0,
		0, 0, 1
	);

	return this;

}

IMO, the above methods are definitely appropriate. /ping @eXponenta .

If these methods were added, then then existing methods could be rewritten:

scale( sx, sy ) {

	this.premultiply( _m3.makeScale( sx, sy ) );

	return this;

}

rotate( theta ) {

	this.premultiply( _m3.makeRotation( theta ) );

	return this;

}

translate( tx, ty ) {

	this.premultiply( _m3.makeTranslation( tx, ty ) );

	return this;

}

But these existing methods do not belong here, IMO. We rotate objects in three.js. We do not rotate matrices.

Plus, they are application-specific, and depend on the handedness of the coordinate frame.

[yomboprime]

But these existing methods do not belong here, IMO. We rotate objects in three.js. We do not rotate matrices.

I agree, we should delete the old methods and use the new ones in SVGLoader.

[eXponenta]

@WestLangley i prefer gl-matrix implementation, looks like your implementation is same as one.

[WestLangley]

According to the inline comments, the gl-matrix function translate( out, a, v )

translates a mat3 by the given vector.

In three.js, we do not "translate matrices"; we translate objects and geometries.

In any event, the gl-matrix translate() function is equivalent to the following in three.js:

translate( tx, ty ) {

	this.multiply( _m3.makeTranslation( tx, ty ) ); // post-multiply

	return this;

}

That is different than pre-multiplying, as is done in the three.js method translate( tx, ty ).

The gl-matrix method fromTranslation( out, v ) is equivalent to the three method makeTranslation( x, y ), which I proposed above.

[Mugen87]

After merging #24985, can this PR and #24731 be closed now?

[WestLangley]

After merging #24985, can this PR and #24731 be closed now?

No, still working on this issue...