fix(Controls) avoid sliding with stroke uniform

[asturur]

Motivation

As reported in #9423 , there is an issue in determining the correct anchor point when strokeUniform is used.
This PR propose a fix by excluding strokeWidth by the calculations when determining the anchor point.

closes #9423

Description

Changes

Gist

In Action

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	912.029 (+0.255)	305.516 (+0.053)

[asturur]

MIssing code cleanup and interaction test.

[asturur]

also polygons with skew do not work ok.
But that could be explictly not supported

[ShaMan123]

You blocked that fix

[ShaMan123]

I have given a quick read
This seems a patch, not a fix
The real issue is that stroke is considered for layout calculations.
That is completely wrong. And has many related issues.
Stroke should be considered for bbox calculation only.

[asturur]

You blocked that fix

it wasn't linked in the issue or mentioned, can you point me to it?

[asturur]

I have given a quick read This seems a patch, not a fix The real issue is that stroke is considered for layout calculations. That is completely wrong. And has many related issues. Stroke should be considered for bbox calculation only.

This the wrong approach imho.
There is a function here that has to calculate an anchor point, it is isolated, and is not the base for other functionalities.
The function doesn't work and a fix that is scoped to the function only is a good fix.
As you say if stroke ( with strokeUniform ) shouldn't considered for layout/position calculation, removing stroke from the calculation when necessary is a solution.

strokeUniform is a weird functionality to support, the real fix would be removing strokeUniform, so stroke can stay happily in calculation.

[github-actions]

Coverage after merging polyline-stroke-uniform into master will be

82.95%

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	6.94%	0%	0%	12.50%	102, 107–108, 113, 125, 125, 125, 125, 125, 127–130, 130, 133, 140, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 87–89, 89, 91, 94–95, 95, 95, 95, 95, 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%

[ShaMan123]

This is a discussion to conduct thoroughly.

[ShaMan123]

It is a major part of the design of fabric and the coords/layout system

[ShaMan123]

strokeUniform will not be that much of a pain if stroke is not part of layout

[ShaMan123]

You blocked that fix

it wasn't linked in the issue or mentioned, can you point me to it?

#8556 (review)
#8556 (comment)
#8556 (comment)

e6ba30c

[asturur]

This needs to be re-examined after #9460 is merged.
I can't mix and match the fixes now for time constrains, but if after #9460 skewX can work we need to update one test to take care of it.

[asturur]

You blocked that fix

it wasn't linked in the issue or mentioned, can you point me to it?

#8556 (review) #8556 (comment) #8556 (comment)

e6ba30c

oh now i understand what you mean.
I thought you were talking about this fix blocked somewhere else.

Yes i still think skewing is an edge case and if you want to support it with polygon editing you are better off merging skew in the points every time you apply it.
The effect is the same and everything is so much easier.

[ritali4912]

If a two points horizontal polyline with strokeWidth = 0, it's height will be 0. Does the divide in below got NaN?
const newPositionNormalized = anchorPoint
.subtract(poly.pathOffset)
.divide(poly._getNonTransformedDimensions())
.multiply(adjustFlip);

[ShaMan123]

getStrokeWidthVector(strokeWidth = this.strokeWidth) {
  const strokeWidthFactor = new fabric.Point(strokeWidth, strokeWidth);
  return this.strokeUniform
    ? strokeWidthFactor.divide(this.getObjectScaling())
    : strokeWidthFactor;
}

[asturur]

this is obsolete