-
Notifications
You must be signed in to change notification settings - Fork 41
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: Erase lines point by point instead of at once #68
Comments
Hi Kushal, thanks for reaching out! This is certainly a feature that I would like to support. I don't have much time to implement it at the moment, but I can always support a PR implementation :) |
Hello @timcreatedit , Thank you for this great library ! I love how simple it is to use. I would love as well to have this erasing mode, it would be the good-to-go for me to choose Scribble as my canvas engine. Do you need/take contributions? From a quick check at your codebase I suggest this: ===> As you do the erasing in the _erasePoint method of ScribbleState _erasePoint(PointerEvent event) {
return value.copyWith.sketch(
lines: value.sketch.lines
.where(
(l) => l.points.every(
(p) =>
(event.localPosition - p.asOffset).distance >
l.width + value.selectedWidth,
),
)
.toList(),
);
} Could we add an enum eraseMode {fullLine, pointOnly} to the Notifier ? and then do a switch. If ScribbleState _eraseInPointOnlyMode(PointerEvent event) {
return value.copyWith.sketch(
lines: value.sketch.lines.map((line) {
final updatedPoints = line.points.where(
(p) => (event.localPosition - p.asOffset).distance > line.width + value.selectedWidth,
).toList();
return line.copyWith(points: updatedPoints);
}).toList(),
);
} Let me know what you think, if I missed something, what else should we take care of with this change? |
Hello @JulienElkaim, thank you so much for the nice words and your offered help! While I agree that a basic implementation of point-by-point erasing wouldn't be difficult, your solution isn't quite correct. I have so far refrained from implementing a point eraser, because it would probably be a bit unreliable. Especially when using the line simplification feature, there might be large stretches of a line that don't actually contain any points, which means the user could be confused by the eraser "not working", or suddenly erasing a bigger part of a line than expected. I think I changed my mind on this now, and I'll provide a simple implementation of it. It won't be the default, and I will just warn consumers of this package about the potential drawbacks. Maybe one day, we can have a sophisticated implementation that generates new points at the end of the resulting lines to allow for precise erasing. |
Hi @timcreatedit,
I am using you package and thanks for creating this package, this has been very usefull for me and saved my time.
I want to provide one suggestion for eraser that currently the eraser is erasing the whole line and it's not good for user experience. Like, if user wants to erase some particular part of a line then it's currently not possible.
So, I would like to suggest to add this type of feature.
Please fill free to contact me if you have any confusion on my suggestion.
Thanks & Best Regards,
Kushal
The text was updated successfully, but these errors were encountered: