Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify PencilBrush render to allow smoother curves #4664

Closed
fredguth opened this issue Feb 2, 2018 · 15 comments
Closed

Modify PencilBrush render to allow smoother curves #4664

fredguth opened this issue Feb 2, 2018 · 15 comments

Comments

@fredguth
Copy link

fredguth commented Feb 2, 2018

This is a feature I intend to implement, but I would like to discuss it before coding.

Motivation

Please compare:
http://fabricjs.com/freedrawing
with
http://paperjs.org/examples/path-simplification/

Notice that paper.js curves seem more realistic.
This is due to 2 factors:

  1. Paper.js uses cubic bezier curves, while Fabric.js uses quadratic bezier curves.
  2. Paper.js uses path simplification, removing points that don't change the curve by a certain epsilon slope. In other words, if 2 adjacent segments of a path are parallel (collinear), transform those 2 segments in one.

Suggested Design

  • create property 'simplified' defaulted to false in BasicBrush.class.js
  • create function simplifyPath(points:Array) in pencil_brush.class.js
  • modify pencil_brush.render() if simplified ===true, by removing points that are parallel.

Alternatives

@asturur
Copy link
Member

asturur commented Feb 2, 2018

So my opinion is that if one is better than the other, visually better and gives less complex path, there is no need to have both.

So if you can change the code of pencil brush in a simpler one, that gives the same visual effect, you can override the previous code.

Also if there is path semplification logic to write, write that as a fabric.util member in order to be reusable on normal fabric.Path

To be reusable the fabric.util obviously, should not be a class member but a simple function that return a simplified path without mutating the input ( if possible )

@asturur
Copy link
Member

asturur commented Feb 2, 2018

about this

adjust brush width with by the movement velocity (see http://szimek.github.io/signature_pad/ and http://paperjs.org/tutorials/interaction/working-with-mouse-vectors/

this can be a subclass of the pencil brush, with a different name that can be or not be part of the main library, depending on usefulness/code quantity/pluggability.

@fredguth
Copy link
Author

fredguth commented Feb 2, 2018

Also if there is path semplification logic to write, write that as a fabric.util member in order to be reusable on normal fabric.Path

An alternative is to write the pencil_brush to use fabric.Path and implement fabric.Path.simplify([tolerance]).

this can be a subclass of the pencil brush, with a different name

Totally agree that is more a ink brush or something like that.

@asturur
Copy link
Member

asturur commented Feb 2, 2018

i think a simple function, a utility is better.
People may work in project and be in need to use it on path strings for any reason.

I see that the paper.js example draw segments and then it update to bezier. If you want to work on it, verify if is possible to draw with quadratic, and then do segment -> bezier in a single pass. Is nice to draw and see smooth curves from the beginning.

@fredguth
Copy link
Author

fredguth commented Feb 14, 2018

@asturur, I get an error trying to build:

fredguth@campari in ~/Code/fabric/fabric.js (bezier●)
$ nvm use v8.2.1
Now using node v8.2.1 (npm v5.6.0)

fredguth@campari in ~/Code/fabric/fabric.js (bezier●)
$ node build.js modules=ALL exclude=node
/Users/fredguth/Code/fabric/fabric.js/build.js:250
  process.chdir(distributionPath);
          ^

Error: ENOENT: no such file or directory, uv_chdir
    at Object.<anonymous> (/Users/fredguth/Code/fabric/fabric.js/build.js:250:11)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:575:3

any idea?
Found error. There was no 'dist' directory.

@asturur
Copy link
Member

asturur commented Feb 14, 2018 via email

@fredguth
Copy link
Author

I found the error. The problem was that I needed to create a dist folder. 😬

@fredguth
Copy link
Author

Btw, I am working on this candidate feature right now.

@asturur
Copy link
Member

asturur commented Feb 19, 2018

#4743

be aware of those changes that are going to be probably merged.

The brushes should have for efficient render, a method to draw just the difference and then also the full render. Pencil brush was an exception and now has been normalized.

@emilwallner
Copy link

@fredguth have you made any progress on this?

@fredguth
Copy link
Author

I stopped working on this. Really sorry. I actually did some progress but I would need to have some spare time to check what is PRable.

@fredguth
Copy link
Author

Maybe we should close the issue and I reopen when I can PR.

@asturur
Copy link
Member

asturur commented Oct 14, 2018

the issue is ok open, it gets closed when is incomplete or does not make sense. Can be in progress forever, this is a free/hobby project.

@plog
Copy link

plog commented Apr 5, 2019

Hi,
I'm also looking for a "draw smoothly" solution and found this: https://stackoverflow.com/a/40653054
The result is quite nice.
Anyone would know how to do this with Fabric ?

@asturur
Copy link
Member

asturur commented Apr 26, 2020

this functionality landed somehow in fabric with recent changes,
The pencil brush has a decimation property that will remove points too clone one to the other and give a path with less points.
Is not exactly the original idea, but is better than not having it.

@asturur asturur closed this as completed Apr 26, 2020
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

No branches or pull requests

4 participants