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 <olliedolan10@gmail.com>
Co-authored-by: Keavon Chambers <keavon@keavon.com>

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<glam::DVec2> = 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<LayerId>, responses: &mut Vec<DocumentResponse>) {
@@ -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)?;
+				let mut responses = vec![DocumentChanged];
-				folder.add_layer(*layer, Some(layer_id), insert_index).ok_or(DocumentError::IndexOutOfBounds)?;
+
+				// 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<LayerId>, responses: &mut Vec<DocumentResponse>) {
+				// Recursively iterate through each layer in a folder and add it to the responses vector
+				fn aggregate_insertions(folder: &FolderLayer, path: &mut Vec<LayerId>, responses: &mut Vec<DocumentResponse>, 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<Vec<u64>> {
+					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::<Option<Vec<_>>>()
+					.ok_or(DocumentError::InvalidPath)?;
+
+				// Sort both vectors by the layer path depth, from shallowest (fewest) to deepest (most)
+				let mut indices: Vec<usize> = (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<LayerId, LayerId> = 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<u64> = 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(
+
-						[
+					Some(responses)
-							vec![DocumentChanged, CreatedLayer { path: new_path }, FolderChanged { path: folder_path.to_vec() }],
-							update_thumbnails_upstream(path.as_slice()),
-						]
-						.concat(),
-					)
 				} 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 } => {
+			Operation::CreateFolder { path, insert_index } => {
-				self.set_layer(&path, Layer::new(LayerDataType::Folder(FolderLayer::default()), DAffine2::IDENTITY.to_cols_array()), -1)?;
+				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();

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

document-legacy/src/operation.rs

@@ -50,9 +50,11 @@ pub enum Operation {
 		layer: Box<Layer>,
 		destination_path: Vec<LayerId>,
 		insert_index: isize,
+		duplicating: bool,
 	},
 	CreateFolder {
 		path: Vec<LayerId>,
+		insert_index: isize,
 	},
 	TransformLayer {
 		path: Vec<LayerId>,

document-legacy/src/response.rs

@@ -11,8 +11,12 @@ pub enum DocumentResponse {
 	FolderChanged {
 		path: Vec<LayerId>,
 	},
+	AddSelectedLayer {
+		additional_layers: Vec<Vec<LayerId>>,
+	},
 	CreatedLayer {
 		path: Vec<LayerId>,
+		is_selected: bool,
 	},
 	DeletedLayer {
 		path: Vec<LayerId>,
@@ -21,6 +25,11 @@ pub enum DocumentResponse {
 	LayerChanged {
 		path: Vec<LayerId>,
 	},
+	MoveSelectedLayersTo {
+		folder_path: Vec<LayerId>,
+		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"),
 		}
 	}
 }

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -124,11 +124,23 @@ impl MessageHandler<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 						for response in document_responses {
 							match &response {
 								DocumentResponse::FolderChanged { path } => 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<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 
 									responses.add(LayerChanged { affected_layer_path: path.clone() });
 									self.layer_range_selection_reference = path.clone();
-									responses.add(AddSelectedLayers {
+
-										additional_layers: vec![path.clone()],
+									if *is_selected {
-									});
+										responses.add(AddSelectedLayers {
+											additional_layers: vec![path.clone()],
+										});
+									}
 								}
 								DocumentResponse::DocumentChanged => responses.add(RenderDocument),
 								DocumentResponse::DeletedSelectedManipulatorPoints => {
@@ -272,7 +287,10 @@ impl MessageHandler<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 				let id = generate_uuid();
 				container_path.push(id);
 				responses.add(DocumentMessage::DeselectAllLayers);
-				responses.add(DocumentOperation::CreateFolder { path: container_path.clone() });
+				responses.add(DocumentOperation::CreateFolder {
+					path: container_path.clone(),
+					insert_index: -1,
+				});
 				responses.add(DocumentMessage::SetLayerExpansion {
 					layer_path: container_path,
 					set_expanded: true,
@@ -417,6 +435,7 @@ impl MessageHandler<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 				responses.add(DocumentOperation::ClearBlobURL { path: layer_path.into() });
 			}
 			GroupSelectedLayers => {
+				// 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<DocumentMessage, (u64, &InputPreprocessorMessageHandler, &Pe
 
 				responses.add(PortfolioMessage::Copy { clipboard: Clipboard::Internal });
 				responses.add(DocumentMessage::DeleteSelectedLayers);
-				responses.add(DocumentOperation::CreateFolder { path: new_folder_path.clone() });
+				responses.add(DocumentOperation::CreateFolder {
+					path: new_folder_path.clone(),
+					insert_index: -1,
+				});
 				responses.add(DocumentMessage::ToggleLayerExpansion { layer_path: new_folder_path.clone() });
 				responses.add(PortfolioMessage::PasteIntoFolder {
 					clipboard: Clipboard::Internal,

editor/src/messages/portfolio/portfolio_message_handler.rs

@@ -395,6 +395,7 @@ impl MessageHandler<PortfolioMessage, (&InputPreprocessorMessageHandler, &Prefer
 							layer: Box::new(entry.layer.clone()),
 							destination_path,
 							insert_index,
+							duplicating: false,
 						});
 					}
 				};
@@ -431,6 +432,7 @@ impl MessageHandler<PortfolioMessage, (&InputPreprocessorMessageHandler, &Prefer
 								layer: Box::new(entry.layer.clone()),
 								destination_path,
 								insert_index: -1,
+								duplicating: false,
 							});
 						}
 

editor/src/messages/tool/tool_messages/select_tool.rs

@@ -368,6 +368,7 @@ impl SelectToolData {
 				layer: Box::new(layer),
 				destination_path: layer_path.clone(),
 				insert_index: -1,
+				duplicating: false,
 			});
 			responses.add(DocumentMessage::UpdateLayerMetadata {
 				layer_path: layer_path.clone(),
@@ -1219,8 +1220,6 @@ fn edit_layer_deepest_manipulation(intersect: &Layer, responses: &mut VecDeque<M
 }
 
 fn recursive_search(document: &DocumentMessageHandler, layer_path: &Vec<u64>, incoming_layer_path_vector: &Vec<u64>) -> 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 {