fix(polyline): stroke bounding rect calculation typescript

[luizzappa]

I migrated the PR #8083 code to typescript and corrected the calculation when there is skew.

closes #8083

Related Issues

• bug(Path): stroke projection on bbox #8025
• Stroke for triangles gets cut off #7847
• fix(): stroke bounding box #7478

Proposed changes

• In calcAngleBetweenVectors function, calculate angle between two vector using atan2 instead of dot product. So eliminated the need to check the angle direction in getBisector function,
• Changed the _setPositionDimensions function in the polyline class to adjust the width/height by the scale/skew.
• Changed the projectStrokeOnPoints function to cover all stroke line join cases and also to uniform stroke.

Working example

The red dotted vector is the bisector returned by the getBisector function. The green dotted vector is in the opposite direction and is what is used to determine which side to plot the projections. The projections are the colored vectors, one color for each point.
To make debugging easier, you can configure the parameters (stroke line join type, strokeUniform, miterLimit, etc..)

Documentation

This section details the calculation of projections for each type of stroke line join.

Summary

• 1. Open path
  • 1.1 Butt
  • 1.2 Round
  • 1.3 Square
• 2. Closed path
  • 2.1 Miter
  • 2.2 Bevel
  • 2.3 Round
    • 2.3.1 Round without skew
    • 2.3.2 Round with skew

Links:

• MDN: https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/stroke-linejoin
• Spec: https://svgwg.org/svg2-draft/painting.html#StrokeLinejoinProperty
• Playground to understand how the line joins works: https://hypertolosana.github.io/efficient-webgl-stroking/index.html

1. Open path

These cases differ in relation to the line-cap property (MDN reference)

1.1 Butt

The projections are calculated by two vectors orthogonal to the stroke.

Scaling effect: only when the stroke is uniform that scaling affects the calculation of projections. It changes the position of the points (see more details here), so we must take it into account to have the new arrangement of the points.

Skew effect: just apply distortion after projection calculation.

1.2 Round

Same as 2.3 Round

1.3 Square

Project a rectangle of points on the stroke in the opposite direction of the vector AT. The projections are the two vectors starting from the vertex pointing to the sides of the square.

Scaling effect: only when the stroke is uniform that scaling affects the calculation of projections. It changes the position of the points (see more details here), so we must take it into account to have the new arrangement of the points.

Skew effect: just apply distortion after projection calculation.

2. Closed path

There are three types of stroke line join (MDN reference)

2.1 Miter

The corner is formed by extending the outer edges of the stroke at the tangents of the path segments until they intersect.

Stroke miter limit: when two line segments meet at a sharp angle, it is possible for the join to extend, far beyond the thickness of the line stroking the path. The stroke-miterlimit imposes a limit on the extent of the line join, if so the join is converted from a miter to a bevel. (MDN reference)

The ratio of miter length (distance between the outer tip and the inner corner of the miter) to stroke-width is directly related to the angle (alpha) of the bisector:

$strokeMiterLimit = \frac{miterLength}{strokeWidth} = \frac{1}{sin(\alpha)}$

Note: when the stroke is uniform, the arrangement of the points is changed (see explanation below), consequently the strokeMiterLimit. In this case we must recalculate this limit.

Scaling effect : scaling changes the arrangement of points, hence the bisector calculation. Take a look:

Plotting a triangle with the points:

A (0, 0)
B (50, -200)
C (100, 0)

The angle of the bisector is alpha.

But when I apply a scaling of 2 only on the X axis, the angle of the bisector changes (beta < alpha), hence the miter vector too. To calculate this new angle we must apply the scaling on the points to find the new vectors that form A, B and C.

scaleX = 2 ; scaleY = 1

scaled A (0 * scaleX , 0 * scaleY) --> (0, 0)
scaled B (50 * scaleX, -200 * scaleY) --> (100,-200)
scaled C (100 * scaleX, 0 * scaleY) --> (200,0)

