Skip to content

feat(text.class.js) add path rendering to textpaths#7328

Merged
asturur merged 9 commits into
masterfrom
textpath-renderpath
Aug 27, 2021
Merged

feat(text.class.js) add path rendering to textpaths#7328
asturur merged 9 commits into
masterfrom
textpath-renderpath

Conversation

@melchiar

Copy link
Copy Markdown
Contributor

This adds path rendering for text objects that contain a path.

image

@asturur - let me know if you're okay with this implementation and I'll add a visual test.

Comment thread src/shapes/text.class.js Outdated
* @param {CanvasRenderingContext2D} ctx Context to render on
*/
_render: function(ctx) {
this.path && this.path._render(ctx);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmm
let me understand this.
So if i don't want to draw a path, what do i do?
i have to set strokeWidth or stroke empty or fill empty?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my thinking yes. Currently though there's no check for the visible property, so maybe I should add that as one more way to hide the path?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. visible is checked somewhere else higher up in the rendering chain, but yes adding that here is ok. and opacity > 0.

@asturur

asturur commented Aug 25, 2021

Copy link
Copy Markdown
Member

i'm not opposed to this, it sounds strange initially, but if i stop thinking isn't bad.
Is a one liner, and is OK to render by default and hide easily setting fill to transparent.
You should go the extra mile and comment it clearly in the source code, that when adding a path to the text, that will be also drawn unless fill is transparent.

@melchiar

Copy link
Copy Markdown
Contributor Author

Sure, I can add some explanatory comments. My use case for this is as a resizing guide, drawing the path only during transform events.

text-path-resizing

@asturur

asturur commented Aug 26, 2021

Copy link
Copy Markdown
Member

We have e method called .isNotVisible that takes in account opacity and isVisible and some other little stuff.

@github-actions

github-actions Bot commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

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.32 |    77.08 |   86.02 |   83.03 |                                               
 fabric.js |   83.32 |    77.08 |   86.02 |   83.03 | ...,29856,29981,30061-30126,30249,30348,30565 
-----------|---------|----------|---------|---------|-----------------------------------------------

asturur
asturur previously approved these changes Aug 27, 2021
@asturur

asturur commented Aug 27, 2021

Copy link
Copy Markdown
Member

i did not make a good choice with tests.
Do not merge before fixing tests

@github-actions

github-actions Bot commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

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.32 |    77.06 |   86.02 |   83.03 |                                               
 fabric.js |   83.32 |    77.06 |   86.02 |   83.03 | ...,29856,29981,30061-30126,30249,30348,30565 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur

asturur commented Aug 27, 2021

Copy link
Copy Markdown
Member

Ok path opacity does not work, just a note does not have to be a bug.

@github-actions

github-actions Bot commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

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.32 |    77.08 |   86.02 |   83.03 |                                               
 fabric.js |   83.32 |    77.08 |   86.02 |   83.03 | ...,29856,29981,30061-30126,30249,30348,30565 
-----------|---------|----------|---------|---------|-----------------------------------------------

@github-actions

github-actions Bot commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

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.32 |    77.06 |   86.02 |   83.03 |                                               
 fabric.js |   83.32 |    77.06 |   86.02 |   83.03 | ...,29856,29981,30061-30126,30249,30348,30565 
-----------|---------|----------|---------|---------|-----------------------------------------------

asturur
asturur previously approved these changes Aug 27, 2021
@github-actions

github-actions Bot commented Aug 27, 2021

Copy link
Copy Markdown
Contributor

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.07 |   86.02 |   83.03 |                                               
 fabric.js |   83.31 |    77.07 |   86.02 |   83.03 | ...,29875,30000,30080-30145,30268,30367,30584 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
asturur merged commit 7243d83 into master Aug 27, 2021
@asturur asturur mentioned this pull request Aug 27, 2021
@eltaiguer

Copy link
Copy Markdown

Hi @asturur if I enliven a IText with a path, I get the following error
core.js:6014 ERROR TypeError: path.isNotVisible is not a function

Maybe there's a check missing or when re-building the IText the Path isn't being recreated?

@asturur

asturur commented Sep 26, 2021

Copy link
Copy Markdown
Member

Can you reproduce it in a fiddle?
Or can you post how are you enliving it?

@digitalml

Copy link
Copy Markdown

https://user-images.githubusercontent.com/9057412/130850444-8856caad-29ae-43f5-af3f-857b6fd82973.gif

@melchiar Would you be so kind as to share the fiddle for the above example you posted? I've checked here http://jsfiddle.net/user/melchiar/fiddles/ but don't see it. Thank you!

@melchiar

Copy link
Copy Markdown
Contributor Author

@digitalml Sure, I actually shared it in the main text on path issue tracker as well #6543 (comment)

https://jsfiddle.net/melchiar/vhgtesoy/

@digitalml

Copy link
Copy Markdown

@melchiar thank you. I did see this one. I was specifically looking for the hidden path version.

@melchiar

Copy link
Copy Markdown
Contributor Author

I haven't made a demo of that code since it's part of our platform, but it's just a matter of using events to show/hide the path during and after performing transformations.

@shyko

shyko commented Mar 1, 2022

Copy link
Copy Markdown

Hi @eltaiguer . I have the same problem - ERROR TypeError: path.isNotVisible is not a function when I use IText type. Did yuo find any solutions?

@melchiar

melchiar commented Mar 2, 2022

Copy link
Copy Markdown
Contributor Author

@shyko the path property is in beta and isn't currently supported for iText objects (just Text objects for now)

@Clemweb

Clemweb commented Aug 17, 2023

Copy link
Copy Markdown

@melchiar Hi, is it possible to have a Fiddle of the demo from the 25/08 ? The interaction is exactly what I need...

Thanks

@kalmigs

kalmigs commented Dec 21, 2023

Copy link
Copy Markdown

@digitalml Sure, I actually shared it in the main text on path issue tracker as well #6543 (comment)

https://jsfiddle.net/melchiar/vhgtesoy/

Thank for this @melchiar! this is what worked for me.
Been searching for this for days for curved text. Tried the old solutions but will not work on Node.js Canvas properly, Found out recently about Text paths and found this. Thank you so much!

jsfiddle is not working, but I just copied code in my project and works perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants