Modify PencilBrush render to allow smoother curves

fredguth commented 6 years ago

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:

Paper.js uses cubic bezier curves, while Fabric.js uses quadratic bezier curves.
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

use cubic bezier curves
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/

asturur commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

~~@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 commented 6 years ago

what is uv_chdir? look like something related to lib_uv not fabricjs

2018-02-14 15:03 GMT+01:00 Fred Guth notifications@github.com:

@asturur https://github.com/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. (/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?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kangax/fabric.js/issues/4664#issuecomment-365616109, or mute the thread https://github.com/notifications/unsubscribe-auth/ABI4QBO_8v1___p1spLJ82Pqyo5ZDMVOks5tUueugaJpZM4R2lgn .

fredguth commented 6 years ago

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

fredguth commented 6 years ago

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

asturur commented 6 years ago

https://github.com/kangax/fabric.js/pull/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 commented 5 years ago

@fredguth have you made any progress on this?

fredguth commented 5 years ago

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 commented 5 years ago

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

asturur commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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.

fabricjs / fabric.js