limitedToCanvasSize property to brush #6719
Conversation
|
I'm not opposed to this, but i would like it to a method of base brush class, and used in all the brushes. It can't be a one thing for the pencil brush. |
src/brushes/base_brush.class.js
Outdated
| */ | ||
|
|
||
| _isOutSideCanvas: function(pointer) { | ||
| if (pointer.x < 0 || pointer.x > this.canvas.getWidth() || pointer.y < 0 || pointer.y > this.canvas.getHeight()) { |
There was a problem hiding this comment.
I would like to return this expression rather than having an if that return true or false.
src/brushes/circle_brush.class.js
Outdated
| @@ -76,6 +76,9 @@ fabric.CircleBrush = fabric.util.createClass(fabric.BaseBrush, /** @lends fabric | |||
| this._render(); | |||
| } | |||
| else { | |||
| if (this.limitedToCanvasSize === true && this._isOutSideCanvas(pointer)) { | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
in the pencil brush we are returning immediately if the condition match.
Why are we allowing the full render here?
src/brushes/base_brush.class.js
Outdated
| return; | ||
| } | ||
| fabric.BaseBrush = fabric.util.createClass( | ||
| /** @lends fabric.BaseBrush.prototype */ { |
There was a problem hiding this comment.
sending this comment one line down makes all the diff bigger than necessary.
This is kind of a problem for an open project since is less clear what has changed and makes the PR harder to review.
|
Hi @CharlesRA i left a couple of minor comments and questions. |
|
Hi ! |
CharlesRA
changed the title
|
Testing this today and merging |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Hello, do I need to do something else for this PR ? |
|
I do not remember outstanding comments, i have just been busy. |
|
Thank you for the PR, and sorry for taking so much time to handle it. |
|
I'll add a test for it, please have a look at how is done, so next time you can write one too |
|
@asturur I hwas having issues with LimitedToCanvasSize when we have zoom on Canvas. I could not draw on the whole canvas, only on a part of it. I think because we ignore zoom , when getting pointer. : Can we please include zoomed canvas support in the code and pass correct pointer in |
|
Hi all, can someone help me on how to modify the circle_brush.class in order to get an ellipse/slanted line instead of a circle as brush tip to draw the freedrawing lines? i think this would be the place to add a calligraphy kind of brush. i tried changing my min.js but it did not take the slanted line or oval as brush tip. please advise |
In fabricjs it's quite difficult to control the different tools and manage their limits. Moreover, the stackoverflow question remained unanswered.
So I added the limitedToCanvasSize property to prevent free drawing outside canvas boudaries.