-
-
Notifications
You must be signed in to change notification settings - Fork 321
feat: allows scaling eraser with display and shows a dotted circumference around it #1639
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,7 @@ void main() { | |
| final List<Stroke> erased = eraser.checkForOverlappingStrokes( | ||
| _eraserPos, | ||
| strokes, | ||
| scale: 1.0 | ||
| ); | ||
|
|
||
| for (final stroke in strokesToErase) { | ||
|
|
@@ -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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
|
|
||
There was a problem hiding this comment.
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
currentToolas 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));