With these vectors we were able to correctly calculate the miter vector.

Skew effect: just apply distortion after projection calculation.

2.2 Bevel

The projections are both orthogonal to the vertex:

Scaling effect: only when the stroke is uniform that scaling affects the calculation of projections. It changes the position of the points (see more details here), so we must take it into account to have the new arrangement of the points.

Skew effect: just apply distortion after projection calculation.

2.3 Round

This case has calculation of projections differently if there is or not the presence of skew.

2.3.1 Round without skew

The projections are the two vectors parallel to the X and Y axes.

Scaling effect: does not directly affect projections (only stroke size).

2.3.2 Round with skew

Projections are the points furthest from the vertex in relation to the direction of the X and Y axis respectively. See the example below.

Consider this case:

There is a point $(X_i, Y_i)$ that will be the furthest Y (max Y) distance from the origin after applying skewY. This is our point of interest.

We have:

[eq.1] $Y_{afterSkewY} = Y_i + X_i * Tan(skewY)$

And we know that $X_i$ is a function of $Y_i$:

$Y_i^2 + X_i^2 = r^2$
[eq.2] $X_i = \pm \sqrt{r^2 - Y_i^2}$

Substituting eq.2 in eq.1 we have:

$Y_{afterSkewY} = Y_i \pm \sqrt{r^2 - Y_i^2} * Tan(skewY)$

So we have an equation in terms of just Y:

$f(Y_i) = Y_i \pm \sqrt{r^2 - Y_i^2} * Tan(skewY)$

The plus/minus sign represent both sides of the circle after skewY:

The furthest Y after skewY (max Y) distance from the origin are the maximum/minimun of $f(Y_i)$:

Computing $f'(Y_i)=0$ for both cases, we find:

$Y_i = \pm \frac{r}{\sqrt{1 + Tan^2(skewY)}}$

Substituting $Y_i$ into the circle equation, we find $X_i$:

$X_i = \pm \sqrt{r^2 - Y_i^2}$

That's our solution:

We found the furthest point on the Y axis, now we need to calculate the furthest point on the X axis:

In this case we have to take into account that if there is skewY it will affect skewX. The canvas first applies skewY and then applies skewX, so the point $(Xi, Yi)$ after applying skewX and skewY will look like this:

1. First apply skewY: $(Xi, Yi + Xi * Tan(skewY))$
2. Second apply skewX: $(Xi + (Yi + Xi * Tan(skewY)) * Tan(skewX), Yi + Xi * Tan(skewY))$

That is, skewX is dependent on the result of skewY.

So the equations we have to solve are:

[eq.1] $X_{afterSkewX} = X_i + Y_i * Tan(skewX)$

[eq.2] $Y_i = \sqrt{r^2 - X_i^2} \mp X_i * Tan(skewY)$

Substituting eq.2 into eq.1:

$X_{afterSkewX} = X_i + \sqrt{r^2 - X_i^2} * Tan(skewX) \mp X * Tan(skewY) * Tan(skewX)$

Computing $f'(X_i)=0$ for both cases, we find:

$X_i = \frac{r^2}{1 + \frac{Tan^2(skewX)}{(\pm Tan(skewX) * Tan(skewY) - 1)^2}}$

Scaling effect: in the case of stroke uniform, we must remove the effect of the scale in the calculation of the farthests points,. Because of this, the equations are slightly modified.

Calculating the furthest point on the Y axis:

[eq.1] $Y_{afterSkewY} = Y_i * (1/scaleY) + X_i * (1/scaleX) * Tan(skewY)$

[eq.2] $X_i = \pm \sqrt{r^2 - Y_i^2}$

Substituting eq.2 into eq.1:

$Y_{afterSkewY} = Y_i * (1/scaleY) \pm \sqrt{r^2 - Y_i^2} * (1/scaleX) * Tan(skewY)$

Computing $f'(Y_i)=0$ for both cases, we find:

