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

Erase faster to increase eraser size #1235

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

PerfectlyInternal
Copy link

Eraser size now scales linearly with pen speed. Implements #819.

@PerfectlyInternal PerfectlyInternal marked this pull request as ready for review September 25, 2024 15:33
@Doublonmousse
Copy link
Collaborator

Doublonmousse commented Sep 29, 2024

Trying this, I feel like there is some additional work to be done.

There is no upper limit to the eraser size. When switching from the pen to the eraser, the initial size can sometimes be too large, even exceeding the visible canvas size (and delete a whole document instead of staying as a small eraser). It might be worse on some devices (an initial small displacement over a very small timestep will give a velocity estimation with very large error bounds, and that timestep might be 1/60 s on some pens down to 1/480 s. Hence the potential very large overestimate even with the initial smoothing). This should be fixed.
An upper bound on speed or eraser size should also be here.
Maybe we can try to re use the ink stroke modeler for this (but output the velocity estimation instead) or do some kalman based filtering (where we can model the error on the speed estimation and act on it if necessary).

Edit : issue from below. However setting an upper bound might not be a bad idea anyway.

The other thing I'm wondering is that on other software that scales the eraser size depending on speed, the actual behavior is not doing a speed estimation and scaling the eraser size with respect to that. There is some inertia to it, or some way to lock or decrease smoothly or in steps the eraser size when the speed decreases.
It might be more akin to a "erase faster means getting the next eraser size and locking it to that size until you stop or slow down" situation.
Realistically, we're not obliged to follow suit on that, but we should come up with a behavior that makes sense and will satisfy the original feature request.

@PerfectlyInternal
Copy link
Author

In the video shown in the issue, the eraser seems to step in size as the user moves quickly for a while, then gradually step down once the user has stopped moving. This could be implemented either as smoothing over a very long period of time, so the eraser needs to be moved quickly for a while in order to get to its full size, or by checking if the eraser has been moving fast for at least X ms, then stepping up the size. Which one do you think would be better?

@Doublonmousse
Copy link
Collaborator

I'm still getting the same issue with the start size being sometimes too large then going down. I will try to spend some times logging things to check where this comes from.

Doing things when the user stops moving is also a little harder to do. With pens, you always have some jitter hence events continue happening (but events stop happening if you stop with a mouse). This is the same issue I had with #1175 (needing a separate thread to call changes in the future and being cautious not to have more than one event fire at the same time because of it). But maybe in that case that's okay not to do that (as you trigger the change of size with movement, not stopping the pen)

Maybe we can use a better designed speed to eraser size function (somewhat of a stepped function so you get the same size for some range of speed) ?
The other idea I had was to add a size to the EraserMotion structure and have it be updated by some well chosen function like size = f(size, speed) so that we get some hysteresis-like behavior (lock/converge to some size levels and change value significantly only for large/slow speed).

Honestly I don't have a perfect answer, though I want to check whether if we can get what we want with the current strategy (smoothing). I don't know what's the better approach here.

@PerfectlyInternal
Copy link
Author

The eraser now needs to exceed some minimum speed in order to start scaling up, and scaling is now a stepped function with hysteresis. The exact values to trigger stepping up/down can be tweaked, so we'll want to play around with those to get something that feels good to use. It might even be a good idea to expose them as config options for the user, since different users will probably have different speeds at which they'll want to trigger eraser scaling.

We still need some way of detecting a lack of motion, since events stop firing when you hold still, even on a drawing tablet. This will need to be faster than the long press detector you already have, but the underlying logic should be similar.

@Doublonmousse
Copy link
Collaborator

@PerfectlyInternal
Copy link
Author

FIxed it, thanks!

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.

2 participants