fix(Fabric.text): account for fontSize in textpath dimensions

[melchiar]

Text on a path is currently using the dimensions of the assigned path which can cause the cache size to be too small.

This fix adds the calculated text height to the object's dimensions to ensure that nothing is clipped in the cache canvas.

Honestly though I'm not crazy about this solution since depending on the path's shape and direction this means the bounding box will sometimes be unnecessarily large. Let me know if you can think of a better idea.

[melchiar]

Obviously this change would require fixing all of the visual tests since the new dimensions change the positioning. I'd like to try to find another solution to this issue if we can though. I've experimented with simply factoring the font size into the cache size calculation and leaving the object's bounding dimensions untouched but doing that seems to cause the cache's resolution suffer.

[asturur]

a better, but complicated idea, would be to get the object size looking at the Path + the character boxes we create. For each char we have the width and the position + angle. That means we can collect all the 4 corners of each character and throw inside a bucket of points that will give us a bounding box.

[melchiar]

a better, but complicated idea, would be to get the object size looking at the Path + the character boxes we create. For each char we have the width and the position + angle. That means we can collect all the 4 corners of each character and throw inside a bucket of points that will give us a bounding box.

That might be a bit over complicated. I'm imagining a scenario like this where text is curved around the top and bottom of two circles. Aligning the two circles to each other is a lot easier if the bounding boxes are uniform and you can use the same center position.

[github-actions]

Code Coverage Summary

> fabric@4.5.1 coverage:report /home/runner/work/fabric.js/fabric.js
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.31 |    77.06 |   86.01 |   83.02 |                                               
 fabric.js |   83.31 |    77.06 |   86.01 |   83.02 | ...,29853,29978,30058-30123,30246,30345,30562 
-----------|---------|----------|---------|---------|-----------------------------------------------

[melchiar]

I've decided to take a different approach and simply adjust the cache dimensions based on the font size. I think this is a better solution than increasing the text object's dimensions since from a UX perspective the path dimensions are more important in my opinion (this is also the way Illustrator does it).

Anyone who wants to include the text in the bounding box can do so with padding.

[melchiar]

No clipping!

[asturur]

shouldn't those be additionalWidth += this.fontSize * this.zoomX to take in account the 0.1 extra size of the previous step?
Still looking at the first line height is better.
You don't want a weird styled text to fool you with fontSize 2 on the object and fontSize 45 on the style

[melchiar]

good point!

[asturur]

ok just the tests are left out, i can do this in a bit.

[github-actions]

Code Coverage Summary

> fabric@4.5.1 coverage:report /home/runner/work/fabric.js/fabric.js
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.31 |    77.04 |   86.01 |   83.02 |                                               
 fabric.js |   83.31 |    77.04 |   86.01 |   83.02 | ...,29853,29978,30058-30123,30246,30345,30562 
-----------|---------|----------|---------|---------|-----------------------------------------------

[github-actions]

Code Coverage Summary

> fabric@4.5.1 coverage:report /home/runner/work/fabric.js/fabric.js
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.31 |    77.06 |   86.01 |   83.02 |                                               
 fabric.js |   83.31 |    77.06 |   86.01 |   83.02 | ...,29853,29978,30058-30123,30246,30345,30562 
-----------|---------|----------|---------|---------|-----------------------------------------------

[emielmolenaar]

Thanks for this fix! Nice work.