$Y_i = \pm \frac{r * 1/scaleY}{\sqrt{(1/scaleY)^2 + (1/scaleX)^2 * Tan^2(skewY)}}$

Calculating the furthest point on the X axis:

[eq.1] $X_{afterSkewX} = X_i / scaleX + Y_i * Tan(skewX)$

[eq.2] $Y_i = \sqrt{r^2 - X_i^2} / scaleY \mp X_i * Tan(skewY) / scaleX$

Substituting eq.2 into eq.1:

$X_{afterSkewX} = X_i / scaleX + \sqrt{r^2 - X_i^2} * Tan(skewX) / scaleY \mp X_i * Tan(skewY) * Tan(skewX) / scaleX$

Computing $f'(X_i)=0$ for both cases, we find:

$X_i = \frac{r^2}{1 + \frac{Tan^2(skewX) * scaleY^2}{(scaleX \mp scaleX * Tan(skewX) * Tan(skewY))^2}}$

TO DO

• Bring here all the documentation that is in the PR fix(polyline): stroke bounding rect calculation #8083
• Handle the case when skewX and skewY are applied at the same time when stroke line join is round (still buggy)
• Handle open path with strokeLinecap equals round and square
• Check if in the _set function when skew is applied, I need to invoke dimensioning for all line type joins
• Adjust http://fabricjs.com/custom-controls-polygon to work correctly with skew (another PR) Poly custom control #8556
• handle 1 point edge case: round projection?

[luizzappa]

@ShaMan123,

I had resolved the need to recalculate BBOX to only when _set method is invoked. However, this causes a point of attention. When a value is directly assigned to any of the properties (skewX, skewY, scaleX, scaleY), eg canvas.getActiveObject().scaleX =2, the _set method is not invoked, and the bbox is not recalculated. (I don't think there's a way to deal with it)

[ShaMan123]

That is how fabric behaves.
It is still ambiguous but already there are features that depend on the dev using the set method.
It was known as a best practice but will become a must.
We might use setters to overcome this limitation as part of the TS migration.
So no worries.

[ShaMan123]

For example text and group already depend on using set

[ShaMan123]

Regarding polygon controls.
The community will appreciate that as well. There are many issues open asking to fix that.
I heard chat of making it part of core.
@asturur ?
v6 has changed some internals that broke it.

[ShaMan123]

What do you think about splitting up the code and creating a function for each case?
If you don't like that I think we should define A, B, C, AB, AC at the top if possible.

• I saw some var. They should all become const or let. VSCODE has an option to quick fix all occurrences if you hover over one.
• export all types

[luizzappa]

What do you think about splitting up the code and creating a function for each case? If you don't like that I think we should define A, B, C, AB, AC at the top if possible.

The code will be cleaner, I will refactor

[ShaMan123]

https://codesandbox.io/s/project-stroke-points-with-skew-correction-forked-xe09oo

can't wait for codesandbox action to work! Then no more of this copy paste bullshit

[ShaMan123]

@luizzappa indicate all is good so I can merge with peace

[ShaMan123]

@asturur surfacing this comment so you won't get surprised.
We found a bug in skewing controls. This PR aggravates it. Following up with a fix (perhaps w/o qrDecompose)

Meanwhile I have managed to fix skewing with controls. I will PR soon so no worries about that. master...fix-skewing-controls

[ShaMan123]

@asturur any objections we make polygon controls part of core using the adjusted logic from the demo @luizzappa created?

[luizzappa]

Everything seems to be correct. Thanks for all the support @ShaMan123 !

