~cbondurant/lipuma-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH 1/2] Implement framework for trait-object tools

Details
Message ID
<20221027141338.108892-1-conner.bondurant@triptych.me>
DKIM signature
missing
Download raw message
Patch: +199 -50
---
I hope these are properly formatted! Test patch email

 src/draw_tools/fractal_line_tool.rs | 152 ++++++++++++++++++++++++++++
 src/draw_tools/mod.rs               |   2 +
 src/draw_tools/tool.rs              |  17 ++++
 src/graphics_scene_widget.rs        |  69 ++++---------
 src/main.rs                         |   1 +
 src/renderobject.rs                 |   8 ++
 6 files changed, 199 insertions(+), 50 deletions(-)
 create mode 100644 src/draw_tools/fractal_line_tool.rs
 create mode 100644 src/draw_tools/mod.rs
 create mode 100644 src/draw_tools/tool.rs

diff --git a/src/draw_tools/fractal_line_tool.rs b/src/draw_tools/fractal_line_tool.rs
new file mode 100644
index 0000000..944ea0a
--- /dev/null
+++ b/src/draw_tools/fractal_line_tool.rs
@@ -0,0 +1,152 @@
use druid::{Data, Point};
use noise::OpenSimplex;
use rand::random;
use std::rc::Rc;

use super::tool::Tool;
use crate::{fractal_line::FractalLine, renderobject::RenderObject};

#[derive(Data, Clone, PartialEq, Eq)]
enum ToolState {
	Drawing,
	Standby,
}

#[derive(Data, Clone)]
pub struct FractalLineTool {
	preview: FractalLine,
	state: ToolState,
}

impl FractalLineTool {
	pub fn new() -> Self {
		Self {
			preview: FractalLine {
				start: Point::ZERO,
				end: Point::ZERO,
				noise: Rc::new(OpenSimplex::new(0)),
				width: 10.0,
				density: 0.05,
				samples: 1000,
			},
			state: ToolState::Standby,
		}
	}
}

impl Tool for FractalLineTool {
	fn enable(&mut self, _data: &mut crate::graphics_scene_widget::GraphicsData) {
		self.state = ToolState::Standby;
	}

	fn disable(&mut self, data: &mut crate::graphics_scene_widget::GraphicsData) {
		match self.state {
			ToolState::Drawing => {
				// get_preview always returns some when drawing
				data.objects.insert(self.get_preview().unwrap());
			}
			ToolState::Standby => (),
		}
	}

	fn on_mouse_move(
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		match self.state {
			ToolState::Drawing => {
				ctx.set_handled();
				self.preview.end = event.pos;
			}
			ToolState::Standby => (),
		}
	}

	fn on_mouse_down(
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		self.state = ToolState::Drawing;
		self.preview = FractalLine {
			start: event.pos,
			end: event.pos,
			noise: Rc::new(OpenSimplex::new(random())),
			width: 10.0,
			density: 0.05,
			samples: 1000,
		};
		ctx.set_handled();
	}

	fn on_mouse_up(
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		match self.state {
			ToolState::Drawing => {
				self.preview.end = event.pos;
				let mut obj = self.get_preview().unwrap();
				obj.z = match data.objects.get_max() {
					Some(obj) => obj.z + 1,
					None => 0,
				};
				self.state = ToolState::Standby;
				data.objects.insert(obj);
				ctx.is_handled();
			}
			ToolState::Standby => (),
		}
	}

	fn on_mouse_wheel(
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		()
	}

	fn on_key_down(
		&mut self,
		_event: &druid::KeyEvent,
		_ctx: &mut druid::EventCtx,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		()
	}

	fn on_key_up(
		&mut self,
		_event: &druid::KeyEvent,
		_ctx: &mut druid::EventCtx,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		()
	}

	fn on_paste(
		&mut self,
		_event: &druid::Clipboard,
		_ctx: &mut druid::EventCtx,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		()
	}

	fn get_preview(&self) -> Option<crate::renderobject::RenderObject> {
		match self.state {
			ToolState::Drawing => Some(RenderObject::new(
				u32::MAX,
				Rc::new(Box::new(self.preview.clone())),
			)),
			ToolState::Standby => None,
		}
	}
}
diff --git a/src/draw_tools/mod.rs b/src/draw_tools/mod.rs
new file mode 100644
index 0000000..033ef91
--- /dev/null
+++ b/src/draw_tools/mod.rs
@@ -0,0 +1,2 @@
pub mod fractal_line_tool;
pub mod tool;
diff --git a/src/draw_tools/tool.rs b/src/draw_tools/tool.rs
new file mode 100644
index 0000000..9aec10a
--- /dev/null
+++ b/src/draw_tools/tool.rs
@@ -0,0 +1,17 @@
use crate::{renderobject::RenderObject, GraphicsData};
use druid::{Clipboard, EventCtx, KeyEvent, MouseEvent};

