timcreatedit / scribble

Scribble is a lightweight library for freehand drawing in Flutter supporting pressure, variable line width and more!
https://pub.dev/packages/scribble
MIT License
115 stars 39 forks source link

feat: Erase lines point by point instead of at once #68

Open kushal2021 opened 4 months ago

kushal2021 commented 4 months ago

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

timcreatedit commented 3 months ago

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 :)

JulienElkaim commented 1 month ago

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 ScribbleNotifier:

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 pointOnly we would execute this:

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? Happy to do a PR if all good.

timcreatedit commented 1 month ago

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. If a point is erased from anywhere but the ends of a line, the line would need to be split into two lines. Otherwise you're removing points from a line, but the line will still be rendered as going through the remaining points.

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.