Regarding the tests, is it better to open a new PR (or are we still in #8087)?

[ShaMan123]

My pleasure.
A new PR and a dedicated test file

[asturur]

1. Precision:

Precision is not THAT important.

I absolutely disagree. The whole point of a bounding box is precision, the whole point of this PR and similar efforts. Product-wise is seems very unprofessional as viewed by a user (myself included).

It is ok to disagree. Solutions and costs needs to be balanced.
If the bouding box is a bit thight on roundy corner on edgy use cases, i m super happy to guess a constant factor and just make the bounding box a bit larger to accomodate that.
When choosing between a bunch of code and an accomodation, ultimate precision isn't that much important.

2. hypot:
   I removed it, though I disagree as well.
   If hypot on chrome is wrong and fabric suffers from it why not expose hypot for devs?
   Since I managed to get the code working without it I'll back off and let lint pick it up in the future.

I just asked you to expose a leaner version of it since here there are no cases of 3+ dimensional distances. Right?
Add code when you need it, and not because someone could one day come here and need it.

Said so the bounding box calculation in this way breaks ( obviously ) the skewing bounding box from controls that at least in the sandbox jump left and right often.
That is to take in account as a possible defect of this feature and we can't do much about it, and that is fine, not everyine is skewing all the time.

For the example of the skew i made before, it should also be taken in account that for roundy corner, and overall different solution could be to trace the path that the strokes create, approximate (basically is an exact match) the roundy part with a beizer curve, and the split it in segments with the measurement code, and then apply the transformations to those points, that would give you more points in total, but would solve the issue under skew or at least it would approximate good enough the skewed rounded corner.

It is already possible to transform a path with a canvas transform and obtain a new path that absorbs the transformation and move all its points and control points to look like identical to the transformed path ( at leat from my tests, i can't back this up with a theorem )

[ShaMan123]

Said so the bounding box calculation in this way breaks ( obviously ) the skewing bounding box from controls that at least in the sandbox jump left and right often.

#8344 (comment)

Add code when you need it, and not because someone could one day come here and need it.

I was being greedy.

I suggested other options to calculate the bbox such as what you've suggested. We could explore them later on.

[asturur]

i kind of get that strokeOffset is the value that tells us in which direction the bounding box move most of its new space, becuase that was always one of the things that this kind of new calculation was adding, the center of the shape with and without stroke is no more the same and this strokeOffset kind of represent that.
But where are we using it? I can't see use it in the PR nor in the codebase.

[ShaMan123]

Trash my comments entirely in favor of
f158267

[asturur]

github mixes comments sometimes ( it does it to me ).
Indeed this answer is for the other thread.
strokeOffset still a mistery? or your answer was related and i don't see how?

[ShaMan123]

Related to the other thread indeed.
Can't find it anymore. Too much chat.

Don't understand your questions

[asturur]

i ll try easier, at the cost of looking unpolite.

Show me where strokeOffset is used.
i see it calculated, not used.

[ShaMan123]

As I wrote in the thread it is meant for the dev.
Think of using originX/Y in a case of a large miter for example. The poly might shift a lot because of this miter. So a dev might want to adjust the position so the origin is in respect to the points instead of the entire stoked miter

[asturur]

ok so i guessed right what is it, and is unused.
I'll think of it.

[asturur]

Regarding polygon controls. The community will appreciate that as well. There are many issues open asking to fix that. I heard chat of making it part of core. @asturur ? v6 has changed some internals that broke it.

We should definitely add polygon/path controls after 6.0 is released.
Not bound to the object automatically, but definitely available and supported

[asturur]

i need to craft a polygon with a translate transform, that i think gets absorbed with top and left, let me do that.

[asturur]

Added the test in master, if it pass you can merge.
I ll probably come back on that automatic update on _set

[asturur]

This PR is good.
There are some things i don't like on the generic changes in polygon, and i will come back to them:

• new special _set: for the polygon and the bounding box
• setDimension is a new function, but has 2 task, setDimensions and return me the top,left and that is weird. Not dangerous, but at least uncommon. Would be probably less weird if it would return all it calculated, and then we extract what we need from it four our edge cases
• boundingBox early return should go away if is only a compiler issue and not a real code issue that fabricJS could trigger ( seems also double protected in the polyline class too. ). In general if a function requires an array of things, it either handles the empty array or pretends a non empty array.
• handle strokeOffset where is needed, to keep a box steady