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

feat(Brush API): Observable, clipPath #8476

Closed
wants to merge 188 commits into from
Closed

feat(Brush API): Observable, clipPath #8476

wants to merge 188 commits into from

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Nov 28, 2022

Motivation

closes #7175
closes #7134
closes #6955
closes #5431

Description

This PR is a port of #7175.
It sets sail to the new Brush API, which IMO should not be called brush anymore.
As part of the new API, brush now extends Observable and is subscribed to all canvas events by default.
I believe this change is so good that it will unlock lots of options including moving interaction code (e.g. group selector) to a dedicated brush.
I also think freeDrawingBrush should be renamed and become an array of interactions (this will enable supporting multi touch interactions).
To allow all this I have exposed a long desired feature, FabricEvent#preventDefault. Brushes listen to the before event (of down, move, up) and prevent default in case they operate and then canvas knows not to run the default action. This is great since there are interactions that should not prevent default behavior such as visualizing pointers etc.
In order to make brush flexible in terms of how it interacts with canvas events I have exposed the subscribe method that wires the brush logic to the canvas events so subclassing this method is very powerful.

Changes

This is a big PR but not a huge one. There are a lot of goldens changed.

API changes:

  • Brush subclasses Observable
  • Observable now fires a FabricEvent instance to support preventDefault and stopPropagation
  • Brush has a subscribe method that connects between lifecycle methods to canvas events. This way a brush can start its lifecycle when contextmenu event is fired instead of when mousedown is fired w/o changing logic accept the subscribe method
  • Created SimpleBrush: a subclass of BaseBrush that is wired to interact with a simple down-move-up lifecycle
  • Exposed a common render + _render method on BaseBrush which conforms to Object#render and is in charge of:
    1. Preparing the context (transformation etc.)
    2. calling _render which is implemented by each brush
    3. render clipPath
    4. cleanup
  • enable/disable brush
  • brush doesn't add result to canvas anymore and fires interaction:completed instead

Minor Changes:

  • Added a clipPath property to BaseBrush and exposed clip path methods
  • Added logic to apply the clip path to the resulting object created by the brush
  • Moved common logic to base brush
  • private flag _isCurrentlyDrawing is now a method isCurrentlyDrawing (doesn't have to be private)
  • canvas fires resize from setDimensions, brush listens to it to call _setBrushStyles instead of canvas doing it
  • canvas down, move, up default behavior has been extracted to dedicated methods for ease coding
  • renamed _onResize => _onWindowResize (confusing)
  • moved _setDimensionsImpl to Canvas instaed of SelectableCanvas
  • removed button from TPointerEventInfo as we discussed
  • deprecated Canvas#isFreeDrawing in favor of a fine grained brush option enabled
  • Moved freedraw goldens to a dedicated folder and added tests that cover this PR

BREAKING:

  • brush doesn't add result to canvas anymore
  • path:created, before:path:created events are deprecated, use interaction:completed instead
  • _render method is now protected, use render instead
  • renamed PencilBrush#convertPointsToSVGPath => getPathFromPoints
  • Canvas#freeDrawingCursor => Brush#cursor
  • Canvas#isFreeDrawing => brush.enable/disable

Gist

In Action (from #7175)

https://codesandbox.io/s/upbeat-microservice-mkbzs?file=/src/App.tsx
STALE sandbox

Edit fabric_app (forked)

Pay no attention to eraser bug. It's fixed by the group PRs.
ezgif com-gif-maker (3)

With Eraser patched up
ezgif com-gif-maker (4)

image

absolutePositioned + viewportTransform
ezgif com-gif-maker (6)

@ShaMan123
Copy link
Contributor Author

This PR is GREAT!

@ShaMan123 ShaMan123 mentioned this pull request Feb 12, 2023
4 tasks
@ShaMan123 ShaMan123 linked an issue Feb 12, 2023 that may be closed by this pull request
4 tasks
@asturur
Copy link
Member

asturur commented May 14, 2023

@ShaMan123 i think there are some things that i agree with in this PR.
Do you mind if i start slicing them and importing in smaller units?

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented May 14, 2023

Go ahead
You should first merge #8430, or at least work with the diff against it if you don't want to work on that now.

@ShaMan123
Copy link
Contributor Author

This is was good refactor and provided a promising direction
However it has become stale
If in need I will probably rewrite what I need so that:

@ShaMan123 ShaMan123 closed this May 7, 2024
@ShaMan123 ShaMan123 deleted the clip-brush branch May 7, 2024 18:56
@asturur asturur restored the clip-brush branch May 7, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants