From fc10575dfa13688bbd39ae7fcb2da3d294b4a467 Mon Sep 17 00:00:00 2001 From: TrueDoctor Date: Sun, 2 May 2021 21:21:39 +0200 Subject: [PATCH] Improve rendering efficiency and add caching (#95) Fixes #84 *Reduce heap allocations * Add caching for rendering svgs * Deduplicate UpdateCanvas Responses --- client/web/src/components/panels/Document.vue | 2 +- client/web/src/response-handler.ts | 2 +- client/web/wasm/src/lib.rs | 3 +- core/document/src/document.rs | 62 +++++++++---------- core/document/src/layers/circle.rs | 9 ++- core/document/src/layers/folder.rs | 12 ++-- core/document/src/layers/line.rs | 9 ++- core/document/src/layers/mod.rs | 38 +++++++++--- core/document/src/layers/polyline.rs | 18 +++--- core/document/src/layers/rect.rs | 9 ++- core/document/src/layers/shape.rs | 6 +- core/document/src/response.rs | 4 +- core/editor/src/dispatcher/events.rs | 2 + core/editor/src/dispatcher/mod.rs | 20 +++++- 14 files changed, 123 insertions(+), 73 deletions(-) diff --git a/client/web/src/components/panels/Document.vue b/client/web/src/components/panels/Document.vue index e15499414..5087b5523 100644 --- a/client/web/src/components/panels/Document.vue +++ b/client/web/src/components/panels/Document.vue @@ -328,7 +328,7 @@ export default defineComponent({ }, }, mounted() { - registerResponseHandler(ResponseType["Document::UpdateCanvas"], (responseData) => { + registerResponseHandler(ResponseType["Tool::UpdateCanvas"], (responseData) => { this.viewportSvg = responseData; }); registerResponseHandler(ResponseType["Tool::SetActiveTool"], (responseData) => { diff --git a/client/web/src/response-handler.ts b/client/web/src/response-handler.ts index 1751bd20b..84e0c1ea0 100644 --- a/client/web/src/response-handler.ts +++ b/client/web/src/response-handler.ts @@ -9,7 +9,7 @@ declare global { } export enum ResponseType { - "Document::UpdateCanvas" = "Document::UpdateCanvas", + "Tool::UpdateCanvas" = "Tool::UpdateCanvas", "Document::ExpandFolder" = "Document::ExpandFolder", "Document::CollapseFolder" = "Document::CollapseFolder", "Tool::SetActiveTool" = "Tool::SetActiveTool", diff --git a/client/web/wasm/src/lib.rs b/client/web/wasm/src/lib.rs index 1fb90188b..b56451315 100644 --- a/client/web/wasm/src/lib.rs +++ b/client/web/wasm/src/lib.rs @@ -31,7 +31,6 @@ fn handle_response(response: Response) { let response_type = response.to_string(); match response { Response::Document(doc) => match doc { - DocumentResponse::UpdateCanvas { document } => send_response(response_type, &[document]), DocumentResponse::ExpandFolder { path, children } => { let children = children .iter() @@ -41,7 +40,9 @@ fn handle_response(response: Response) { send_response(response_type, &[path_to_string(path), children]) } DocumentResponse::CollapseFolder { path } => send_response(response_type, &[path_to_string(path)]), + DocumentResponse::DocumentChanged => log::error!("Wasm wrapper got request to update the document"), }, + Response::Tool(ToolResponse::UpdateCanvas { document }) => send_response(response_type, &[document]), Response::Tool(ToolResponse::SetActiveTool { tool_name }) => send_response(response_type, &[tool_name]), } } diff --git a/core/document/src/document.rs b/core/document/src/document.rs index 670f134c9..4c2e04ab5 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -37,33 +37,29 @@ impl Document { /// Renders the layer graph with the root `path` as an SVG string. /// This operation merges currently mounted folder and document_folder /// contents together. - pub fn render(&self, path: &mut Vec) -> String { + pub fn render(&mut self, path: &mut Vec, svg: &mut String) { if !self.work_mount_path.as_slice().starts_with(path) { - match &self.layer(path).unwrap().data { - LayerDataTypes::Folder(_) => (), - element => { - path.pop(); - return element.render(); - } - } + self.layer_mut(path).unwrap().render(); + path.pop(); + return; } if path.as_slice() == self.work_mount_path { - let mut out = self.document_folder(path).unwrap().render(); - out += self.work.render().as_str(); + self.document_folder_mut(path).unwrap().render(svg); + self.work.render(svg); path.pop(); - return out; } - let mut out = String::with_capacity(30); - for element in self.folder(path).unwrap().layer_ids.iter() { - path.push(*element); - out += self.render(path).as_str(); + let ids = self.folder(path).unwrap().layer_ids.clone(); + for element in ids { + path.push(element); + self.render(path, svg); } - out } /// Wrapper around render, that returns the whole document as a Response. - pub fn render_root(&self) -> DocumentResponse { - DocumentResponse::UpdateCanvas { document: self.render(&mut vec![]) } + pub fn render_root(&mut self) -> String { + let mut svg = String::new(); + self.render(&mut vec![], &mut svg); + svg } fn is_mounted(&self, mount_path: &[LayerId], path: &[LayerId]) -> bool { @@ -90,6 +86,7 @@ impl Document { /// or if the requested layer is not of type folder. /// This function respects mounted folders and will thus not contain the layers already /// present in the document if a temporary folder is mounted on top. + /// If you manually edit the folder you have to set the cache_dirty flag yourself. pub fn folder_mut(&mut self, mut path: &[LayerId]) -> Result<&mut Folder, DocumentError> { let mut root = if self.is_mounted(self.work_mount_path.as_slice(), path) { path = &path[self.work_mount_path.len()..]; @@ -119,6 +116,7 @@ impl Document { /// or if the requested layer is not of type folder. /// This function does **not** respect mounted folders and will always return the current /// state of the document, disregarding any temporary modifications. + /// If you manually edit the folder you have to set the cache_dirty flag yourself. pub fn document_folder_mut(&mut self, path: &[LayerId]) -> Result<&mut Folder, DocumentError> { let mut root = &mut self.root; for id in path { @@ -134,6 +132,7 @@ impl Document { } /// Returns a mutable reference to the layer struct at the specified `path`. + /// If you manually edit the layer you have to set the cache_dirty flag yourself. pub fn layer_mut(&mut self, path: &[LayerId]) -> Result<&mut Layer, DocumentError> { let (path, id) = split_path(path)?; self.folder_mut(path)?.layer_mut(id).ok_or(DocumentError::LayerNotFound) @@ -143,6 +142,7 @@ impl Document { pub fn set_layer(&mut self, path: &[LayerId], layer: Layer) -> Result<(), DocumentError> { let mut folder = &mut self.root; if let Ok((path, id)) = split_path(path) { + self.layer_mut(path)?.cache_dirty = true; folder = self.folder_mut(path)?; if let Some(folder_layer) = folder.layer_mut(id) { *folder_layer = layer; @@ -157,6 +157,7 @@ impl Document { /// Passing a negative `insert_index` indexes relative to the end. /// -1 is equivalent to adding the layer to the top. pub fn add_layer(&mut self, path: &[LayerId], layer: Layer, insert_index: isize) -> Result { + let _ = self.layer_mut(path).map(|x| x.cache_dirty = true); let folder = self.folder_mut(path)?; folder.add_layer(layer, insert_index).ok_or(DocumentError::IndexOutOfBounds) } @@ -164,6 +165,7 @@ impl Document { /// Deletes the layer specified by `path`. pub fn delete(&mut self, path: &[LayerId]) -> Result<(), DocumentError> { let (path, id) = split_path(path)?; + let _ = self.layer_mut(path).map(|x| x.cache_dirty = true); self.document_folder_mut(path)?.remove_layer(id)?; Ok(()) } @@ -193,7 +195,7 @@ impl Document { Operation::AddCircle { path, insert_index, cx, cy, r, style } => { self.add_layer(&path, Layer::new(LayerDataTypes::Circle(layers::Circle::new(kurbo::Point::new(cx, cy), r, style))), insert_index)?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::AddRect { path, @@ -210,7 +212,7 @@ impl Document { insert_index, )?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::AddLine { path, @@ -227,13 +229,13 @@ impl Document { insert_index, )?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::AddPen { path, insert_index, points, style } => { let points: Vec = points.into_iter().map(|it| it.into()).collect(); let polyline = PolyLine::new(points, style); self.add_layer(&path, Layer::new(LayerDataTypes::PolyLine(polyline)), insert_index)?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::AddShape { path, @@ -248,17 +250,17 @@ impl Document { let s = Shape::new(kurbo::Point::new(x0, y0), kurbo::Vec2 { x: x0 - x1, y: y0 - y1 }, sides, style); self.add_layer(&path, Layer::new(LayerDataTypes::Shape(s)), insert_index)?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::DeleteLayer { path } => { self.delete(&path)?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::AddFolder { path } => { self.set_layer(&path, Layer::new(LayerDataTypes::Folder(Folder::default())))?; - Some(vec![self.render_root()]) + Some(vec![DocumentResponse::DocumentChanged]) } Operation::MountWorkingFolder { path } => { self.work_operations.clear(); @@ -290,17 +292,13 @@ impl Document { self.work = Folder::default(); let mut responses = vec![]; for operation in ops.into_iter().take(len) { - if let Some(op_responses) = self.handle_operation(operation)? { - for response in op_responses { - if !matches!(response, DocumentResponse::UpdateCanvas { .. }) { - responses.push(response); - } - } + if let Some(mut op_responses) = self.handle_operation(operation)? { + responses.append(&mut op_responses); } } let children = self.layer_panel(path.as_slice())?; - Some(vec![self.render_root(), DocumentResponse::ExpandFolder { path, children }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::ExpandFolder { path, children }]) } }; Ok(responses) diff --git a/core/document/src/layers/circle.rs b/core/document/src/layers/circle.rs index f1b6e6471..44aca73a8 100644 --- a/core/document/src/layers/circle.rs +++ b/core/document/src/layers/circle.rs @@ -1,6 +1,8 @@ use super::style; use super::LayerData; +use std::fmt::Write; + #[derive(Debug, Clone, Copy, PartialEq, Default)] pub struct Circle { shape: kurbo::Circle, @@ -17,13 +19,14 @@ impl Circle { } impl LayerData for Circle { - fn render(&self) -> String { - format!( + fn render(&mut self, svg: &mut String) { + let _ = write!( + svg, r#""#, self.shape.center.x, self.shape.center.y, self.shape.radius, self.style.render(), - ) + ); } } diff --git a/core/document/src/layers/folder.rs b/core/document/src/layers/folder.rs index 8d625b3e2..fa0a0f4be 100644 --- a/core/document/src/layers/folder.rs +++ b/core/document/src/layers/folder.rs @@ -2,6 +2,8 @@ use crate::{DocumentError, LayerId}; use super::{Layer, LayerData, LayerDataTypes}; +use std::fmt::Write; + #[derive(Debug, Clone, PartialEq)] pub struct Folder { next_assignment_id: LayerId, @@ -10,12 +12,10 @@ pub struct Folder { } impl LayerData for Folder { - fn render(&self) -> String { - self.layers - .iter() - .filter(|layer| layer.visible) - .map(|layer| layer.data.render()) - .fold(String::with_capacity(self.layers.len() * 30), |s, n| s + "\n" + &n) + fn render(&mut self, svg: &mut String) { + self.layers.iter_mut().for_each(|layer| { + let _ = writeln!(svg, "{}", layer.render()); + }); } } impl Folder { diff --git a/core/document/src/layers/line.rs b/core/document/src/layers/line.rs index ab9e991dc..022923bb5 100644 --- a/core/document/src/layers/line.rs +++ b/core/document/src/layers/line.rs @@ -1,6 +1,8 @@ use super::style; use super::LayerData; +use std::fmt::Write; + #[derive(Debug, Clone, Copy, PartialEq)] pub struct Line { shape: kurbo::Line, @@ -17,14 +19,15 @@ impl Line { } impl LayerData for Line { - fn render(&self) -> String { - format!( + fn render(&mut self, svg: &mut String) { + let _ = write!( + svg, r#""#, self.shape.p0.x, self.shape.p0.y, self.shape.p1.x, self.shape.p1.y, self.style.render(), - ) + ); } } diff --git a/core/document/src/layers/mod.rs b/core/document/src/layers/mod.rs index 4fd5a76b9..95f6ead16 100644 --- a/core/document/src/layers/mod.rs +++ b/core/document/src/layers/mod.rs @@ -19,7 +19,7 @@ pub mod folder; pub use folder::Folder; pub trait LayerData { - fn render(&self) -> String; + fn render(&mut self, svg: &mut String); } #[derive(Debug, Clone, PartialEq)] @@ -33,14 +33,14 @@ pub enum LayerDataTypes { } impl LayerDataTypes { - pub fn render(&self) -> String { + pub fn render(&mut self, svg: &mut String) { match self { - Self::Folder(f) => f.render(), - Self::Circle(c) => c.render(), - Self::Rect(r) => r.render(), - Self::Line(l) => l.render(), - Self::PolyLine(pl) => pl.render(), - Self::Shape(s) => s.render(), + Self::Folder(f) => f.render(svg), + Self::Circle(c) => c.render(svg), + Self::Rect(r) => r.render(svg), + Self::Line(l) => l.render(svg), + Self::PolyLine(pl) => pl.render(svg), + Self::Shape(s) => s.render(svg), } } } @@ -50,10 +50,30 @@ pub struct Layer { pub visible: bool, pub name: Option, pub data: LayerDataTypes, + pub cache: String, + pub cache_dirty: bool, } impl Layer { pub fn new(data: LayerDataTypes) -> Self { - Self { visible: true, name: None, data } + Self { + visible: true, + name: None, + data, + cache: String::new(), + cache_dirty: true, + } + } + + pub fn render(&mut self) -> &str { + if !self.visible { + return ""; + } + if self.cache_dirty { + self.cache.clear(); + self.data.render(&mut self.cache); + self.cache_dirty = false; + } + self.cache.as_str() } } diff --git a/core/document/src/layers/polyline.rs b/core/document/src/layers/polyline.rs index ffb01198c..4ff64254d 100644 --- a/core/document/src/layers/polyline.rs +++ b/core/document/src/layers/polyline.rs @@ -19,24 +19,26 @@ impl PolyLine { } impl LayerData for PolyLine { - fn render(&self) -> String { + fn render(&mut self, svg: &mut String) { if self.points.is_empty() { - return String::new(); + return; } - let points = self.points.iter().fold(String::new(), |mut acc, p| { - let _ = write!(&mut acc, " {:.3} {:.3}", p.x, p.y); - acc + let _ = write!(svg, r#""#, &points[1..], self.style.render()) + let _ = write!(svg, r#"" {}/>"#, self.style.render()); } } #[test] fn polyline_should_render() { - let polyline = PolyLine { + let mut polyline = PolyLine { points: vec![kurbo::Point::new(3.0, 4.12354), kurbo::Point::new(1.0, 5.54)], style: style::PathStyle::new(Some(style::Stroke::new(crate::color::Color::GREEN, 0.4)), None), }; - assert_eq!(r#""#, polyline.render()); + let mut svg = String::new(); + polyline.render(&mut svg); + assert_eq!(r#""#, svg); } diff --git a/core/document/src/layers/rect.rs b/core/document/src/layers/rect.rs index 469ba7c25..eada1a765 100644 --- a/core/document/src/layers/rect.rs +++ b/core/document/src/layers/rect.rs @@ -1,6 +1,8 @@ use super::style; use super::LayerData; +use std::fmt::Write; + #[derive(Debug, Clone, Copy, PartialEq)] pub struct Rect { shape: kurbo::Rect, @@ -17,14 +19,15 @@ impl Rect { } impl LayerData for Rect { - fn render(&self) -> String { - format!( + fn render(&mut self, svg: &mut String) { + let _ = write!( + svg, r#""#, self.shape.min_x(), self.shape.min_y(), self.shape.width(), self.shape.height(), self.style.render(), - ) + ); } } diff --git a/core/document/src/layers/shape.rs b/core/document/src/layers/shape.rs index 427ebdf88..488278103 100644 --- a/core/document/src/layers/shape.rs +++ b/core/document/src/layers/shape.rs @@ -3,6 +3,8 @@ use crate::shape_points; use super::style; use super::LayerData; +use std::fmt::Write; + #[derive(Debug, Clone, Copy, PartialEq)] pub struct Shape { shape: shape_points::ShapePoints, @@ -19,7 +21,7 @@ impl Shape { } impl LayerData for Shape { - fn render(&self) -> String { - format!(r#""#, self.shape, self.style.render(),) + fn render(&mut self, svg: &mut String) { + let _ = write!(svg, r#""#, self.shape, self.style.render(),); } } diff --git a/core/document/src/response.rs b/core/document/src/response.rs index e7c3f1ad0..3ca625e4b 100644 --- a/core/document/src/response.rs +++ b/core/document/src/response.rs @@ -29,7 +29,7 @@ impl fmt::Display for LayerType { #[repr(C)] // TODO - Make Copy when possible pub enum DocumentResponse { - UpdateCanvas { document: String }, + DocumentChanged, CollapseFolder { path: Vec }, ExpandFolder { path: Vec, children: Vec }, } @@ -37,7 +37,7 @@ pub enum DocumentResponse { impl fmt::Display for DocumentResponse { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { let name = match self { - DocumentResponse::UpdateCanvas { .. } => "UpdateCanvas", + DocumentResponse::DocumentChanged { .. } => "DocumentChanged", DocumentResponse::CollapseFolder { .. } => "CollapseFolder", DocumentResponse::ExpandFolder { .. } => "ExpandFolder", }; diff --git a/core/editor/src/dispatcher/events.rs b/core/editor/src/dispatcher/events.rs index a123f7ec6..2879d95e8 100644 --- a/core/editor/src/dispatcher/events.rs +++ b/core/editor/src/dispatcher/events.rs @@ -33,6 +33,7 @@ pub enum Event { #[repr(C)] pub enum ToolResponse { SetActiveTool { tool_name: String }, + UpdateCanvas { document: String }, } impl fmt::Display for ToolResponse { @@ -41,6 +42,7 @@ impl fmt::Display for ToolResponse { let name = match_variant_name!(match (self) { SetActiveTool, + UpdateCanvas, }); formatter.write_str(name) diff --git a/core/editor/src/dispatcher/mod.rs b/core/editor/src/dispatcher/mod.rs index f8a608ea3..df86a9c64 100644 --- a/core/editor/src/dispatcher/mod.rs +++ b/core/editor/src/dispatcher/mod.rs @@ -107,13 +107,29 @@ impl Dispatcher { } } - let (tool_responses, operations) = editor_state + let (mut tool_responses, operations) = editor_state .tool_state .tool_data .active_tool()? .handle_input(event, &editor_state.document, &editor_state.tool_state.document_tool_data); - let document_responses = self.dispatch_operations(&mut editor_state.document, operations); + let mut document_responses = self.dispatch_operations(&mut editor_state.document, operations); + //let changes = document_responses.drain_filter(|x| x == DocumentResponse::DocumentChanged); + let mut canvas_dirty = false; + let mut i = 0; + while i < document_responses.len() { + if matches!(document_responses[i], DocumentResponse::DocumentChanged) { + canvas_dirty = true; + document_responses.remove(i); + } else { + i += 1; + } + } + if canvas_dirty { + tool_responses.push(ToolResponse::UpdateCanvas { + document: editor_state.document.render_root(), + }) + } self.dispatch_responses(tool_responses); self.dispatch_responses(document_responses);