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(EraserBrush): V2 #7470

Merged
merged 92 commits into from
Jan 16, 2022
Merged

feat(EraserBrush): V2 #7470

merged 92 commits into from
Jan 16, 2022

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Nov 2, 2021

UPDATE
The eraser has moved https://github.com/ShaMan123/erase2d

Motivation

Solves eraser problem (opaic objects) by applying a mask over the brush.
As a side effect solved other problems such as deep clipping and erasing.
And enabled the awesome feature of undoing erasing.
While boosting performance so it seems.

Previous Drawing Logic

Render objects in 3 steps:

  1. All non-erasables on bottom on main/bottom context
  2. All erasables on top context
  3. While erasing use the path to clip step 2.
  4. Non-erasable overlay

This was great except when you had a non erasable opaic object. It would draw on step 1 and 2, doubling itself.
Trying to solve it in context would not work, somewhere it would still be doubled or missing.

Current Drawing Logic

While erasing:

  1. clip main context with path
  2. render path with a mask on top context. Mask is produced by rendering all non-erasable objects.

This approach supports inverted erasing, AKA recovering, out of the box. We simply produce a mask rendered with all erasable objects with their eraser (clip path) inverted.
Use this feature as follows:

brush.inverted = true;

Outcome

  1. Better performance
  2. EraserBrush doubles the opacity of semi-transparent non-erasable shapes during free drawing #7445 is solved
  3. animations while erasing - if object is erasable it produces a funny output (dev should handle it making the animated objects non-erasable: once feat(animation): animations registry #7528 is merged the brush can handle this case as well by default)
  4. Supports/solves eraser and recover brush for fabricjs( canvas) #5187 feat(EraserBrush): inverted brush #7271

Try it out

https://codesandbox.io/s/2hhyp
https://codesandbox.io/s/soz8h (tests erasing with inverted and nested clip paths - erase and select objects to see)
The left top circle is opaic and not erasable.

Migration from V1

The Eraser object is now a subclass of Group. This means that loading from JSON will break between versions.
Use this code to transform your json payload to the new version.

Perf

pff

Next

I've pushed the use of clipping beyond the limit of svg. It seems fabric has advanced further than svg in clipping abilities so toSVG should be amended to using masks, not clip paths in order to support all functionalities.

@ShaMan123 ShaMan123 changed the title feat(EraserBrush): pattern brush feat(EraserBrush): V2 Nov 2, 2021
render only erased parts of objects so not to double opacity of objects.
@ShaMan123
Copy link
Contributor Author

I want a way to check performance compared to prev version.
Need ideas

@ShaMan123 ShaMan123 marked this pull request as ready for review November 4, 2021 06:24
@asturur
Copy link
Member

asturur commented Nov 4, 2021

You can use the inspector of google chrome.
It will give you scripting time and FPS on the canvas.

@stale stale bot added the stale Issue marked as stale by the stale bot label Nov 19, 2021
@ShaMan123 ShaMan123 removed the stale Issue marked as stale by the stale bot label Nov 19, 2021
@stale
Copy link

stale bot commented Dec 14, 2021

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.

@stale stale bot added the stale Issue marked as stale by the stale bot label Dec 14, 2021
@ShaMan123 ShaMan123 removed the stale Issue marked as stale by the stale bot label Dec 14, 2021
@stale stale bot added the stale Issue marked as stale by the stale bot label Dec 28, 2021
@GavinLi2016
Copy link

ShaMan123, i found after erased an image. this image can not use clipPath to clip any polygon again.
can you take a see if it is my mistake. thanks
https://codepen.io/gavinli2016/project/editor/AnrdOk

@ShaMan123
Copy link
Contributor Author

What about changelog?

@ShaMan123 ShaMan123 requested a review from asturur January 16, 2022 16:06
@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Jan 16, 2022

CHANGELOG committed

asturur
asturur previously approved these changes Jan 16, 2022
@asturur
Copy link
Member

asturur commented Jan 16, 2022

Can you fix the dist folder? i tried but i can't fixing it by check out

@ShaMan123
Copy link
Contributor Author

image

@asturur
Copy link
Member

asturur commented Jan 16, 2022

try from commandline with the classic git checkout master dist ?

@ShaMan123
Copy link
Contributor Author

DONE!

@ShaMan123 ShaMan123 requested a review from asturur January 16, 2022 16:24
@asturur asturur merged commit 38cfcda into fabricjs:master Jan 16, 2022
@asturur
Copy link
Member

asturur commented Jan 16, 2022

do we need to merge the second one now?

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Jan 16, 2022

yes.
it's supposed to have no diff because I merged it into this one.

@ShaMan123 ShaMan123 deleted the eraser-v2 branch January 16, 2022 16:41
@ShaMan123
Copy link
Contributor Author

Well no. I'll shut it down

@asturur asturur mentioned this pull request Jan 26, 2022
rockerBOO pushed a commit to rockerBOO/fabric.js that referenced this pull request Jan 28, 2022
@komagic
Copy link

komagic commented Feb 7, 2022

the case still have edge when using inverted eraser,to implement the feature, only need globalCompositionOperation

@ShaMan123
Copy link
Contributor Author

@komagic if you've encountered a bug do open a bug report. There's no way to understand what you are experiencing.
And unfortunately setting globalCompositionOperation does not suffice.

@komagic
Copy link

komagic commented Feb 7, 2022

@ShaMan123
Copy link
Contributor Author

@komagic if you've encountered a bug do open a bug report. There's no way to understand what you are experiencing. And unfortunately setting globalCompositionOperation does not suffice.

my comment still stands.

@ShaMan123 ShaMan123 mentioned this pull request Dec 8, 2022
5 tasks
@ShaMan123
Copy link
Contributor Author

The eraser has moved https://github.com/ShaMan123/erase2d

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.

4 participants