From 6d3df31c54507b902266d1a85d4aa5c697e732fe Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 20 Jul 2021 00:42:08 +0200 Subject: [PATCH 01/14] Refactor rearrange layers --- core/document/src/document.rs | 34 +-- core/document/src/layers/folder.rs | 207 ------------------ core/document/src/operation.rs | 5 +- core/editor/src/communication/dispatcher.rs | 12 +- .../src/document/document_message_handler.rs | 56 ++--- core/editor/src/input/input_mapper.rs | 2 +- 6 files changed, 42 insertions(+), 274 deletions(-) diff --git a/core/document/src/document.rs b/core/document/src/document.rs index 1aa9354c6f..e45b61772e 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -27,9 +27,8 @@ impl Default for Document { } fn split_path(path: &[LayerId]) -> Result<(&[LayerId], LayerId), DocumentError> { - let id = path.last().ok_or(DocumentError::InvalidPath)?; - let folder_path = &path[0..path.len() - 1]; - Ok((folder_path, *id)) + let (id, path) = path.split_last().ok_or(DocumentError::InvalidPath)?; + Ok((path, *id)) } impl Document { @@ -221,24 +220,8 @@ impl Document { /// Deletes the layer specified by `path`. pub fn delete(&mut self, path: &[LayerId]) -> Result<(), DocumentError> { let (path, id) = split_path(path)?; - if let Ok(layer) = self.layer_mut(path) { - layer.cache_dirty = true; - } - self.document_folder_mut(path)?.as_folder_mut()?.remove_layer(id)?; - Ok(()) - } - - pub fn reorder_layers(&mut self, source_paths: &[Vec], target_path: &[LayerId]) -> Result<(), DocumentError> { - // TODO: Detect when moving between folders and handle properly - - let source_layer_ids = source_paths - .iter() - .map(|x| x.last().cloned().ok_or(DocumentError::LayerNotFound)) - .collect::, DocumentError>>()?; - - self.root.as_folder_mut()?.reorder_layers(source_layer_ids, *target_path.last().ok_or(DocumentError::LayerNotFound)?)?; - - Ok(()) + let _ = self.layer_mut(path).map(|x| x.cache_dirty = true); + self.document_folder_mut(path)?.as_folder_mut()?.remove_layer(id) } pub fn layer_axis_aligned_bounding_box(&self, path: &[LayerId]) -> Result, DocumentError> { @@ -323,10 +306,10 @@ impl Document { let (path, _) = split_path(path.as_slice()).unwrap_or_else(|_| (&[], 0)); Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.to_vec() }]) } - Operation::PasteLayer { path, layer } => { + Operation::PasteLayer { path, layer, insert_index } => { let folder = self.folder_mut(path)?; //FIXME: This clone of layer should be avoided somehow - folder.add_layer(layer.clone(), -1).ok_or(DocumentError::IndexOutOfBounds)?; + folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?; Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.clone() }]) } @@ -416,11 +399,6 @@ impl Document { self.mark_as_dirty(path)?; Some(vec![DocumentResponse::DocumentChanged]) } - Operation::ReorderLayers { source_paths, target_path } => { - self.reorder_layers(source_paths, target_path)?; - - Some(vec![DocumentResponse::DocumentChanged]) - } }; if !matches!( operation, diff --git a/core/document/src/layers/folder.rs b/core/document/src/layers/folder.rs index d57d1e0d6d..adaccc5e73 100644 --- a/core/document/src/layers/folder.rs +++ b/core/document/src/layers/folder.rs @@ -65,72 +65,6 @@ impl Folder { Ok(()) } - pub fn reorder_layers(&mut self, source_ids: Vec, target_id: LayerId) -> Result<(), DocumentError> { - let source_pos = self.position_of_layer(source_ids[0])?; - let source_pos_end = source_pos + source_ids.len() - 1; - let target_pos = self.position_of_layer(target_id)?; - - let mut last_pos = source_pos; - for layer_id in &source_ids[1..] { - let layer_pos = self.position_of_layer(*layer_id)?; - if (layer_pos as i32 - last_pos as i32).abs() > 1 { - // Selection is not contiguous - return Err(DocumentError::NonReorderableSelection); - } - last_pos = layer_pos; - } - - if source_pos < target_pos { - // Moving layers up the hierarchy - - // Prevent shifting past end - if source_pos_end + 1 >= self.layers.len() { - return Err(DocumentError::NonReorderableSelection); - } - - fn reorder_up(arr: &mut Vec, source_pos: usize, source_pos_end: usize, target_pos: usize) - where - T: Clone, - { - *arr = [ - &arr[0..source_pos], // Elements before selection - &arr[source_pos_end + 1..=target_pos], // Elements between selection end and target - &arr[source_pos..=source_pos_end], // Selection itself - &arr[target_pos + 1..], // Elements before target - ] - .concat(); - } - - reorder_up(&mut self.layers, source_pos, source_pos_end, target_pos); - reorder_up(&mut self.layer_ids, source_pos, source_pos_end, target_pos); - } else { - // Moving layers down the hierarchy - - // Prevent shifting past end - if source_pos == 0 { - return Err(DocumentError::NonReorderableSelection); - } - - fn reorder_down(arr: &mut Vec, source_pos: usize, source_pos_end: usize, target_pos: usize) - where - T: Clone, - { - *arr = [ - &arr[0..target_pos], // Elements before target - &arr[source_pos..=source_pos_end], // Selection itself - &arr[target_pos..source_pos], // Elements between selection and target - &arr[source_pos_end + 1..], // Elements before selection - ] - .concat(); - } - - reorder_down(&mut self.layers, source_pos, source_pos_end, target_pos); - reorder_down(&mut self.layer_ids, source_pos, source_pos_end, target_pos); - } - - Ok(()) - } - /// Returns a list of layers in the folder pub fn list_layers(&self) -> &[LayerId] { self.layer_ids.as_slice() @@ -213,144 +147,3 @@ impl Default for Folder { } } } - -#[cfg(test)] -mod test { - use glam::{DAffine2, DVec2}; - - use crate::layers::{style::PathStyle, Ellipse, Layer, LayerDataTypes, Line, PolyLine, Rect, Shape}; - - use super::Folder; - - #[test] - fn reorder_layers() { - let mut folder = Folder::default(); - - let identity_transform = DAffine2::IDENTITY.to_cols_array(); - folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0); - folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1); - folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2); - folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3); - folder.add_layer( - Layer::new(LayerDataTypes::PolyLine(PolyLine::new(vec![DVec2::ZERO, DVec2::ONE])), identity_transform, PathStyle::default()), - 4, - ); - - assert_eq!(folder.layer_ids[0], 0); - assert_eq!(folder.layer_ids[1], 1); - assert_eq!(folder.layer_ids[2], 2); - assert_eq!(folder.layer_ids[3], 3); - assert_eq!(folder.layer_ids[4], 4); - - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - assert!(matches!(folder.layer(4).unwrap().data, LayerDataTypes::PolyLine(_))); - - assert_eq!(folder.layer_ids.len(), 5); - assert_eq!(folder.layers.len(), 5); - - folder.reorder_layers(vec![0, 1], 2).unwrap(); - - assert_eq!(folder.layer_ids[0], 2); - // Moved layers - assert_eq!(folder.layer_ids[1], 0); - assert_eq!(folder.layer_ids[2], 1); - - assert_eq!(folder.layer_ids[3], 3); - assert_eq!(folder.layer_ids[4], 4); - - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - // Moved layers - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - assert!(matches!(folder.layer(4).unwrap().data, LayerDataTypes::PolyLine(_))); - - assert_eq!(folder.layer_ids.len(), 5); - assert_eq!(folder.layers.len(), 5); - } - - #[test] - fn reorder_layer_to_top() { - let mut folder = Folder::default(); - - let identity_transform = DAffine2::IDENTITY.to_cols_array(); - folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0); - folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1); - folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2); - folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3); - - assert_eq!(folder.layer_ids[0], 0); - assert_eq!(folder.layer_ids[1], 1); - assert_eq!(folder.layer_ids[2], 2); - assert_eq!(folder.layer_ids[3], 3); - - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - - assert_eq!(folder.layer_ids.len(), 4); - assert_eq!(folder.layers.len(), 4); - - folder.reorder_layers(vec![1], 3).unwrap(); - - assert_eq!(folder.layer_ids[0], 0); - assert_eq!(folder.layer_ids[1], 2); - assert_eq!(folder.layer_ids[2], 3); - // Moved layer - assert_eq!(folder.layer_ids[3], 1); - - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - // Moved layer - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - - assert_eq!(folder.layer_ids.len(), 4); - assert_eq!(folder.layers.len(), 4); - } - - #[test] - fn reorder_non_contiguous_selection() { - let mut folder = Folder::default(); - - let identity_transform = DAffine2::IDENTITY.to_cols_array(); - folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0); - folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1); - folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2); - folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3); - - assert_eq!(folder.layer_ids[0], 0); - assert_eq!(folder.layer_ids[1], 1); - assert_eq!(folder.layer_ids[2], 2); - assert_eq!(folder.layer_ids[3], 3); - - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - - assert_eq!(folder.layer_ids.len(), 4); - assert_eq!(folder.layers.len(), 4); - - folder.reorder_layers(vec![0, 2], 3).expect_err("Non-contiguous selections can't be reordered"); - - // Expect identical state - assert_eq!(folder.layer_ids[0], 0); - assert_eq!(folder.layer_ids[1], 1); - assert_eq!(folder.layer_ids[2], 2); - assert_eq!(folder.layer_ids[3], 3); - - assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_))); - assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_))); - assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_))); - assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_))); - - assert_eq!(folder.layer_ids.len(), 4); - assert_eq!(folder.layers.len(), 4); - } -} diff --git a/core/document/src/operation.rs b/core/document/src/operation.rs index a05eecf119..fc1e7f3ca3 100644 --- a/core/document/src/operation.rs +++ b/core/document/src/operation.rs @@ -51,6 +51,7 @@ pub enum Operation { PasteLayer { layer: Layer, path: Vec, + insert_index: isize, }, AddFolder { path: Vec, @@ -80,8 +81,4 @@ pub enum Operation { path: Vec, color: Color, }, - ReorderLayers { - source_paths: Vec>, - target_path: Vec, - }, } diff --git a/core/editor/src/communication/dispatcher.rs b/core/editor/src/communication/dispatcher.rs index 71f1bf7f11..fd491d1743 100644 --- a/core/editor/src/communication/dispatcher.rs +++ b/core/editor/src/communication/dispatcher.rs @@ -123,7 +123,7 @@ mod test { let document_before_copy = editor.dispatcher.document_message_handler.active_document().document.clone(); editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap(); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone(); let layers_before_copy = document_before_copy.root.as_folder().unwrap().layers(); @@ -156,7 +156,7 @@ mod test { editor.handle_message(Message::Document(DocumentMessage::SelectLayers(vec![vec![shape_id]]))).unwrap(); editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap(); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone(); @@ -220,8 +220,8 @@ mod test { editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap(); editor.handle_message(Message::Document(DocumentMessage::DeleteSelectedLayers)).unwrap(); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone(); @@ -282,8 +282,8 @@ mod test { editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap(); editor.handle_message(Message::Document(DocumentMessage::DeleteSelectedLayers)).unwrap(); editor.draw_rect(0, 800, 12, 200); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); - editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap(); let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone(); diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index 0b46bb38b4..27bcf29cd1 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -26,7 +26,7 @@ pub enum DocumentMessage { DuplicateSelectedLayers, CopySelectedLayers, SetBlendModeForSelectedLayers(BlendMode), - PasteLayers, + PasteLayers { path: Vec, insert_index: isize }, AddFolder(Vec), RenameLayer(Vec, String), ToggleLayerVisibility(Vec), @@ -56,8 +56,9 @@ pub enum DocumentMessage { WheelCanvasZoom, SetCanvasRotation(f64), NudgeSelectedLayers(f64, f64), - ReorderSelectedLayers(i32), FlipLayer(Vec, bool, bool), + MoveSelectedLayersTo { path: Vec, insert_index: isize }, + ReorderSelectedLayers(i32), // relatve_positon, } impl From for DocumentMessage { @@ -382,10 +383,17 @@ impl MessageHandler for DocumentMessageHand } } } - PasteLayers => { + PasteLayers { path, insert_index } => { for layer in self.copy_buffer.iter() { //TODO: Should be the path to the current folder instead of root - responses.push_back(DocumentOperation::PasteLayer { layer: layer.clone(), path: vec![] }.into()) + responses.push_back( + DocumentOperation::PasteLayer { + layer: layer.clone(), + path: path.clone(), + insert_index, + } + .into(), + ) } } SelectLayers(paths) => { @@ -576,33 +584,25 @@ impl MessageHandler for DocumentMessageHand responses.push_back(operation.into()); } } - ReorderSelectedLayers(delta) => { - let selected_layer_paths: Vec> = self.selected_layers_sorted(); + MoveSelectedLayersTo { path, insert_index } => { + responses.push_back(DocumentMessage::CopySelectedLayers.into()); + responses.push_back(DocumentMessage::DeleteSelectedLayers.into()); + responses.push_back(DocumentMessage::PasteLayers { path, insert_index }.into()); + } + ReorderSelectedLayers(relative_positon) => { let all_layer_paths = self.all_layers_sorted(); - - let max_index = all_layer_paths.len() as i64 - 1; - let num_layers_selected = selected_layer_paths.len() as i64; - - let mut selected_layer_index = -1; - let mut next_layer_index = -1; - for (i, path) in all_layer_paths.iter().enumerate() { - if *path == selected_layer_paths[0] { - selected_layer_index = i as i32; - // Skip past selection length when moving up - let offset = if delta > 0 { num_layers_selected - 1 } else { 0 }; - next_layer_index = (selected_layer_index as i64 + delta as i64 + offset).clamp(0, max_index) as i32; - break; + if let Some(insert_path) = all_layer_paths + .iter() + .position(|path| self.layerdata(&path).selected) + .map(|pos| all_layer_paths.get((pos as i64 + relative_positon as i64).clamp(0, all_layer_paths.len() as i64 - 1) as usize)) + .flatten() + { + let (id, path) = insert_path.split_last().expect("Can't move the root folder"); + if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { + let insert_index = folder.position_of_layer(*id).unwrap() as isize; + responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) } } - - if next_layer_index != -1 && next_layer_index != selected_layer_index { - let operation = DocumentOperation::ReorderLayers { - source_paths: selected_layer_paths.clone(), - target_path: all_layer_paths[next_layer_index as usize].to_vec(), - }; - responses.push_back(operation.into()); - responses.push_back(DocumentMessage::SelectLayers(selected_layer_paths).into()); - } } FlipLayer(path, flip_horizontal, flip_vertical) => { if let Ok(layer) = self.active_document_mut().document.layer_mut(&path) { diff --git a/core/editor/src/input/input_mapper.rs b/core/editor/src/input/input_mapper.rs index ccf6880859..78991a0212 100644 --- a/core/editor/src/input/input_mapper.rs +++ b/core/editor/src/input/input_mapper.rs @@ -111,7 +111,7 @@ macro_rules! mapping { impl Default for Mapping { fn default() -> Self { let mappings = mapping![ - entry! {action=DocumentMessage::PasteLayers, key_down=KeyV, modifiers=[KeyControl]}, + entry! {action=DocumentMessage::PasteLayers{path: vec![], insert_index: -1}, key_down=KeyV, modifiers=[KeyControl]}, entry! {action=DocumentMessage::EnableSnapping, key_down=KeyShift}, entry! {action=DocumentMessage::DisableSnapping, key_up=KeyShift}, // Select From c1197529ae7de22208fb81a2cf12d613fc8f82b2 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 20 Jul 2021 01:00:06 +0200 Subject: [PATCH 02/14] Keep selection during reordering --- core/document/src/document.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/document/src/document.rs b/core/document/src/document.rs index e45b61772e..6d68036e05 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -309,9 +309,14 @@ impl Document { Operation::PasteLayer { path, layer, insert_index } => { let folder = self.folder_mut(path)?; //FIXME: This clone of layer should be avoided somehow - folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?; + let id = folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?; + let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.clone() }]) + Some(vec![ + DocumentResponse::DocumentChanged, + DocumentResponse::SelectLayer { path: path.clone() }, + DocumentResponse::FolderChanged { path }, + ]) } Operation::DuplicateLayer { path } => { let layer = self.layer(&path)?.clone(); From 7b55e73ec24c2a7b8fd819ce8c743855e2eb4f3c Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 20 Jul 2021 22:30:01 +0200 Subject: [PATCH 03/14] Fix paste layer selection --- core/document/src/document.rs | 12 ++++++------ core/document/src/response.rs | 4 ++-- core/editor/src/document/document_message_handler.rs | 3 +-- core/editor/src/tool/tools/ellipse.rs | 1 + core/editor/src/tool/tools/line.rs | 1 + core/editor/src/tool/tools/pen.rs | 1 + core/editor/src/tool/tools/rectangle.rs | 1 + core/editor/src/tool/tools/shape.rs | 1 + 8 files changed, 14 insertions(+), 10 deletions(-) diff --git a/core/document/src/document.rs b/core/document/src/document.rs index 6d68036e05..913391e988 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -259,19 +259,19 @@ impl Document { let id = self.add_layer(&path, Layer::new(LayerDataTypes::Ellipse(layers::Ellipse::new()), *transform, *style), *insert_index)?; let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::SelectLayer { path }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::CreatedLayer { path }]) } Operation::AddRect { path, insert_index, transform, style } => { let id = self.add_layer(&path, Layer::new(LayerDataTypes::Rect(Rect), *transform, *style), *insert_index)?; let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::SelectLayer { path }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::CreatedLayer { path }]) } Operation::AddLine { path, insert_index, transform, style } => { let id = self.add_layer(&path, Layer::new(LayerDataTypes::Line(Line), *transform, *style), *insert_index)?; let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::SelectLayer { path }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::CreatedLayer { path }]) } Operation::AddPen { path, @@ -284,7 +284,7 @@ impl Document { let polyline = PolyLine::new(points); let id = self.add_layer(&path, Layer::new(LayerDataTypes::PolyLine(polyline), *transform, *style), *insert_index)?; let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::SelectLayer { path }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::CreatedLayer { path }]) } Operation::AddShape { path, @@ -298,7 +298,7 @@ impl Document { let id = self.add_layer(&path, Layer::new(LayerDataTypes::Shape(s), *transform, *style), *insert_index)?; let path = [path.clone(), vec![id]].concat(); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::SelectLayer { path }]) + Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::CreatedLayer { path }]) } Operation::DeleteLayer { path } => { self.delete(&path)?; @@ -314,7 +314,7 @@ impl Document { Some(vec![ DocumentResponse::DocumentChanged, - DocumentResponse::SelectLayer { path: path.clone() }, + DocumentResponse::CreatedLayer { path: path.clone() }, DocumentResponse::FolderChanged { path }, ]) } diff --git a/core/document/src/response.rs b/core/document/src/response.rs index c4e74bc96b..c822b349b9 100644 --- a/core/document/src/response.rs +++ b/core/document/src/response.rs @@ -7,7 +7,7 @@ use std::fmt; pub enum DocumentResponse { DocumentChanged, FolderChanged { path: Vec }, - SelectLayer { path: Vec }, + CreatedLayer { path: Vec }, } impl fmt::Display for DocumentResponse { @@ -15,7 +15,7 @@ impl fmt::Display for DocumentResponse { let name = match self { DocumentResponse::DocumentChanged { .. } => "DocumentChanged", DocumentResponse::FolderChanged { .. } => "FolderChanged", - DocumentResponse::SelectLayer { .. } => "SelectLayer", + DocumentResponse::CreatedLayer { .. } => "SelectLayer", }; formatter.write_str(name) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index 27bcf29cd1..a4c18c30cc 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -429,9 +429,8 @@ impl MessageHandler for DocumentMessageHand .into_iter() .map(|response| match response { DocumentResponse::FolderChanged { path } => self.handle_folder_changed(path), - DocumentResponse::SelectLayer { path } => { + DocumentResponse::CreatedLayer { path } => { if !self.active_document().document.work_mounted { - self.clear_selection(); self.select_layer(&path) } else { None diff --git a/core/editor/src/tool/tools/ellipse.rs b/core/editor/src/tool/tools/ellipse.rs index 5180c84766..b5905facb3 100644 --- a/core/editor/src/tool/tools/ellipse.rs +++ b/core/editor/src/tool/tools/ellipse.rs @@ -86,6 +86,7 @@ impl Fsm for EllipseToolFsmState { // TODO - introduce comparison threshold when operating with canvas coordinates (https://github.com/GraphiteEditor/Graphite/issues/100) if data.drag_start != data.drag_current { responses.push_back(make_operation(data, tool_data, transform)); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(Operation::CommitTransaction.into()); } diff --git a/core/editor/src/tool/tools/line.rs b/core/editor/src/tool/tools/line.rs index 578536f181..3f21b5cf51 100644 --- a/core/editor/src/tool/tools/line.rs +++ b/core/editor/src/tool/tools/line.rs @@ -143,6 +143,7 @@ fn update_state( *(state(data)) = value; responses.push_back(Operation::ClearWorkingFolder.into()); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(make_operation(data, tool_data, transform)); new_state diff --git a/core/editor/src/tool/tools/pen.rs b/core/editor/src/tool/tools/pen.rs index d21a90bfcb..b6d8889dca 100644 --- a/core/editor/src/tool/tools/pen.rs +++ b/core/editor/src/tool/tools/pen.rs @@ -96,6 +96,7 @@ impl Fsm for PenToolFsmState { if data.points.len() >= 2 { responses.push_back(make_operation(data, tool_data, false)); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(Operation::CommitTransaction.into()); } else { responses.push_back(Operation::DiscardWorkingFolder.into()); diff --git a/core/editor/src/tool/tools/rectangle.rs b/core/editor/src/tool/tools/rectangle.rs index 0b4c9045ca..05a33277bc 100644 --- a/core/editor/src/tool/tools/rectangle.rs +++ b/core/editor/src/tool/tools/rectangle.rs @@ -85,6 +85,7 @@ impl Fsm for RectangleToolFsmState { // TODO - introduce comparison threshold when operating with canvas coordinates (https://github.com/GraphiteEditor/Graphite/issues/100) if data.drag_start != data.drag_current { responses.push_back(make_operation(data, tool_data, transform)); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(Operation::CommitTransaction.into()); } diff --git a/core/editor/src/tool/tools/shape.rs b/core/editor/src/tool/tools/shape.rs index 5d4fdc3ee2..26906531d0 100644 --- a/core/editor/src/tool/tools/shape.rs +++ b/core/editor/src/tool/tools/shape.rs @@ -93,6 +93,7 @@ impl Fsm for ShapeToolFsmState { // TODO - introduce comparison threshold when operating with canvas coordinates (https://github.com/GraphiteEditor/Graphite/issues/100) if data.drag_start != data.drag_current { responses.push_back(make_operation(data, tool_data, transform)); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(Operation::CommitTransaction.into()); } From 6c3f2a80d2f6ae9a42f3557ebb1f0c479299c4d6 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Tue, 20 Jul 2021 23:30:10 +0200 Subject: [PATCH 04/14] Remove junk from layer matadata --- core/document/src/document.rs | 39 ++++++++++++++----- core/document/src/response.rs | 2 + .../src/document/document_message_handler.rs | 7 +++- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/core/document/src/document.rs b/core/document/src/document.rs index 913391e988..337b114901 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -251,6 +251,17 @@ impl Document { Ok(()) } + fn working_paths(&mut self) -> Vec> { + log::debug!("deleting: {:?}", self.work.as_folder().unwrap().layer_ids); + self.work + .as_folder() + .unwrap() + .layer_ids + .iter() + .map(|id| self.work_mount_path.iter().chain([*id].iter()).cloned().collect()) + .collect() + } + /// Mutate the document by applying the `operation` to it. If the operation necessitates a /// reaction from the frontend, responses may be returned. pub fn handle_operation(&mut self, operation: Operation) -> Result>, DocumentError> { @@ -303,19 +314,23 @@ impl Document { Operation::DeleteLayer { path } => { self.delete(&path)?; - let (path, _) = split_path(path.as_slice()).unwrap_or_else(|_| (&[], 0)); - Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.to_vec() }]) + let (folder, _) = split_path(path.as_slice()).unwrap_or_else(|_| (&[], 0)); + Some(vec![ + DocumentResponse::DocumentChanged, + DocumentResponse::DeletedLayer { path: path.clone() }, + DocumentResponse::FolderChanged { path: folder.to_vec() }, + ]) } Operation::PasteLayer { path, layer, insert_index } => { let folder = self.folder_mut(path)?; //FIXME: This clone of layer should be avoided somehow let id = folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?; - let path = [path.clone(), vec![id]].concat(); + let full_path = [path.clone(), vec![id]].concat(); Some(vec![ DocumentResponse::DocumentChanged, - DocumentResponse::CreatedLayer { path: path.clone() }, - DocumentResponse::FolderChanged { path }, + DocumentResponse::CreatedLayer { path: full_path }, + DocumentResponse::FolderChanged { path: path.clone() }, ]) } Operation::DuplicateLayer { path } => { @@ -331,11 +346,13 @@ impl Document { Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.clone() }]) } Operation::MountWorkingFolder { path } => { + let mut responses: Vec<_> = self.working_paths().into_iter().map(|path| DocumentResponse::DeletedLayer { path }).collect(); self.work_mount_path = path.clone(); self.work_operations.clear(); self.work = Layer::new(LayerDataTypes::Folder(Folder::default()), DAffine2::IDENTITY.to_cols_array(), PathStyle::default()); self.work_mounted = true; - None + responses.push(DocumentResponse::DocumentChanged); + Some(responses) } Operation::TransformLayer { path, transform } => { let layer = self.document_layer_mut(path).unwrap(); @@ -353,18 +370,23 @@ impl Document { Some(vec![DocumentResponse::DocumentChanged]) } Operation::DiscardWorkingFolder => { + let mut responses: Vec<_> = self.working_paths().into_iter().map(|path| DocumentResponse::DeletedLayer { path }).collect(); self.work_operations.clear(); self.work_mount_path = vec![]; self.work = Layer::new(LayerDataTypes::Folder(Folder::default()), DAffine2::IDENTITY.to_cols_array(), PathStyle::default()); self.work_mounted = false; - Some(vec![DocumentResponse::DocumentChanged]) + responses.push(DocumentResponse::DocumentChanged); + Some(responses) } Operation::ClearWorkingFolder => { + let mut responses: Vec<_> = self.working_paths().into_iter().map(|path| DocumentResponse::DeletedLayer { path }).collect(); self.work_operations.clear(); self.work = Layer::new(LayerDataTypes::Folder(Folder::default()), DAffine2::IDENTITY.to_cols_array(), PathStyle::default()); - Some(vec![DocumentResponse::DocumentChanged]) + responses.push(DocumentResponse::DocumentChanged); + Some(responses) } Operation::CommitTransaction => { + let mut responses: Vec<_> = self.working_paths().into_iter().map(|path| DocumentResponse::DeletedLayer { path }).collect(); let mut ops = Vec::new(); let mut path: Vec = vec![]; std::mem::swap(&mut path, &mut self.work_mount_path); @@ -372,7 +394,6 @@ impl Document { self.work_mounted = false; self.work_mount_path = vec![]; self.work = Layer::new(LayerDataTypes::Folder(Folder::default()), DAffine2::IDENTITY.to_cols_array(), PathStyle::default()); - let mut responses = vec![]; for operation in ops.into_iter() { if let Some(mut op_responses) = self.handle_operation(operation)? { responses.append(&mut op_responses); diff --git a/core/document/src/response.rs b/core/document/src/response.rs index c822b349b9..419085b9ea 100644 --- a/core/document/src/response.rs +++ b/core/document/src/response.rs @@ -8,6 +8,7 @@ pub enum DocumentResponse { DocumentChanged, FolderChanged { path: Vec }, CreatedLayer { path: Vec }, + DeletedLayer { path: Vec }, } impl fmt::Display for DocumentResponse { @@ -16,6 +17,7 @@ impl fmt::Display for DocumentResponse { DocumentResponse::DocumentChanged { .. } => "DocumentChanged", DocumentResponse::FolderChanged { .. } => "FolderChanged", DocumentResponse::CreatedLayer { .. } => "SelectLayer", + DocumentResponse::DeletedLayer { .. } => "DeleteLayer", }; formatter.write_str(name) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index a4c18c30cc..3dcb182dd8 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -362,7 +362,6 @@ impl MessageHandler for DocumentMessageHand DeleteSelectedLayers => { let paths = self.selected_layers_sorted(); for path in paths { - self.active_document_mut().layer_data.remove(&path); responses.push_back(DocumentOperation::DeleteLayer { path }.into()) } } @@ -385,7 +384,7 @@ impl MessageHandler for DocumentMessageHand } PasteLayers { path, insert_index } => { for layer in self.copy_buffer.iter() { - //TODO: Should be the path to the current folder instead of root + log::trace!("pasting into folder {:?} as index: {}", path, insert_index); responses.push_back( DocumentOperation::PasteLayer { layer: layer.clone(), @@ -429,6 +428,10 @@ impl MessageHandler for DocumentMessageHand .into_iter() .map(|response| match response { DocumentResponse::FolderChanged { path } => self.handle_folder_changed(path), + DocumentResponse::DeletedLayer { path } => { + self.active_document_mut().layer_data.remove(&path); + None + } DocumentResponse::CreatedLayer { path } => { if !self.active_document().document.work_mounted { self.select_layer(&path) From c286ea2aab6975fbecd6cd89444772f698d8b40d Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 21 Jul 2021 00:16:50 +0200 Subject: [PATCH 05/14] Add function to get non_selected_layers --- .../src/document/document_message_handler.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index 3dcb182dd8..0807e9fa71 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -142,7 +142,7 @@ impl DocumentMessageHandler { } /// Returns the paths to all layers in order, optionally including only selected layers - fn layers_sorted(&self, only_selected: bool) -> Vec> { + fn layers_sorted(&self, selected: Option) -> Vec> { // Compute the indices for each layer to be able to sort them // TODO: Replace with drain_filter https://github.com/rust-lang/rust/issues/59618 let mut layers_with_indices: Vec<(Vec, Vec)> = self @@ -150,7 +150,7 @@ impl DocumentMessageHandler { .layer_data .iter() // 'path.len() > 0' filters out root layer since it has no indices - .filter_map(|(path, data)| (!path.is_empty() && !only_selected || data.selected).then(|| path.clone())) + .filter_map(|(path, data)| (!path.is_empty() && selected.is_none() || (data.selected == selected.unwrap_or(data.selected))).then(|| path.clone())) .filter_map(|path| { // Currently it is possible that layer_data contains layers that are don't actually exist // and thus indices_for_path can return an error. We currently skip these layers and log a warning. @@ -171,12 +171,17 @@ impl DocumentMessageHandler { /// Returns the paths to all layers in order fn all_layers_sorted(&self) -> Vec> { - self.layers_sorted(false) + self.layers_sorted(None) } /// Returns the paths to all selected layers in order fn selected_layers_sorted(&self) -> Vec> { - self.layers_sorted(true) + self.layers_sorted(Some(true)) + } + + /// Returns the paths to all selected layers in order + fn non_selected_layers_sorted(&self) -> Vec> { + self.layers_sorted(Some(false)) } } @@ -587,18 +592,21 @@ impl MessageHandler for DocumentMessageHand } } MoveSelectedLayersTo { path, insert_index } => { + log::debug!("moving layers to {:?} at pos {}", path, insert_index); responses.push_back(DocumentMessage::CopySelectedLayers.into()); responses.push_back(DocumentMessage::DeleteSelectedLayers.into()); responses.push_back(DocumentMessage::PasteLayers { path, insert_index }.into()); } ReorderSelectedLayers(relative_positon) => { let all_layer_paths = self.all_layers_sorted(); + let non_selected_layers = self.non_selected_layers_sorted(); if let Some(insert_path) = all_layer_paths .iter() .position(|path| self.layerdata(&path).selected) - .map(|pos| all_layer_paths.get((pos as i64 + relative_positon as i64).clamp(0, all_layer_paths.len() as i64 - 1) as usize)) + .map(|pos| non_selected_layers.get((pos as i64 + relative_positon as i64).clamp(0, all_layer_paths.len() as i64 - 1) as usize)) .flatten() { + log::debug!("inserting at {:?}", insert_path); let (id, path) = insert_path.split_last().expect("Can't move the root folder"); if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { let insert_index = folder.position_of_layer(*id).unwrap() as isize; From 612d4335f65e2abec539c642c08b8d2c67bc608d Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 21 Jul 2021 09:47:04 +0200 Subject: [PATCH 06/14] Cleanup --- core/editor/src/document/document_message_handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index 0807e9fa71..5c219fe5f2 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -150,7 +150,7 @@ impl DocumentMessageHandler { .layer_data .iter() // 'path.len() > 0' filters out root layer since it has no indices - .filter_map(|(path, data)| (!path.is_empty() && selected.is_none() || (data.selected == selected.unwrap_or(data.selected))).then(|| path.clone())) + .filter_map(|(path, data)| (!path.is_empty() && (data.selected == selected.unwrap_or(data.selected))).then(|| path.clone())) .filter_map(|path| { // Currently it is possible that layer_data contains layers that are don't actually exist // and thus indices_for_path can return an error. We currently skip these layers and log a warning. From fd3504847bffd8e77e603e61abbd95d53a2646ac Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 22 Jul 2021 19:12:05 +0200 Subject: [PATCH 07/14] Fix selection ordering --- .../src/document/document_message_handler.rs | 49 ++++++++++++++----- core/editor/src/tool/tools/line.rs | 2 +- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index 5c219fe5f2..edaae05348 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -378,7 +378,7 @@ impl MessageHandler for DocumentMessageHand CopySelectedLayers => { let paths = self.selected_layers_sorted(); self.copy_buffer.clear(); - for path in paths { + for path in paths.iter().rev() { match self.active_document().document.layer(&path).map(|t| t.clone()) { Ok(layer) => { self.copy_buffer.push(layer); @@ -599,18 +599,41 @@ impl MessageHandler for DocumentMessageHand } ReorderSelectedLayers(relative_positon) => { let all_layer_paths = self.all_layers_sorted(); - let non_selected_layers = self.non_selected_layers_sorted(); - if let Some(insert_path) = all_layer_paths - .iter() - .position(|path| self.layerdata(&path).selected) - .map(|pos| non_selected_layers.get((pos as i64 + relative_positon as i64).clamp(0, all_layer_paths.len() as i64 - 1) as usize)) - .flatten() - { - log::debug!("inserting at {:?}", insert_path); - let (id, path) = insert_path.split_last().expect("Can't move the root folder"); - if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { - let insert_index = folder.position_of_layer(*id).unwrap() as isize; - responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + let selected_layers = self.selected_layers_sorted(); + let pivot = match relative_positon.signum() { + -1 => selected_layers.first(), + 1 => selected_layers.last(), + _ => unreachable!(), + } + .expect("called move layers with an empty selection"); + if let Some(pos) = all_layer_paths.iter().position(|path| path == pivot) { + log::debug!("pos {:?}", pos); + let max = all_layer_paths.len() as i64 - selected_layers.len() as i64; + log::debug!("max {:?}", max); + log::debug!("relative_positon {:?}", relative_positon); + log::debug!("all_layer_paths: {:?}", all_layer_paths); + let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; + log::debug!("inser_pos {:?}", insert_pos); + let insert = all_layer_paths.get(insert_pos); + if let Some(insert_path) = insert { + log::debug!("inserting at {:?}", insert_path); + let (id, path) = insert_path.split_last().expect("Can't move the root folder"); + if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { + // TODO: Ignore existing thingsin folder + // + let selected: Vec<_> = selected_layers + .iter() + .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) + .map(|x| x.last().unwrap()) + .collect(); + log::debug!("selected {:?}", selected); + let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); + log::debug!("non selected {:?}", non_selected); + let offset = if relative_positon < 0 { 0 } else { 1 } as usize; + let fallback = offset * (non_selected.len() - 1); + let insert_index = non_selected.iter().position(|x| *x == id).unwrap_or(fallback) as isize + offset as isize; + responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + } } } } diff --git a/core/editor/src/tool/tools/line.rs b/core/editor/src/tool/tools/line.rs index 3f21b5cf51..6504c7b41e 100644 --- a/core/editor/src/tool/tools/line.rs +++ b/core/editor/src/tool/tools/line.rs @@ -93,6 +93,7 @@ impl Fsm for LineToolFsmState { // TODO - introduce comparison threshold when operating with canvas coordinates (https://github.com/GraphiteEditor/Graphite/issues/100) if data.drag_start != data.drag_current { responses.push_back(make_operation(data, tool_data, transform)); + responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(Operation::CommitTransaction.into()); } @@ -143,7 +144,6 @@ fn update_state( *(state(data)) = value; responses.push_back(Operation::ClearWorkingFolder.into()); - responses.push_back(DocumentMessage::DeselectAllLayers.into()); responses.push_back(make_operation(data, tool_data, transform)); new_state From 637e7a665d1fb1407fb6cc3c929ee0d3bc415942 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 08:29:05 +0200 Subject: [PATCH 08/14] Add debuging output --- core/editor/src/document/document_message_handler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index edaae05348..c9d86adbbc 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -378,7 +378,7 @@ impl MessageHandler for DocumentMessageHand CopySelectedLayers => { let paths = self.selected_layers_sorted(); self.copy_buffer.clear(); - for path in paths.iter().rev() { + for path in paths { match self.active_document().document.layer(&path).map(|t| t.clone()) { Ok(layer) => { self.copy_buffer.push(layer); @@ -386,6 +386,7 @@ impl MessageHandler for DocumentMessageHand Err(e) => warn!("Could not access selected layer {:?}: {:?}", path, e), } } + log::debug!("copy_buffer: {:?}", self.copy_buffer); } PasteLayers { path, insert_index } => { for layer in self.copy_buffer.iter() { From 4ba1954865781a421c47937ef76a94445d4d35ac Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 09:22:58 +0200 Subject: [PATCH 09/14] Fix more selection issues --- .../src/document/document_message_handler.rs | 71 +++++++++++-------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index c9d86adbbc..fda28943be 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -389,7 +389,7 @@ impl MessageHandler for DocumentMessageHand log::debug!("copy_buffer: {:?}", self.copy_buffer); } PasteLayers { path, insert_index } => { - for layer in self.copy_buffer.iter() { + let paste = |layer: &Layer, responses: &mut VecDeque<_>| { log::trace!("pasting into folder {:?} as index: {}", path, insert_index); responses.push_back( DocumentOperation::PasteLayer { @@ -399,6 +399,15 @@ impl MessageHandler for DocumentMessageHand } .into(), ) + }; + if insert_index == -1 { + for layer in self.copy_buffer.iter() { + paste(layer, responses) + } + } else { + for layer in self.copy_buffer.iter().rev() { + paste(layer, responses) + } } } SelectLayers(paths) => { @@ -601,39 +610,39 @@ impl MessageHandler for DocumentMessageHand ReorderSelectedLayers(relative_positon) => { let all_layer_paths = self.all_layers_sorted(); let selected_layers = self.selected_layers_sorted(); - let pivot = match relative_positon.signum() { + if let Some(pivot) = match relative_positon.signum() { -1 => selected_layers.first(), 1 => selected_layers.last(), _ => unreachable!(), - } - .expect("called move layers with an empty selection"); - if let Some(pos) = all_layer_paths.iter().position(|path| path == pivot) { - log::debug!("pos {:?}", pos); - let max = all_layer_paths.len() as i64 - selected_layers.len() as i64; - log::debug!("max {:?}", max); - log::debug!("relative_positon {:?}", relative_positon); - log::debug!("all_layer_paths: {:?}", all_layer_paths); - let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; - log::debug!("inser_pos {:?}", insert_pos); - let insert = all_layer_paths.get(insert_pos); - if let Some(insert_path) = insert { - log::debug!("inserting at {:?}", insert_path); - let (id, path) = insert_path.split_last().expect("Can't move the root folder"); - if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { - // TODO: Ignore existing thingsin folder - // - let selected: Vec<_> = selected_layers - .iter() - .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) - .map(|x| x.last().unwrap()) - .collect(); - log::debug!("selected {:?}", selected); - let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); - log::debug!("non selected {:?}", non_selected); - let offset = if relative_positon < 0 { 0 } else { 1 } as usize; - let fallback = offset * (non_selected.len() - 1); - let insert_index = non_selected.iter().position(|x| *x == id).unwrap_or(fallback) as isize + offset as isize; - responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + } { + if let Some(pos) = all_layer_paths.iter().position(|path| path == pivot) { + log::debug!("pos {:?}", pos); + let max = all_layer_paths.len() as i64 - selected_layers.len() as i64; + log::debug!("max {:?}", max); + log::debug!("relative_positon {:?}", relative_positon); + log::debug!("all_layer_paths: {:?}", all_layer_paths); + let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; + log::debug!("inser_pos {:?}", insert_pos); + let insert = all_layer_paths.get(insert_pos); + if let Some(insert_path) = insert { + log::debug!("inserting at {:?}", insert_path); + let (id, path) = insert_path.split_last().expect("Can't move the root folder"); + if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { + // TODO: Ignore existing thingsin folder + // + let selected: Vec<_> = selected_layers + .iter() + .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) + .map(|x| x.last().unwrap()) + .collect(); + log::debug!("selected {:?}", selected); + let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); + log::debug!("non selected {:?}", non_selected); + let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { selected.len() }; + let fallback = offset * (non_selected.len().clamp(1, usize::MAX) - 1); + let insert_index = non_selected.iter().position(|x| *x == id).unwrap_or(fallback) as isize + offset as isize; + responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + } } } } From 08d638c3858d0f15494b72b7177806f726669511 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 09:36:24 +0200 Subject: [PATCH 10/14] Another fix --- core/editor/src/document/document_message_handler.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index fda28943be..a0ae04e403 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -638,7 +638,11 @@ impl MessageHandler for DocumentMessageHand log::debug!("selected {:?}", selected); let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); log::debug!("non selected {:?}", non_selected); - let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { selected.len() }; + let offset = if relative_positon < 0 || non_selected.is_empty() { + 0 + } else { + (relative_positon as usize).clamp(0, selected.len()) + }; let fallback = offset * (non_selected.len().clamp(1, usize::MAX) - 1); let insert_index = non_selected.iter().position(|x| *x == id).unwrap_or(fallback) as isize + offset as isize; responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) From 3de804201dc29dcbf5a5841de2865c165e5252d7 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 12:52:41 +0200 Subject: [PATCH 11/14] Change max calculation --- .../src/document/document_message_handler.rs | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index a0ae04e403..e287bd6b18 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -617,35 +617,38 @@ impl MessageHandler for DocumentMessageHand } { if let Some(pos) = all_layer_paths.iter().position(|path| path == pivot) { log::debug!("pos {:?}", pos); - let max = all_layer_paths.len() as i64 - selected_layers.len() as i64; + let max = all_layer_paths.len() as i64 - 1; log::debug!("max {:?}", max); log::debug!("relative_positon {:?}", relative_positon); log::debug!("all_layer_paths: {:?}", all_layer_paths); let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; log::debug!("inser_pos {:?}", insert_pos); let insert = all_layer_paths.get(insert_pos); - if let Some(insert_path) = insert { - log::debug!("inserting at {:?}", insert_path); - let (id, path) = insert_path.split_last().expect("Can't move the root folder"); - if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { - // TODO: Ignore existing thingsin folder - // - let selected: Vec<_> = selected_layers - .iter() - .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) - .map(|x| x.last().unwrap()) - .collect(); - log::debug!("selected {:?}", selected); - let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); - log::debug!("non selected {:?}", non_selected); - let offset = if relative_positon < 0 || non_selected.is_empty() { - 0 - } else { - (relative_positon as usize).clamp(0, selected.len()) - }; - let fallback = offset * (non_selected.len().clamp(1, usize::MAX) - 1); - let insert_index = non_selected.iter().position(|x| *x == id).unwrap_or(fallback) as isize + offset as isize; - responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + let position_change = (insert_pos) as i32 - pos as i32; + log::debug!("pos_change {:?}", position_change); + if position_change != 0 { + if let Some(insert_path) = insert { + log::debug!("inserting at {:?}", insert_path); + let (id, path) = insert_path.split_last().expect("Can't move the root folder"); + if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { + // TODO: Ignore existing thingsin folder + // + let selected: Vec<_> = selected_layers + .iter() + .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) + .map(|x| x.last().unwrap()) + .collect(); + log::debug!("selected {:?}", selected); + let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); + log::debug!("non selected {:?}", non_selected); + let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { 1 }; + log::debug!("offset {:?}", offset); + let fallback = offset * (non_selected.len()); + log::debug!("fallback {:?}", fallback); + let insert_index = non_selected.iter().position(|x| *x == id).map(|x| x + offset).unwrap_or(fallback) as isize; + //let insert_index = insert_index.clamp(0, non_selected.len() as isize - (non_selected.len() as isize).signum()); + responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) + } } } } From c812365a41bdb6a4f8a63adb502933c1318b013f Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 13:01:22 +0200 Subject: [PATCH 12/14] Remove debug statements --- .../src/document/document_message_handler.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index e287bd6b18..a7daf8db54 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -386,7 +386,6 @@ impl MessageHandler for DocumentMessageHand Err(e) => warn!("Could not access selected layer {:?}: {:?}", path, e), } } - log::debug!("copy_buffer: {:?}", self.copy_buffer); } PasteLayers { path, insert_index } => { let paste = |layer: &Layer, responses: &mut VecDeque<_>| { @@ -602,7 +601,6 @@ impl MessageHandler for DocumentMessageHand } } MoveSelectedLayersTo { path, insert_index } => { - log::debug!("moving layers to {:?} at pos {}", path, insert_index); responses.push_back(DocumentMessage::CopySelectedLayers.into()); responses.push_back(DocumentMessage::DeleteSelectedLayers.into()); responses.push_back(DocumentMessage::PasteLayers { path, insert_index }.into()); @@ -616,37 +614,23 @@ impl MessageHandler for DocumentMessageHand _ => unreachable!(), } { if let Some(pos) = all_layer_paths.iter().position(|path| path == pivot) { - log::debug!("pos {:?}", pos); let max = all_layer_paths.len() as i64 - 1; - log::debug!("max {:?}", max); - log::debug!("relative_positon {:?}", relative_positon); - log::debug!("all_layer_paths: {:?}", all_layer_paths); let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; - log::debug!("inser_pos {:?}", insert_pos); let insert = all_layer_paths.get(insert_pos); let position_change = (insert_pos) as i32 - pos as i32; - log::debug!("pos_change {:?}", position_change); if position_change != 0 { if let Some(insert_path) = insert { - log::debug!("inserting at {:?}", insert_path); let (id, path) = insert_path.split_last().expect("Can't move the root folder"); if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { - // TODO: Ignore existing thingsin folder - // let selected: Vec<_> = selected_layers .iter() .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) .map(|x| x.last().unwrap()) .collect(); - log::debug!("selected {:?}", selected); let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); - log::debug!("non selected {:?}", non_selected); let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { 1 }; - log::debug!("offset {:?}", offset); let fallback = offset * (non_selected.len()); - log::debug!("fallback {:?}", fallback); let insert_index = non_selected.iter().position(|x| *x == id).map(|x| x + offset).unwrap_or(fallback) as isize; - //let insert_index = insert_index.clamp(0, non_selected.len() as isize - (non_selected.len() as isize).signum()); responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) } } From be4b93a4ac70626928ac99bc88f8cda82bf8eaaf Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 13:48:08 +0200 Subject: [PATCH 13/14] Add test case (non functional) --- core/editor/src/communication/dispatcher.rs | 25 +++++++++++++++++++ .../src/document/document_message_handler.rs | 6 ++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/core/editor/src/communication/dispatcher.rs b/core/editor/src/communication/dispatcher.rs index fd491d1743..f3398b9e6c 100644 --- a/core/editor/src/communication/dispatcher.rs +++ b/core/editor/src/communication/dispatcher.rs @@ -82,6 +82,7 @@ impl Dispatcher { #[cfg(test)] mod test { use crate::{ + communication::DocumentMessageHandler, message_prelude::{DocumentMessage, Message}, misc::test_utils::EditorTestUtils, Editor, @@ -302,4 +303,28 @@ mod test { assert_eq!(&layers_after_copy[4], rect_before_copy); assert_eq!(&layers_after_copy[5], ellipse_before_copy); } + #[test] + /// - create rect, shape and ellipse + /// - select ellipse and rect + /// - move them down and back up again + fn move_seletion() { + init_logger(); + let mut editor = create_editor_with_three_layers(); + + let verify_order = |handler: &mut DocumentMessageHandler| (handler.all_layers_sorted(), handler.non_selected_layers_sorted(), handler.selected_layers_sorted()); + + editor.handle_message(Message::Document(DocumentMessage::SelectLayers(vec![vec![0], vec![2]]))).unwrap(); + + editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(1))).unwrap(); + let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); + assert_eq!(all, non_selected.into_iter().chain(selected.into_iter()).collect::>()); + + editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(1))).unwrap(); + let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); + assert_eq!(all, selected.into_iter().chain(non_selected.into_iter()).collect::>()); + + editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(i32::MIN))).unwrap(); + let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); + assert_eq!(all, non_selected.into_iter().chain(selected.into_iter()).collect::>()); + } } diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index a7daf8db54..e1b609e3c6 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -170,17 +170,17 @@ impl DocumentMessageHandler { } /// Returns the paths to all layers in order - fn all_layers_sorted(&self) -> Vec> { + pub fn all_layers_sorted(&self) -> Vec> { self.layers_sorted(None) } /// Returns the paths to all selected layers in order - fn selected_layers_sorted(&self) -> Vec> { + pub fn selected_layers_sorted(&self) -> Vec> { self.layers_sorted(Some(true)) } /// Returns the paths to all selected layers in order - fn non_selected_layers_sorted(&self) -> Vec> { + pub fn non_selected_layers_sorted(&self) -> Vec> { self.layers_sorted(Some(false)) } } From 6a63fbf39b47d106fa39ba0a507e74c29074ac9d Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Fri, 23 Jul 2021 14:08:33 +0200 Subject: [PATCH 14/14] Add tests --- core/document/src/document.rs | 1 - core/editor/src/communication/dispatcher.rs | 4 +-- .../src/document/document_message_handler.rs | 29 +++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/core/document/src/document.rs b/core/document/src/document.rs index 337b114901..0f8847496c 100644 --- a/core/document/src/document.rs +++ b/core/document/src/document.rs @@ -252,7 +252,6 @@ impl Document { } fn working_paths(&mut self) -> Vec> { - log::debug!("deleting: {:?}", self.work.as_folder().unwrap().layer_ids); self.work .as_folder() .unwrap() diff --git a/core/editor/src/communication/dispatcher.rs b/core/editor/src/communication/dispatcher.rs index f3398b9e6c..fe521b8f4d 100644 --- a/core/editor/src/communication/dispatcher.rs +++ b/core/editor/src/communication/dispatcher.rs @@ -319,11 +319,11 @@ mod test { let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); assert_eq!(all, non_selected.into_iter().chain(selected.into_iter()).collect::>()); - editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(1))).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(-1))).unwrap(); let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); assert_eq!(all, selected.into_iter().chain(non_selected.into_iter()).collect::>()); - editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(i32::MIN))).unwrap(); + editor.handle_message(Message::Document(DocumentMessage::ReorderSelectedLayers(i32::MAX))).unwrap(); let (all, non_selected, selected) = verify_order(&mut editor.dispatcher.document_message_handler); assert_eq!(all, non_selected.into_iter().chain(selected.into_iter()).collect::>()); } diff --git a/core/editor/src/document/document_message_handler.rs b/core/editor/src/document/document_message_handler.rs index e1b609e3c6..1b2088d59e 100644 --- a/core/editor/src/document/document_message_handler.rs +++ b/core/editor/src/document/document_message_handler.rs @@ -617,22 +617,19 @@ impl MessageHandler for DocumentMessageHand let max = all_layer_paths.len() as i64 - 1; let insert_pos = (pos as i64 + relative_positon as i64).clamp(0, max) as usize; let insert = all_layer_paths.get(insert_pos); - let position_change = (insert_pos) as i32 - pos as i32; - if position_change != 0 { - if let Some(insert_path) = insert { - let (id, path) = insert_path.split_last().expect("Can't move the root folder"); - if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { - let selected: Vec<_> = selected_layers - .iter() - .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) - .map(|x| x.last().unwrap()) - .collect(); - let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); - let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { 1 }; - let fallback = offset * (non_selected.len()); - let insert_index = non_selected.iter().position(|x| *x == id).map(|x| x + offset).unwrap_or(fallback) as isize; - responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) - } + if let Some(insert_path) = insert { + let (id, path) = insert_path.split_last().expect("Can't move the root folder"); + if let Some(folder) = self.active_document().document.document_layer(path).ok().map(|layer| layer.as_folder().ok()).flatten() { + let selected: Vec<_> = selected_layers + .iter() + .filter(|layer| layer.starts_with(path) && layer.len() == path.len() + 1) + .map(|x| x.last().unwrap()) + .collect(); + let non_selected: Vec<_> = folder.layer_ids.iter().filter(|id| selected.iter().all(|x| x != id)).collect(); + let offset = if relative_positon < 0 || non_selected.is_empty() { 0 } else { 1 }; + let fallback = offset * (non_selected.len()); + let insert_index = non_selected.iter().position(|x| *x == id).map(|x| x + offset).unwrap_or(fallback) as isize; + responses.push_back(DocumentMessage::MoveSelectedLayersTo { path: path.to_vec(), insert_index }.into()) } } }