pub trait Tool {
	fn enable(&mut self, data: &mut GraphicsData);
	fn disable(&mut self, data: &mut GraphicsData);

	fn on_mouse_move(&mut self, event: &MouseEvent, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn on_mouse_down(&mut self, event: &MouseEvent, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn on_mouse_up(&mut self, event: &MouseEvent, ctx: &mut EventCtx, data: &mut GraphicsData);

	fn on_mouse_wheel(&mut self, event: &MouseEvent, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn on_key_down(&mut self, event: &KeyEvent, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn on_key_up(&mut self, event: &KeyEvent, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn on_paste(&mut self, event: &Clipboard, ctx: &mut EventCtx, data: &mut GraphicsData);
	fn get_preview(&self) -> Option<RenderObject>;
}
diff --git a/src/graphics_scene_widget.rs b/src/graphics_scene_widget.rs
index 7433c92..1fc6b26 100644
--- a/src/graphics_scene_widget.rs
+++ b/src/graphics_scene_widget.rs
@@ -10,8 +10,9 @@ use druid::{Data, Widget};

use noise::OpenSimplex;

use crate::draw_tools::fractal_line_tool::FractalLineTool;
use crate::draw_tools::tool::Tool;
use crate::drawable::Drawable;
use crate::fractal_line::FractalLine;
use crate::renderobject::RenderObject;

#[derive(Data, Clone, Debug)]
@@ -20,7 +21,7 @@ pub struct Line(Point, Point, Rc<OpenSimplex>);
#[derive(Data, Clone)]
pub struct GraphicsData {
	pub objects: OrdSet<RenderObject>,
	pub preview: Option<FractalLine>,
	pub preview: Option<RenderObject>,
}

pub enum GraphicsEngineState {
@@ -32,6 +33,7 @@ pub struct GraphicsWidget {
	pub state: GraphicsEngineState,
	change_list: OrdSet<RenderObject>,
	remove_list: Vector<RenderObject>,
	current_tool: Rc<Box<dyn Tool>>,
}

impl GraphicsWidget {
@@ -40,6 +42,7 @@ impl GraphicsWidget {
			state: GraphicsEngineState::Default,
			change_list: OrdSet::new(),
			remove_list: Vector::new(),
			current_tool: Rc::new(Box::new(FractalLineTool::new())),
		}
	}

@@ -66,49 +69,22 @@ impl Widget<GraphicsData> for GraphicsWidget {
	) {
		match event {
			druid::Event::WindowConnected => {}
			druid::Event::MouseDown(event) => match self.state {
				GraphicsEngineState::Default => {
					self.enter_state(GraphicsEngineState::Drawing);
					data.preview = Some(FractalLine {
						start: event.pos,
						end: event.pos,
						noise: Rc::new(OpenSimplex::new(rand::random())),
						width: 10.0,
						density: 0.05,
						samples: 1000,
					});
				}
				GraphicsEngineState::Drawing => (),
			},
			druid::Event::MouseUp(_) => match self.state {
				GraphicsEngineState::Default => (),
				GraphicsEngineState::Drawing => {
					data.objects.insert(RenderObject {
						transform: Affine::scale(1.0),
						drawable: Rc::new(Box::new(data.preview.take().unwrap())),
						z: match data.objects.get_max() {
							Some(v) => v.z + 1,
							None => 0,
						},
					});

					data.preview = None;
					self.enter_state(GraphicsEngineState::Default);
				}
			},
			druid::Event::MouseMove(event) => {
				if let GraphicsEngineState::Drawing = self.state {
					if let Some(preview) = &mut data.preview {
						preview.end = event.pos;
					}
				}
			}
			druid::Event::MouseDown(event) => Rc::get_mut(&mut self.current_tool)
				.unwrap()
				.on_mouse_down(event, ctx, data),
			druid::Event::MouseUp(event) => Rc::get_mut(&mut self.current_tool)
				.unwrap()
				.on_mouse_up(event, ctx, data),
			druid::Event::MouseMove(event) => Rc::get_mut(&mut self.current_tool)
				.unwrap()
				.on_mouse_move(event, ctx, data),
			druid::Event::WindowSize(_) => {
				// Need to request full repaint to ensure everything draws correctly
				ctx.request_paint();
			}
			_ => (),
		}
		data.preview = self.current_tool.get_preview();
	}

	fn lifecycle(
@@ -162,18 +138,11 @@ impl Widget<GraphicsData> for GraphicsWidget {

		if let (Some(old), Some(new)) = (&old_data.preview, &data.preview) {
			if !old.same(new) {
				self.change_list.insert(RenderObject {
					transform: Affine::scale(1.0),
					drawable: Rc::new(Box::new(new.clone())),
					z: match data.objects.get_max() {
						Some(v) => v.z + 1,
						None => 0,
					},
				});
				self.change_list.insert(new.clone());
			}

			ctx.request_paint_rect(old.AABB());
			ctx.request_paint_rect(new.AABB());
			ctx.request_paint_rect(old.get_drawable().AABB());
			ctx.request_paint_rect(new.get_drawable().AABB());
		}
	}

@@ -215,7 +184,7 @@ impl Widget<GraphicsData> for GraphicsWidget {
			robj.paint(ctx, env);
		}
		if let Some(line) = &data.preview {
			line.paint(ctx, env, &Affine::rotate(0.0));
			line.paint(ctx, env);
		}
		ctx.restore().unwrap();
		self.change_list.clear();
diff --git a/src/main.rs b/src/main.rs
index 1418d97..9a77fd3 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2,6 +2,7 @@ use druid::im::ordset;
use druid::{theme, AppLauncher, Color, PlatformError, Widget, WindowDesc};

mod bound;
mod draw_tools;
mod drawable;
mod fractal_line;
mod graphics_scene_widget;
diff --git a/src/renderobject.rs b/src/renderobject.rs
index b2847d1..f3ba3d2 100644
--- a/src/renderobject.rs
+++ b/src/renderobject.rs
@@ -41,6 +41,14 @@ impl RenderObject {
		});
	}

	pub fn new(z: u32, drawable: Rc<Box<dyn Drawable>>) -> Self {
		Self {
			z,
			transform: Affine::new([1.0, 0.0, 0.0, 1.0, 0.0, 0.0]),
			drawable,
		}
	}

	pub fn get_drawable(&self) -> Rc<Box<dyn Drawable>> {
		Rc::clone(&self.drawable)
	}
-- 
2.34.1

[PATCH 2/2] Appendectomy of code no longer needed

Details
Message ID
<20221027141338.108892-2-conner.bondurant@triptych.me>
In-Reply-To
<20221027141338.108892-1-conner.bondurant@triptych.me> (view parent)
DKIM signature
missing
Download raw message
Patch: +5 -26
---
 src/draw_tools/fractal_line_tool.rs | 10 +++++-----
 src/graphics_scene_widget.rs        | 21 ---------------------
 2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/src/draw_tools/fractal_line_tool.rs b/src/draw_tools/fractal_line_tool.rs
index 944ea0a..aa4b371 100644
--- a/src/draw_tools/fractal_line_tool.rs
+++ b/src/draw_tools/fractal_line_tool.rs
@@ -53,7 +53,7 @@ impl Tool for FractalLineTool {
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		match self.state {
			ToolState::Drawing => {
@@ -68,7 +68,7 @@ impl Tool for FractalLineTool {
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		self.state = ToolState::Drawing;
		self.preview = FractalLine {
@@ -106,9 +106,9 @@ impl Tool for FractalLineTool {

	fn on_mouse_wheel(
		&mut self,
		event: &druid::MouseEvent,
		ctx: &mut druid::EventCtx,
		data: &mut crate::graphics_scene_widget::GraphicsData,
		_event: &druid::MouseEvent,
		_ctx: &mut druid::EventCtx,
		_data: &mut crate::graphics_scene_widget::GraphicsData,
	) {
		()
	}
diff --git a/src/graphics_scene_widget.rs b/src/graphics_scene_widget.rs
index 1fc6b26..433d10f 100644
--- a/src/graphics_scene_widget.rs
+++ b/src/graphics_scene_widget.rs
@@ -2,7 +2,6 @@ use std::rc::Rc;

use druid::im::OrdSet;
use druid::im::Vector;
use druid::Affine;
use druid::Color;
use druid::Point;
use druid::RenderContext;
@@ -12,7 +11,6 @@ use noise::OpenSimplex;

use crate::draw_tools::fractal_line_tool::FractalLineTool;
use crate::draw_tools::tool::Tool;
use crate::drawable::Drawable;
use crate::renderobject::RenderObject;

#[derive(Data, Clone, Debug)]
@@ -24,13 +22,7 @@ pub struct GraphicsData {
	pub preview: Option<RenderObject>,
}

pub enum GraphicsEngineState {
	Default,
	Drawing,
}

pub struct GraphicsWidget {
	pub state: GraphicsEngineState,
	change_list: OrdSet<RenderObject>,
	remove_list: Vector<RenderObject>,
	current_tool: Rc<Box<dyn Tool>>,
@@ -39,24 +31,11 @@ pub struct GraphicsWidget {
impl GraphicsWidget {
	pub fn new() -> Self {
		Self {
			state: GraphicsEngineState::Default,
			change_list: OrdSet::new(),
			remove_list: Vector::new(),
			current_tool: Rc::new(Box::new(FractalLineTool::new())),
		}
	}

	fn enter_state(&mut self, new_state: GraphicsEngineState) {
		self.exit_state();
		self.state = new_state;
	}

	fn exit_state(&self) {
		match self.state {
			GraphicsEngineState::Default => (),
			GraphicsEngineState::Drawing => (),
		}
	}
}

impl Widget<GraphicsData> for GraphicsWidget {
-- 
2.34.1
Details
Message ID
<d3ded2822d3a1e676ca8ee7449346ea8c8f663fa.camel@ntietz.com>
In-Reply-To
<20221027141338.108892-1-conner.bondurant@triptych.me> (view parent)
DKIM signature
missing
Download raw message
This looks pretty good to me. I like the pattern of dispatching to the
tool object, which will then know how to handle the events. I wonder if
all of the events are necessary—are things like mousewheel in there for
future expansion? I left one suggestion that I think could make it
simpler, but this way is good (and of course, your project not mine!).

> ---
> I hope these are properly formatted! Test patch email

Seemed to work!

> +#[derive(Data, Clone, PartialEq, Eq)]
> +enum ToolState {
> +       Drawing,
> +       Standby,
> +}

For things like this I usually like to derive Debug, too.

> +
> +pub trait Tool {
> +       fn enable(&mut self, data: &mut GraphicsData);
> +       fn disable(&mut self, data: &mut GraphicsData);
> +
> +       fn on_mouse_move(&mut self, event: &MouseEvent, ctx: &mut
> EventCtx, data: &mut GraphicsData);
> +       fn on_mouse_down(&mut self, event: &MouseEvent, ctx: &mut
> EventCtx, data: &mut GraphicsData);
> +       fn on_mouse_up(&mut self, event: &MouseEvent, ctx: &mut
> EventCtx, data: &mut GraphicsData);
> +
> +       fn on_mouse_wheel(&mut self, event: &MouseEvent, ctx: &mut
> EventCtx, data: &mut GraphicsData);
> +       fn on_key_down(&mut self, event: &KeyEvent, ctx: &mut
> EventCtx, data: &mut GraphicsData);
> +       fn on_key_up(&mut self, event: &KeyEvent, ctx: &mut EventCtx,
> data: &mut GraphicsData);
> +       fn on_paste(&mut self, event: &Clipboard, ctx: &mut EventCtx,
> data: &mut GraphicsData);

Right now you have to implement a bunch of methods on each struct that
is Tool, even if it won't use them. If you add another method to the
trait, it'll have to also be added on all the Tools.

Did you consider instead creating one handler method, something like
`on_event`, and an `Event` enum that wraps around the other events?
Then Tool impls could switch on Event and choose which which events to
match on, and ignore the other ones. That could make some of the Tool
implementations simpler if they're not going to all use all the events.
Details
Message ID
<d9a7a676-c1f5-5b72-b022-e0576da87fc2@triptych.me>
In-Reply-To
<d3ded2822d3a1e676ca8ee7449346ea8c8f663fa.camel@ntietz.com> (view parent)
DKIM signature
missing
Download raw message
> Right now you have to implement a bunch of methods on each struct that
> is Tool, even if it won't use them. If you add another method to the
> trait, it'll have to also be added on all the Tools.
> 
> Did you consider instead creating one handler method, something like
> `on_event`, and an `Event` enum that wraps around the other events?
> Then Tool impls could switch on Event and choose which which events to
> match on, and ignore the other ones. That could make some of the Tool
> implementations simpler if they're not going to all use all the events.

I had definitely thought about using one big match statement, though it 
felt a little bit messy to do it that way. Messy in that I wouldn't be 
able to hop as easily to a specific handler in the tool.

Though now that I'm thinking about it some more I really could have just 
had the event handler function call out to individual handler functions 
that arn't exposed through the trait interface, which would be the best 
of both worlds.

Time to make a v3 patch I guess :)
Reply to thread Export thread (mbox)