patch(matrix): expose calcPlaneRotation

[ShaMan123]

Motivation

I need to calc angles of planes/matrices in certain cases.
I prefer to avoid decomposition because in most cases I need the radian value and not the degree.
In addition the rest of the calculation is not needed and is wasteful in terms of perf.
Also I believe fabric can and should reduce its dependancy on decomposition for all the known and unknown reasons, some I discovered in #8767
This is a small step in that direction.

Description

Expose calcPlaneRotation and use it across fabric.
There a number of usages I left in functions I dislike and that I have changed in several open PRs.
I do not want conflicts nor do I want to patch them because they need extensive work.

Changes

Gist

In Action

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	911.774 (+0.103)	305.463 (+0.023)

[ShaMan123]

small and concise

[asturur]

getTotalAngle has the benefit of returning a shortcut of the plane rotation for simple objects that have no nested situations.

So please keep using it where it was, since you removed the overhead of qrDecompose from it. The conversion from radians to degree is a multiplication and is not worth loosing the optimization for the common use case.

[ShaMan123]

So please keep using it where it was, since you removed the overhead of qrDecompose from it. The conversion from radians to degree is a multiplication and is not worth loosing the optimization for the common use case.

are you talking about this change??
I don't get your reason, what do you mean?
Why change to an angle and back to rad? It is purly redudant

[asturur]

yes i want to keep getTotalAngle in use because it avoids using the calcTransformMatrix when there is no need for it.
if you need calcPlaneRotation to shortcut some computation cost of qrDecompose that is fine, if you think is useful to expose that is fine too,

It may be redundant to convert an angle to a degree, but is way less than creating the matrix from scratch when there is no need to it because we just need the angle.

By putting calcPlaneRotation in place where a simple angle would be ok you are wasting more than you are gaining.
So that is not ok.

We added getTotalAngle because we had to take in account the new edge case of nested objects, but that is not the norm. That would have been just 'this.angle' before, and that is what it is in most of the cases.

Your Motivation:

Motivation 1:

I need to calc angles of planes/matrices in certain cases.
I prefer to avoid decomposition because in most cases I need the radian value and not the degree.
In addition the rest of the calculation is not needed and is wasteful in terms of perf.

^ this are facts and are correct, i can't really say nothing about. An additive change and some calculation change, fine.

Motivation 2:

Also I believe fabric can and should reduce its dependancy on decomposition for all the known and unknown reasons

^ this is a belief to which i disagree with and so the path of not changing is the preferred one.

That is not the direction we will go if we don't find a framework to collaborate in advance on changes.

[ShaMan123]

this is a belief to which i disagree with and so the path of not changing is the preferred one.
That is not the direction we will go if we don't find a framework to collaborate in advance on changes.

That is why I wrote I believe

It may be redundant to convert an angle to a degree, but is way less than creating the matrix from scratch when there is no need to it because we just need the angle.
By putting calcPlaneRotation in place where a simple angle would be ok you are wasting more than you are gaining.
So that is not ok.

I don't see a case where a matrix is not created, please enlighten me.
Also, please understand that the last PRs you were very picky about asoteric perf issues so this feels kind of off. Indeed I did not think of the matrix creation because I am sure it is created once rendering starts or setCoords is called, and that happens in every use case.

I do not want to defend so enlighten me instead.

[asturur]

is not esoteric performance issues.
calcTransformMatrix is more costly than accessing this.angle.
The cache validation part of getTransformMatrix is probably as expensive as calculating the matrix itself and is not done when render start, is done every time.

So all the time you are running findCornerQuadrant on an object that has no group, calling getTotalAngle will let us pick the best way to get angle for that situation.
That's it.

[github-actions]

Coverage after merging calcPlaneRotation into master will be

