WIP: Eraser implementation #7

Draft
Franciman wants to merge 8 commits from eraserhead into master
2 changed files with 194 additions and 67 deletions
Showing only changes of commit be11d8e6ee - Show all commits

View File

@ -36,16 +36,61 @@ impl CanvasElement {
}
}
use im::Vector;
// O(1) complexity
fn segment_intersects_rect(segment: &druid::kurbo::PathSeg, rect: druid::Rect) -> bool {
match segment {
druid::kurbo::PathSeg::Line(line) => rect.contains(line.p0)
|| rect.contains(line.p1),
druid::kurbo::PathSeg::Quad(quad) => rect.contains(quad.p0)
|| rect.contains(quad.p1)
|| rect.contains(quad.p2),
druid::kurbo::PathSeg::Cubic(cubic) => rect.contains(cubic.p0)
|| rect.contains(cubic.p1)
|| rect.contains(cubic.p2),
}
}

I'm not sure about this code: if a line intersects a Rect, but the line start and end are outside of the rect, will this return true?

I'm not sure about this code: if a line intersects a Rect, but the line start and end are outside of the rect, will this return true?
// A canvas contains all elements to be drawn
pub type Canvas = Vector<CanvasElement>;
#[derive(Clone, druid::Data)]
pub struct Canvas {
pub elements: im::Vector<CanvasElement>,
}
impl Canvas {
pub fn new() -> Canvas {
Canvas {
elements: im::vector![],
}
}
// O(n) complexity, where n is the number of segments belonging to paths
// that intersect the eraser rectangle
pub fn erase(&mut self, eraser_rect: druid::Rect) {
for elem in self.elements.iter_mut() {
if elem.bounding_box().intersect(eraser_rect).area() > 0.0 {
match elem {
CanvasElement::Freehand{path, thickness: _} => {
// Remove segments intersecting the eraser
let new_segments = (*path).kurbo_path.segments().filter(|path| ! segment_intersects_rect(path, eraser_rect));
path.kurbo_path = druid::kurbo::BezPath::from_path_segments(new_segments);

I think we should split the various parts in more Canvas::Freehand parts. This way, when we implement selection etc, we can identify paths with a single connected stroke

I think we should split the various parts in more Canvas::Freehand parts. This way, when we implement selection etc, we can identify paths with a single connected stroke
}
}
}
}
}
pub fn push_back(&mut self, element: CanvasElement) {

Maybe "add_element"?

Maybe "add_element"?
self.elements.push_back(element);
}
}
#[derive(Clone, druid::Data)]
pub struct VersionedCanvas {
// We internally guarantee that this vector
// is never empty
versions: Vector<Canvas>,
versions: im::Vector<Canvas>,
curr_version: usize,
}
@ -59,7 +104,6 @@ impl VersionedCanvas {
// Get current canvas version
pub fn get(&self) -> &Canvas {
let focus = self.versions.focus();
self.versions.get(self.curr_version).unwrap()
}
@ -71,6 +115,10 @@ impl VersionedCanvas {
self.curr_version > 0
}
pub fn version(&self) -> usize {
self.curr_version
}
pub fn undo(&mut self) {
if self.has_older_versions() {
self.curr_version = self.curr_version - 1;

View File

@ -14,34 +14,121 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
use druid::im::{vector};
use druid::kurbo::BezPath;
use druid::kurbo::{BezPath, Rect};
use druid::widget::prelude::*;
use druid::{AppLauncher, Color, Data, Event, LocalizedString, WindowDesc};
use stiletto::{CanvasElement, VersionedCanvas, Canvas};
// Tools that can be used to interact with the canvas
#[derive(Clone, Data)]
enum CanvasTool {
Pen { current_element: Option<CanvasElement> },

I'm not sure that current_element should be associated with the Pen; a Pen should have thickness and color informations only

I'm not sure that current_element should be associated with the Pen; a Pen should have thickness and color informations only
Review

When does the current path being drawn make sense?
I felt a little uncomfortable keeping this in CanvasData, because there is no current path being drawn when we erase. Probably with the state machine we can achieve a cleaner vision, though.

When does `the current path being drawn` make sense? I felt a little uncomfortable keeping this in CanvasData, because there is no current path being drawn when we erase. Probably with the state machine we can achieve a cleaner vision, though.
Review

It still fits the CanvasData, because it is a piece of data associated to the Canvas (indeed, the currently drawn item). When there is no currently drawn item, is set to None; for the time being, current_element != None is the result of is_drawing()

It still fits the CanvasData, because it is a piece of data associated to the Canvas (indeed, the currently drawn item). When there is no currently drawn item, is set to None; for the time being, current_element != None is the result of is_drawing()
Eraser { eraser_rect: Option<Rect> },
}
impl CanvasTool {
fn new_pen() -> CanvasTool {
CanvasTool::Pen { current_element: None }
}
fn new_eraser() -> CanvasTool {
CanvasTool::Eraser { eraser_rect: None }
}
}
#[derive(Clone, Data)]
struct CanvasData {
current_element: Option<CanvasElement>,
//elements: Vector<CanvasElement>,
current_tool: CanvasTool,
enrico marked this conversation as resolved
Review

Nice

Nice
elements: VersionedCanvas,
}
impl CanvasData {
fn is_drawing(&self) -> bool {
/*fn is_drawing(&self) -> bool {
self.current_element.is_some()
}*/
fn set_tool(&mut self, tool: CanvasTool) {
self.current_tool = tool;
}
fn perform_undo(&mut self) {
if !self.is_drawing() {
self.elements.undo();
}
self.elements.undo();
}
fn perform_redo(&mut self) {
if !self.is_drawing() {
self.elements.redo();
self.elements.redo();
}
fn handle_tool_event(&mut self, event: &Event) {
match &mut self.current_tool {
CanvasTool::Pen { current_element } => {
match event {
Event::MouseDown(mouse_event) => {
let mut kurbo_path = BezPath::new();
kurbo_path.move_to((mouse_event.pos.x, mouse_event.pos.y));
*current_element = Some(CanvasElement::Freehand {
path: stiletto::Path { kurbo_path },
thickness: 2.0,
});
}
Event::MouseMove(mouse_event) => {
if current_element.is_some() {
if let Some(current_element) = current_element.as_mut() {
current_element
.get_path_mut()
.unwrap()
.kurbo_path
.line_to((mouse_event.pos.x, mouse_event.pos.y));
}
}
}
Event::MouseUp(_) => {
if current_element.is_some() {
if let Some(current_element) = current_element.take() {
self.elements.update(move |canvas: &Canvas| -> Canvas {
let mut new_canvas = canvas.clone();
new_canvas.push_back(current_element);
return new_canvas;
});
}
}
}
_ => {}
}
},
CanvasTool::Eraser { eraser_rect } => {
match event {
Event::MouseDown(mouse_event) => {
let rect = druid::Rect::from_center_size(mouse_event.pos, druid::Size::new(10.0, 10.0));
*eraser_rect = Some(rect);
// Create a new undo version each time the mouse is down
self.elements.update(|canvas: &Canvas| -> Canvas {
let mut new_canvas = canvas.clone();
new_canvas.erase(rect);
return new_canvas;
});
}
Event::MouseMove(mouse_event) => {
if eraser_rect.is_some() {
let rect = druid::Rect::from_center_size(mouse_event.pos, druid::Size::new(10.0, 10.0));
*eraser_rect = Some(rect);
// We don't want too many levels of undoing
// So we make irreversible changes as long as
// the mouse is pressed.
self.elements.irreversible_update(|canvas: &mut Canvas| {
canvas.erase(rect);
});
}
}
Event::MouseUp(_) => {
*eraser_rect = None;
}
_ => {}
}
},
}
}
}
@ -50,41 +137,8 @@ struct CanvasWidget;
impl Widget<CanvasData> for CanvasWidget {
fn event(&mut self, _ctx: &mut EventCtx, event: &Event, data: &mut CanvasData, _env: &Env) {
match event {
Event::MouseDown(mouse_event) => {
let mut kurbo_path = BezPath::new();
kurbo_path.move_to((mouse_event.pos.x, mouse_event.pos.y));
data.current_element = Some(CanvasElement::Freehand {
path: stiletto::Path { kurbo_path },
thickness: 2.0,
});
}
Event::MouseMove(mouse_event) => {
if data.is_drawing() {
if let Some(current_element) = data.current_element.as_mut() {
current_element
.get_path_mut()
.unwrap()
.kurbo_path
.line_to((mouse_event.pos.x, mouse_event.pos.y));
}
}
}
Event::MouseUp(_) => {
if data.is_drawing() {
if let Some(current_element) = data.current_element.take() {
data.handle_tool_event(event);
data.elements.update(move |canvas: &Canvas| -> Canvas {
let mut new_canvas = canvas.clone();
new_canvas.push_back(current_element);
return new_canvas;
});
}
}
}
_ => {}
}
}
fn lifecycle(
@ -103,16 +157,34 @@ impl Widget<CanvasData> for CanvasWidget {
data: &CanvasData,
_env: &Env,
) {
// the current_element is moved to the elements array, no need to repaint
if old_data.is_drawing() && !data.is_drawing() {
return;
}
if data.is_drawing() {
if let Some(e) = data.current_element.as_ref() {
ctx.request_paint_rect(e.bounding_box());
match (&old_data.current_tool, &data.current_tool) {
(CanvasTool::Pen{ current_element: old_element }, CanvasTool::Pen{ current_element: new_element }) => {
// the current_element is moved to the elements array, no need to repaint
if old_element.is_some() && !new_element.is_some() {
return
}
if new_element.is_some() {
if let Some(e) = new_element.as_ref() {
ctx.request_paint_rect(e.bounding_box());
}
} else {
ctx.request_paint();
}
}
(CanvasTool::Eraser{ eraser_rect: _ }, CanvasTool::Eraser{ eraser_rect }) => {
// We just stopped erasing, no need to repaint
if let Some(rect) = eraser_rect {
ctx.request_paint_rect(*rect);
} else {
ctx.request_paint();
}
}
_ => {
// we just changed the canvas tool, there is no need to repaint
return;

I feel like we should use a state machine to identify the state the app or the canvas is in; maybe this would simplify this logic, or at least make it more explicit

I feel like we should use a state machine to identify the state the app or the canvas is in; maybe this would simplify this logic, or at least make it more explicit
}
} else {
ctx.request_paint();
}
}
@ -143,24 +215,31 @@ impl Widget<CanvasData> for CanvasWidget {
// and we only want to clear this widget's area.
ctx.fill(rect, &Color::WHITE);
for element in data.elements.get().iter() {
for element in data.elements.get().elements.iter() {

lol the name choices are unfortunate

lol the name choices are unfortunate
element.draw(ctx);
}
if let Some(element) = &data.current_element {
element.draw(ctx);
match &data.current_tool {
CanvasTool::Pen { current_element } => {
if let Some(element) = &current_element {
element.draw(ctx);
}
}
_ => {}
}
}
}
fn build_ui() -> impl Widget<CanvasData> {
use druid::widget::{Align, Button, CrossAxisAlignment, Flex, SizedBox};
let toolbar = Flex::row()
.cross_axis_alignment(CrossAxisAlignment::Center)
.with_spacer(30.0)
.with_child(
Button::new("Undo").on_click(|_ctx: &mut EventCtx, data: &mut CanvasData, _env: &Env| data.perform_undo())
)
.with_child(Button::new("Redo").on_click(|_ctx: &mut EventCtx, data: &mut CanvasData, _env: &Env| data.perform_redo()));
.with_child(Button::new("Undo").on_click(|_ctx, data: &mut CanvasData, _env| data.perform_undo()))
.with_child(Button::new("Redo").on_click(|_ctx, data: &mut CanvasData, _env| data.perform_redo()))
.with_child(Button::new("Pen").on_click(|_ctx, data: &mut CanvasData, _env| data.set_tool(CanvasTool::new_pen())))
.with_child(Button::new("Eraser").on_click(|_ctx, data: &mut CanvasData, _env| data.set_tool(CanvasTool::new_eraser())));

I think this is fine for now

I think this is fine for now
Flex::column()
.cross_axis_alignment(CrossAxisAlignment::Center)
@ -176,8 +255,8 @@ pub fn main() {
LocalizedString::new("custom-widget-demo-window-title").with_placeholder("Stiletto"),
);
let canvas_data = CanvasData {
current_element: None,
elements: VersionedCanvas::new(vector![]),
current_tool: CanvasTool::new_pen(),
elements: VersionedCanvas::new(Canvas::new()),
};
AppLauncher::with_window(window)
.use_simple_logger()