From bb1e7c44cfef8291bc8021c7164ca900673c2e9d Mon Sep 17 00:00:00 2001 From: Christopher Mendoza <57922269+ChrisMend19@users.noreply.github.com> Date: Fri, 19 May 2023 00:41:16 -0700 Subject: [PATCH] Make duplicating folders also duplicate its children (#1178) * Latest changes * Add layer attempt * layer tree is now correct after duplicating layers * Latest changes * Latest Changes * Recursive idea * Moving Layers to Dup Folder - not done * latest changes * latest progress * Latest Changes * Latest Changes * Latest Changes * Latest * Latest * Latest * Duplicating Folders works * Initial Refactoring * Ready for QA * Doesn't select all the children after duplicate anymore * First pass code review with major cleanup * Removed unused next_asssignment_id function and updated FolderLayer struct * Removed unused logic * First iteration of cleaning up the code * Added Ollie's suggestions * Code review cleanup --------- Co-authored-by: Ollie Dolan Co-authored-by: Keavon Chambers --- document-legacy/src/document.rs | 225 ++++++++++++++++-- document-legacy/src/layers/folder_layer.rs | 6 + document-legacy/src/operation.rs | 2 + document-legacy/src/response.rs | 11 + .../document/document_message_handler.rs | 34 ++- .../portfolio/portfolio_message_handler.rs | 2 + .../tool/tool_messages/select_tool.rs | 3 +- 7 files changed, 250 insertions(+), 33 deletions(-) diff --git a/document-legacy/src/document.rs b/document-legacy/src/document.rs index c68ee19f..9dcd5351 100644 --- a/document-legacy/src/document.rs +++ b/document-legacy/src/document.rs @@ -10,7 +10,9 @@ use glam::{DAffine2, DVec2}; use serde::{Deserialize, Serialize}; use std::cmp::max; use std::collections::hash_map::DefaultHasher; +use std::collections::HashMap; use std::hash::{Hash, Hasher}; +use std::vec; /// A number that identifies a layer. /// This does not technically need to be unique globally, only within a folder. @@ -477,21 +479,48 @@ impl Document { self.set_layer(&path, layer, insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::AddRect { path, insert_index, transform, style } => { let layer = Layer::new(LayerDataType::Shape(ShapeLayer::rectangle(style)), transform); self.set_layer(&path, layer, insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::AddLine { path, insert_index, transform, style } => { let layer = Layer::new(LayerDataType::Shape(ShapeLayer::line(style)), transform); self.set_layer(&path, layer, insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::AddFrame { path, @@ -503,7 +532,16 @@ impl Document { self.set_layer(&path, layer, insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::SetLayerPreserveAspect { layer_path, preserve_aspect } => { if let Ok(layer) = self.layer_mut(&layer_path) { @@ -520,7 +558,7 @@ impl Document { } => { let shape = ShapeLayer::new(subpath, style); self.set_layer(&path, Layer::new(LayerDataType::Shape(shape), transform), insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path }]].concat()) + Some(vec![DocumentChanged, CreatedLayer { path, is_selected: true }]) } Operation::AddPolyline { path, @@ -531,7 +569,17 @@ impl Document { } => { let points: Vec = points.iter().map(|&it| it.into()).collect(); self.set_layer(&path, Layer::new(LayerDataType::Shape(ShapeLayer::poly_line(points, style)), transform), insert_index)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::DeleteLayer { path } => { fn aggregate_deletions(folder: &FolderLayer, path: &mut Vec, responses: &mut Vec) { @@ -559,35 +607,94 @@ impl Document { destination_path, layer, insert_index, + duplicating, } => { let (folder_path, layer_id) = split_path(&destination_path)?; - let folder = self.folder_mut(folder_path)?; - folder.add_layer(*layer, Some(layer_id), insert_index).ok_or(DocumentError::IndexOutOfBounds)?; + let mut responses = vec![DocumentChanged]; + + // If we are duplicating, use the parent layer path as the folder we insert to + let (created_layer_path, folder_changed_path) = if duplicating { + let folder = self.folder_mut(&destination_path)?; + let new_layer_id = folder.add_layer(*layer, None, insert_index).ok_or(DocumentError::IndexOutOfBounds)?; + + ([destination_path.as_slice(), &[new_layer_id]].concat(), destination_path.clone()) + } else { + let folder = self.folder_mut(folder_path)?; + folder.add_layer(*layer, Some(layer_id), insert_index).ok_or(DocumentError::IndexOutOfBounds)?; + + (destination_path.clone(), folder_path.to_vec()) + }; + + responses.push(CreatedLayer { + path: created_layer_path, + is_selected: !duplicating, + }); + responses.push(FolderChanged { path: folder_changed_path.to_vec() }); + responses.extend(update_thumbnails_upstream(&destination_path)); + self.mark_as_dirty(&destination_path)?; - fn aggregate_insertions(folder: &FolderLayer, path: &mut Vec, responses: &mut Vec) { + // Recursively iterate through each layer in a folder and add it to the responses vector + fn aggregate_insertions(folder: &FolderLayer, path: &mut Vec, responses: &mut Vec, duplicating: bool) { for (id, layer) in folder.layer_ids.iter().zip(folder.layers()) { path.push(*id); - responses.push(DocumentResponse::CreatedLayer { path: path.clone() }); + + responses.push(DocumentResponse::CreatedLayer { + path: path.clone(), + is_selected: !duplicating, + }); if let LayerDataType::Folder(f) = &layer.data { - aggregate_insertions(f, path, responses); + aggregate_insertions(f, path, responses, duplicating); } + path.pop(); } } - let mut responses = Vec::new(); if let Ok(folder) = self.folder(&destination_path) { - aggregate_insertions(folder, &mut destination_path.as_slice().to_vec(), &mut responses) + aggregate_insertions(folder, &mut destination_path.as_slice().to_vec(), &mut responses, duplicating); }; - responses.extend([DocumentChanged, CreatedLayer { path: destination_path.clone() }, FolderChanged { path: folder_path.to_vec() }]); - responses.extend(update_thumbnails_upstream(&destination_path)); Some(responses) } Operation::DuplicateLayer { path } => { + // Notes for review: I wasn't sure on how to apply unwrap_or() to lines of code that use the function self.layer() + let layer = self.layer(&path)?.clone(); + let layer_is_folder = layer.as_folder().is_ok(); let (folder_path, _) = split_path(path.as_slice()).unwrap_or((&[], 0)); + + // Recursively collect each of the nested folders and shapes if the layer is a folder + fn recursive_collect(document: &mut Document, layer_path: &[u64]) -> Vec> { + let mut duplicated_layers_so_far = Vec::new(); + + let children = document.folder_children_paths(layer_path); + for child in children { + if document.is_folder(&child) { + duplicated_layers_so_far.push(child.to_vec()); + duplicated_layers_so_far.append(&mut recursive_collect(document, &child)); + } else { + duplicated_layers_so_far.push(child); + } + } + + duplicated_layers_so_far + } + let mut duplicated_layers = if layer_is_folder { recursive_collect(self, &path) } else { Vec::new() }; + + // Iterate through each layer path and collect the corresponding Layer objects into one vector + let duplicated_layers_objects = duplicated_layers + .iter() + .map(|layer_path| self.layer(layer_path.as_slice()).cloned().ok()) + .collect::>>() + .ok_or(DocumentError::InvalidPath)?; + + // Sort both vectors by the layer path depth, from shallowest (fewest) to deepest (most) + let mut indices: Vec = (0..duplicated_layers.len()).collect(); + indices.sort_by_key(|&i| duplicated_layers[i].len()); + duplicated_layers.sort_by_key(|a| a.len()); + let duplicate_layer_objects_sorted: Vec<&Layer> = indices.iter().map(|&i| &duplicated_layers_objects[i]).collect(); + let folder = self.folder_mut(folder_path)?; let selected_id = path.last().copied().unwrap_or_default(); @@ -595,14 +702,73 @@ impl Document { if let Some(new_layer_id) = folder.add_layer(layer, None, insert_index) { let new_path = [folder_path, &[new_layer_id]].concat(); + + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: new_path.clone(), + is_selected: true, + }, + FolderChanged { path: folder_path.to_vec() }, + ]; + responses.extend(update_thumbnails_upstream(path.as_slice())); + + if layer_is_folder { + let new_folder = self.folder_mut(&new_path)?; + // Clear the new folders layer_ids/layers because they contain the layer_ids/layers of the layers that were duplicated + new_folder.layer_ids = vec![]; + new_folder.layers = vec![]; + // Generate a new next assignment ID to avoid collision + new_folder.generate_new_folder_ids(); + + let mut old_to_new_layer_id: HashMap = HashMap::new(); + + for (i, duplicate_layer) in duplicated_layers.into_iter().enumerate() { + let Some(old_layer_id) = duplicate_layer.last().cloned() else { continue; }; + + // Iterate through each ID of the current duplicate layer + // If the dictionary contains the ID, we know the duplicate folder has been created already. Use the existing layer ID instead of creating a new one + let sub_layer = &duplicate_layer[new_path.len()..]; + let new_sub_path: Vec = sub_layer.iter().filter_map(|id| old_to_new_layer_id.get(id).cloned()).collect(); + + // Combine the new path with the IDs of the duplicate layer path to create the path where we insert the duplicate layer + let mut updated_layer = duplicate_layer_objects_sorted.get(i).unwrap().to_owned().clone(); + let updated_layer_path_parent = [new_path.clone(), new_sub_path].concat(); + + // Clear the new folder's layer_ids and layers because they contain the layer_ids/layers of the layer were duplicated + if self.is_folder(duplicate_layer) { + let updated_layer_as_folder: &mut FolderLayer = updated_layer.as_folder_mut()?; + updated_layer_as_folder.layer_ids = vec![]; + updated_layer_as_folder.layers = vec![]; + updated_layer_as_folder.generate_new_folder_ids() + } + + let result = self + .handle_operation(Operation::InsertLayer { + layer: Box::new(updated_layer), + destination_path: updated_layer_path_parent, + insert_index: -1, + duplicating: true, + }) + .ok() + .flatten() + .unwrap_or_default(); + + // Collect the new ID of the duplicated layer from the InsertLayer Operation + // Map the layer ID of the layer we're duplicating to the new ID + if let DocumentResponse::CreatedLayer { path, .. } = result.get(1).unwrap() { + if let Some(new_layer_id) = path.last() { + old_to_new_layer_id.entry(old_layer_id).or_insert(*new_layer_id); + } + } + + responses.extend(result); + } + } + self.mark_as_dirty(folder_path)?; - Some( - [ - vec![DocumentChanged, CreatedLayer { path: new_path }, FolderChanged { path: folder_path.to_vec() }], - update_thumbnails_upstream(path.as_slice()), - ] - .concat(), - ) + + Some(responses) } else { return Err(DocumentError::IndexOutOfBounds); } @@ -611,11 +777,20 @@ impl Document { self.layer_mut(&path)?.name = Some(name); Some(vec![LayerChanged { path }]) } - Operation::CreateFolder { path } => { - self.set_layer(&path, Layer::new(LayerDataType::Folder(FolderLayer::default()), DAffine2::IDENTITY.to_cols_array()), -1)?; + Operation::CreateFolder { path, insert_index } => { + self.set_layer(&path, Layer::new(LayerDataType::Folder(FolderLayer::default()), DAffine2::IDENTITY.to_cols_array()), insert_index)?; self.mark_as_dirty(&path)?; - Some([vec![DocumentChanged, CreatedLayer { path: path.clone() }], update_thumbnails_upstream(&path)].concat()) + let mut responses = vec![ + DocumentChanged, + CreatedLayer { + path: path.clone(), + is_selected: true, + }, + ]; + responses.extend(update_thumbnails_upstream(&path)); + + Some(responses) } Operation::TransformLayer { path, transform } => { let layer = self.layer_mut(&path).unwrap(); diff --git a/document-legacy/src/layers/folder_layer.rs b/document-legacy/src/layers/folder_layer.rs index c870f50f..b13ba36a 100644 --- a/document-legacy/src/layers/folder_layer.rs +++ b/document-legacy/src/layers/folder_layer.rs @@ -3,6 +3,8 @@ use super::style::RenderData; use crate::intersection::Quad; use crate::{DocumentError, LayerId}; +use graphene_core::uuid::generate_uuid; + use glam::DVec2; use serde::{Deserialize, Serialize}; @@ -147,6 +149,10 @@ impl FolderLayer { Some(&mut self.layers[pos]) } + pub fn generate_new_folder_ids(&mut self) { + self.next_assignment_id = generate_uuid(); + } + /// Returns `true` if the folder contains a layer with the given [LayerId]. /// /// # Example diff --git a/document-legacy/src/operation.rs b/document-legacy/src/operation.rs index 1a53fd9d..c3ff39a5 100644 --- a/document-legacy/src/operation.rs +++ b/document-legacy/src/operation.rs @@ -50,9 +50,11 @@ pub enum Operation { layer: Box, destination_path: Vec, insert_index: isize, + duplicating: bool, }, CreateFolder { path: Vec, + insert_index: isize, }, TransformLayer { path: Vec, diff --git a/document-legacy/src/response.rs b/document-legacy/src/response.rs index 3d1537dd..0f187f41 100644 --- a/document-legacy/src/response.rs +++ b/document-legacy/src/response.rs @@ -11,8 +11,12 @@ pub enum DocumentResponse { FolderChanged { path: Vec, }, + AddSelectedLayer { + additional_layers: Vec>, + }, CreatedLayer { path: Vec, + is_selected: bool, }, DeletedLayer { path: Vec, @@ -21,6 +25,11 @@ pub enum DocumentResponse { LayerChanged { path: Vec, }, + MoveSelectedLayersTo { + folder_path: Vec, + insert_index: isize, + reverse_index: bool, + }, DeletedSelectedManipulatorPoints, } @@ -29,10 +38,12 @@ impl fmt::Display for DocumentResponse { match self { DocumentResponse::DocumentChanged { .. } => write!(f, "DocumentChanged"), DocumentResponse::FolderChanged { .. } => write!(f, "FolderChanged"), + DocumentResponse::AddSelectedLayer { .. } => write!(f, "AddSelectedLayer"), DocumentResponse::CreatedLayer { .. } => write!(f, "CreatedLayer"), DocumentResponse::LayerChanged { .. } => write!(f, "LayerChanged"), DocumentResponse::DeletedLayer { .. } => write!(f, "DeleteLayer"), DocumentResponse::DeletedSelectedManipulatorPoints { .. } => write!(f, "DeletedSelectedManipulatorPoints"), + DocumentResponse::MoveSelectedLayersTo { .. } => write!(f, "MoveSelectedLayersTo"), } } } diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index bbca76b6..aae23128 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -124,11 +124,23 @@ impl MessageHandler responses.add(FolderChanged { affected_folder_path: path.clone() }), + DocumentResponse::AddSelectedLayer { additional_layers } => responses.add(AddSelectedLayers { + additional_layers: additional_layers.clone(), + }), DocumentResponse::DeletedLayer { path } => { self.layer_metadata.remove(path); } DocumentResponse::LayerChanged { path } => responses.add(LayerChanged { affected_layer_path: path.clone() }), - DocumentResponse::CreatedLayer { path } => { + DocumentResponse::MoveSelectedLayersTo { + folder_path, + insert_index, + reverse_index, + } => responses.add(MoveSelectedLayersTo { + folder_path: folder_path.clone(), + insert_index: insert_index.clone(), + reverse_index: reverse_index.clone(), + }), + DocumentResponse::CreatedLayer { path, is_selected } => { if self.layer_metadata.contains_key(path) { warn!("CreatedLayer overrides existing layer metadata."); } @@ -136,9 +148,12 @@ impl MessageHandler responses.add(RenderDocument), DocumentResponse::DeletedSelectedManipulatorPoints => { @@ -272,7 +287,10 @@ impl MessageHandler { + // TODO: Add code that changes the insert index of the new folder based on the selected layer let mut new_folder_path = self.document_legacy.shallowest_common_folder(self.selected_layers()).unwrap_or(&[]).to_vec(); // Required for grouping parent folders with their own children @@ -428,7 +447,10 @@ impl MessageHandler, incoming_layer_path_vector: &Vec) -> bool { - // TODO: fix below, then QA - // DOUBLE CLICK BROKEN let layer_paths = document.document_legacy.folder_children_paths(layer_path); for path in layer_paths { if path == *incoming_layer_path_vector {