82.98%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	7.69%	100%	0%	14.29%	17, 20, 23, 35, 38, 41
src
ClassRegistry.ts	100%	100%	100%	100%
Collection.ts	94.71%	94.64%	86.67%	97.09%	101, 104, 207–208, 233–234
CommonMethods.ts	96.55%	87.50%	100%	100%	9
Intersection.ts	100%	100%	100%	100%
Observable.ts	87.76%	85.29%	87.50%	89.58%	134–135, 160–161, 32–33, 41, 50, 78, 89
Point.ts	100%	100%	100%	100%
Shadow.ts	98.36%	95.65%	100%	100%	178
cache.ts	97.06%	90%	100%	100%	57
config.ts	75%	66.67%	66.67%	82.76%	130, 138, 138, 138, 138, 138–140, 151–153
constants.ts	100%	100%	100%	100%
src/Pattern
Pattern.ts	92.31%	91.89%	90%	93.55%	119, 130, 139, 32, 95
src/brushes
BaseBrush.ts	100%	100%	100%	100%
CircleBrush.ts	0%	0%	0%	0%	108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 13, 136, 138, 23–24, 32–36, 40–44, 51–54, 62–66, 68, 76, 76, 76, 76, 76–77, 79, 79, 79–82, 84, 92–93, 95, 97–99
PatternBrush.ts	97.06%	87.50%	100%	100%	21
PencilBrush.ts	91.06%	82.35%	100%	93.81%	122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
SprayBrush.ts	0%	0%	0%	0%	107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 149, 149, 149–152, 154–157, 161–162, 164, 166–169, 17, 172, 179, 18, 180, 182, 184–185, 187, 194–195, 197–198, 20, 201, 201, 208, 208, 21, 212, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
Canvas.ts	78.87%	76.67%	82.76%	79.88%	1000–1001, 1001, 1001, 1008–1009, 1017–1018, 1018, 1018–1019, 1025, 1027, 1055–1057, 1060–1061, 1065–1066, 1185–1187, 1190–1191, 1264, 1383, 148, 1505, 173, 282–283, 286–290, 295, 318–319, 32, 324–329, 349, 349, 349–350, 350, 350–351, 359, 36, 364–365, 365, 365–366, 368, 377, 383–384, 384, 384, 427, 435, 439, 439, 439–440, 442, 525–526, 526, 526–527, 533, 533, 533–535, 555, 557, 557, 557–558, 558, 558, 561, 561, 561–562, 565, 574–575, 577–578, 580, 580–581, 583–584, 596–597, 597, 597–598, 600–605, 611, 618, 655, 655, 655, 657, 659–664, 670, 676, 676, 676–677, 679, 682, 687, 700, 728, 728, 789–790, 790, 790, 790, 790, 790, 793–794, 797, 797–799, 802–803, 879, 891, 898, 898, 898, 911, 944, 965–966, 982–983, 983, 983–985, 988–989, 989, 989, 991, 999, 999, 999
CanvasOptions.ts	100%	100%	100%	100%
SelectableCanvas.ts	94.35%	92.06%	94.23%	96.20%	1080, 1082, 1084–1085, 301, 471–472, 474–475, 475, 475, 524–525, 586–587, 600, 640–642, 674, 679–680, 707–708, 880, 880–881, 884, 904, 904, 953, 961
StaticCanvas.ts	96.78%	93.09%	100%	98.53%	1031, 1041, 1093–1094, 1097, 1132–1133, 1209, 1218, 1218, 1222, 1222, 1269–1270, 187–188, 204, 571, 583–584, 914–915, 915, 915–916
StaticCanvasOptions.ts	100%	100%	100%	100%
TextEditingManager.ts	84.31%	71.43%	91.67%	88%	17–18, 18, 18–19, 19, 19
src/canvas/DOMManagers
CanvasDOMManager.ts	95.52%	70%	100%	100%	21–22, 29
StaticCanvasDOMManager.ts	97.50%	88.89%	100%	100%	33
util.ts	86.67%	80.56%	83.33%	93.94%	14, 26, 63–64, 67, 67, 74, 93–94
src/color
Color.ts	94.96%	91.67%	96.30%	96.05%	233, 258–259, 267–268, 48
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	85.71%	76.92%	100%	89.74%	55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
Control.ts	94.44%	93.10%	91.67%	96.77%	183, 249, 354
changeWidth.ts	100%	100%	100%	100%
commonControls.ts	100%	100%	100%	100%
controlRendering.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
drag.ts	100%	100%	100%	100%
fireEvent.ts	88.89%	75%	100%	100%	13
polyControl.ts	7.25%	0%	0%	13.51%	103, 108, 120, 120, 120, 120, 120, 122–125, 125, 128, 135, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–84, 84, 86, 89–90, 90, 90, 90, 90, 92, 97
rotate.ts	19.57%	12.50%	50%	21.43%	41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
scale.ts	93.57%	92.94%	100%	93.67%	129–130, 132–134, 148–149, 181–183, 42
scaleSkew.ts	78.79%	64.29%	100%	85.71%	27, 29, 29, 29, 31, 33, 35
skew.ts	91.03%	79.31%	100%	97.67%	131–132, 163–164, 171, 177, 179
util.ts	100%	100%	100%	100%
wrapWithFireEvent.ts	100%	100%	100%	100%
wrapWithFixedAnchor.ts	100%	1

[ShaMan123]

I actually forgot to expose this