Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion lib/components/canvas/_canvas_painter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:saber/components/canvas/_rectangle_stroke.dart';
import 'package:saber/components/canvas/_stroke.dart';
import 'package:saber/data/editor/page.dart';
import 'package:saber/data/extensions/color_extensions.dart';
import 'package:saber/data/tools/eraser.dart';
import 'package:saber/data/tools/highlighter.dart';
import 'package:saber/data/tools/laser_pointer.dart';
import 'package:saber/data/tools/select.dart';
Expand Down Expand Up @@ -55,6 +56,7 @@ class CanvasPainter extends CustomPainter {
_drawCurrentStroke(canvas);
_drawDetectedShape(canvas);
_drawSelection(canvas);
_drawEraserIndicator(canvas);
_drawPageIndicator(canvas, size);
}

Expand All @@ -74,7 +76,8 @@ class CanvasPainter extends CustomPainter {
showPageIndicator != oldDelegate.showPageIndicator ||
pageIndex != oldDelegate.pageIndex ||
totalPages != oldDelegate.totalPages ||
currentScale != oldDelegate.currentScale;
currentScale != oldDelegate.currentScale ||
page.eraserPosition != oldDelegate.page.eraserPosition;
}

void _drawHighlighterStrokes(Canvas canvas, Rect canvasRect) {
Expand Down Expand Up @@ -250,6 +253,25 @@ class CanvasPainter extends CustomPainter {
);
}

void _drawEraserIndicator(Canvas canvas) {
if (page.eraserPosition == null) return;
final radius = Eraser().size / currentScale;

final path = Path()
..addOval(Rect.fromCircle(center: page.eraserPosition!, radius: radius));
Comment on lines +256 to +261
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better approach would be to store the eraser position in the Eraser object.
The canvas painter should take in currentTool as an argument, then we can do something like this:

   void _drawEraserIndicator(Canvas canvas) {
-    if (page.eraserPosition == null) return;
-    final radius = Eraser().size / currentScale;
+    final eraser = currentTool;
+    if (eraser is! Eraser) return;
+    final radius = eraser.size / currentScale;
    
    final path = Path()
-      ..addOval(Rect.fromCircle(center: page.eraserPosition!, radius: radius));
+      ..addOval(Rect.fromCircle(center: eraser.position, radius: radius));


canvas.drawPath(
dashPath(
path,
dashArray: CircularIntervalList([5 / currentScale, 5 / currentScale]),
),
Paint()
..color = Colors.grey
..strokeWidth = 1.0 / currentScale
..style = PaintingStyle.stroke,
);
}

static const double _pageIndicatorFontSize = 20;
static const double _pageIndicatorPadding = 5;
void _drawPageIndicator(Canvas canvas, Size pageSize) {
Expand Down
2 changes: 2 additions & 0 deletions lib/data/editor/page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class EditorPage extends ChangeNotifier implements HasSize {
@override
final Size size;

Offset? eraserPosition;

late final CanvasKey innerCanvasKey = CanvasKey();
RenderBox? _renderBox;
RenderBox? get renderBox {
Expand Down
8 changes: 5 additions & 3 deletions lib/data/tools/eraser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ class Eraser extends Tool {
/// Returns any [strokes] that are close to the given [eraserPos].
List<Stroke> checkForOverlappingStrokes(
Offset eraserPos,
List<Stroke> strokes,
) {
List<Stroke> strokes, {
required double scale,
}) {
final effectiveSqrSize = sqrSize / square(scale);
final List<Stroke> overlapping = [];
for (int i = 0; i < strokes.length; i++) {
final stroke = strokes[i];
if (_shouldStrokeBeErased(eraserPos, stroke, sqrSize)) {
if (_shouldStrokeBeErased(eraserPos, stroke, effectiveSqrSize)) {
overlapping.add(stroke);
_erased.add(stroke);
}
Expand Down
5 changes: 5 additions & 0 deletions lib/pages/editor/editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,11 @@ class EditorState extends State<Editor> {
currentPressure,
);
} else if (currentTool is Eraser) {
page.eraserPosition = position;
for (final stroke in (currentTool as Eraser).checkForOverlappingStrokes(
position,
page.strokes,
scale: _transformationController.value.approxScale,
)) {
page.strokes.remove(stroke);
}
Expand Down Expand Up @@ -605,9 +607,11 @@ class EditorState extends State<Editor> {
(currentTool as Pen).onDragUpdate(position, currentPressure);
page.redrawStrokes();
} else if (currentTool is Eraser) {
page.eraserPosition = position;
for (final stroke in (currentTool as Eraser).checkForOverlappingStrokes(
position,
page.strokes,
scale: _transformationController.value.approxScale,
)) {
page.strokes.remove(stroke);
}
Expand Down Expand Up @@ -661,6 +665,7 @@ class EditorState extends State<Editor> {
),
);
} else if (currentTool is Eraser) {
page.eraserPosition = null;
final erased = (currentTool as Eraser).onDragEnd();
if (tmpTool != null &&
(stylusButtonPressed || stows.disableEraserAfterUse.value)) {
Expand Down
46 changes: 46 additions & 0 deletions test/tools_eraser_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ void main() {
final List<Stroke> erased = eraser.checkForOverlappingStrokes(
_eraserPos,
strokes,
scale: 1.0
);

for (final stroke in strokesToErase) {
Expand Down Expand Up @@ -86,6 +87,51 @@ void main() {
reason: 'The correct strokes should have been erased',
);
});

test('Test that eraser size scales inversely with zoom', () {
final eraser = Eraser(size: 10);
// At scale 1.0, this stroke is at the edge (distance = size)
// At scale 2.0, the effective size should be 5, so this stroke (at 10) should NOT be erased
final strokeAtEdge = _strokeWithPoint(
_eraserPos + const Offset(1, 0) * eraser.size,
);

// Check with scale 1.0
var erased = eraser.checkForOverlappingStrokes(_eraserPos, [
strokeAtEdge,
], scale: 1.0);
expect(
erased,
contains(strokeAtEdge),
reason: 'Should erase stroke at edge with scale 1.0',
);
Comment on lines +103 to +107
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use expect(erased, contains(strokeAtEdge), reason: ...) so we get better error messages when a test fails.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, there's just one you forgot to change :p


// Check with scale 2.0 (eraser should be smaller in document coordinates)
erased = eraser.checkForOverlappingStrokes(_eraserPos, [
strokeAtEdge,
], scale: 2.0);
expect(
erased.contains(strokeAtEdge),
false,
reason:
'Should NOT erase stroke at distance 10 when scale is 2.0 (effective size 5)',
);

// At scale 0.5, effective size should be 20.
// A stroke at distance 15 should be erased.
final strokeFurtherAway = _strokeWithPoint(
_eraserPos + const Offset(1.5, 0) * eraser.size,
);
erased = eraser.checkForOverlappingStrokes(_eraserPos, [
strokeFurtherAway,
], scale: 0.5);
expect(
erased,
contains(strokeFurtherAway),
reason:
'Should erase stroke at distance 15 when scale is 0.5 (effective size 20)',
);
});
}

Stroke _strokeWithPoint(Offset point) => Stroke(
Expand Down
Loading