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

Add bulk deletion variant of eraser #41

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

finnboeger
Copy link
Contributor

Again code mainly by @rdbeach.
Adds secondary function to eraser where a rectangle can be drawn and everything that is fully inside this rectangle will get deleted.

@lovasoa
Copy link
Owner

lovasoa commented Apr 26, 2020

What is this 2d.js ? I don't have anything against using an external library, but if we do so, then we should probably switch the frontend to use some kind of compilation system, and use npm for dependencies. And we have to make sure any library we use is compatible with the GPL license of this repository.

Making the frontend to a proper web application, using npm, adds some overhead, but I think it's with it. What do you think ?

@finnboeger
Copy link
Contributor Author

I'm not 100% sure, I believe it is used in the masking eraser. @rdbeach Can you explain it? Otherwise I'd work around it and implement necessary functions myself. I have removed it for now.

@finnboeger
Copy link
Contributor Author

I have adjusted the erase function to check all points within a given radius (determined by the pencil size). This fixes #8
In conjunction with a pointer that also uses the pencil size it should work quite well

@rdbeach
Copy link
Contributor

rdbeach commented Apr 27, 2020

D2 is not used for the masking eraser. It's used for the transform tool. The library is from http://www.kevlindev.com/. It had some capabilities to put handles on objects and transform them a bit. I used it as a starting point for making the transform tool. With a bit of reworking, you could probably get rid of it.

@lovasoa
Copy link
Owner

lovasoa commented Apr 27, 2020

Good that we could get rid of 2d.js. I find the tool a little bit unintuitive as it is. I think we should completely remove the original delete on hover behavior, and the tool should just :

  • on click: delete the clicked element, if any
  • on drag: start a selection

@finnboeger
Copy link
Contributor Author

I quite like the current delete function to quickly delete handwritten text. With the increased width of the eraser it works a lot better than it currently does. It would benefit from a different UI that better explains what each tool does.

@lovasoa lovasoa force-pushed the master branch 2 times, most recently from d2ff89d to 189ebbe Compare April 27, 2020 22:56
client-data/board.html Outdated Show resolved Hide resolved
Co-authored-by: Ophir LOJKINE <ophir.lojkine@auto-grid.com>
@lovasoa
Copy link
Owner

lovasoa commented May 3, 2020

The svg standard defines a function to list the elements inside a rectangle: getIntersectionList. Unfortunately, browser support isn't there yet (there is an 11 years old bug report in Firefox), but we could use it when it's there.

@lovasoa
Copy link
Owner

lovasoa commented May 3, 2020

And if you want to test your changes on large boards, you can download the ic board: http://wbo.ophir.dev/download/ic

There is also ic2, and ic3 .

@finnboeger
Copy link
Contributor Author

I am currently performing some testing with the ic board. The multi-eraser that deletes everything fully contained in a rectangular area has no issues with large boards, the regular eraser in both the click and drag variety has significant issues, locking up the CPU for several seconds. I will attempt to create a function that can determine whether the cursor is over the path of a svg shape and check if I can get it to be faster than the current solution

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.

None yet

3 participants