jess
/
Graphite
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix group creation parenting (#455) 

* WIP fix of folder crash / indent

* Fixed most crashes

* Known cases of crash / incorrect behavior resolved

* Removed todo & readability tweak

* Removed left over test

* Resolved clippy issue resulting from merging master

* Replace recursive tree traversal with prefix matching

* Reverted changes for #453 and added undo functionality to Paste

* Make uuid generator thread local for tests

* Maintain layer order, test still failing 50% of the time

* Reverting back to known working with the knowledge we can optimize later.

This reverts commit 1aaf5c0d7dbe67cd9f4ba8b536a4771a2cef6439.

* Revert "Make uuid generator thread local for tests"

This reverts commit d68e3b9c4ebf639c4b2ecae43f5ad3d060dcb12b.

* Revert "Reverted changes for #453 and added undo functionality to Paste"

This reverts commit d66992ac9440f244d461a6a815ae2a08eede47d7.

* Revert "Replace recursive tree traversal with prefix matching"

This reverts commit 6bcbb9f82f62b1f9aa00287110fbce64e1d2f0db.

* Reverted to working state and added comment back to optimized commit hash

* Re-removed the changes involving #453

Co-authored-by: Dennis <dennis@kobert.dev>
This commit is contained in:
Oliver Davies 2022-01-04 19:18:28 -08:00 committed by Keavon Chambers
parent a805120638
commit f63d428d32
5 changed files with 77 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
editor/src/document/document_file.rs
View File

@ -273,6 +273,37 @@ impl DocumentMessageHandler {
self.layer_metadata.iter().filter_map(|(path, data)| data.selected.then(|| path.as_slice()))
}
pub fn selected_layers_without_children(&self) -> Vec<Vec<LayerId>> {
// TODO optimize this after MVP. Look at commit 1aaf5c0d7dbe67cd9f4ba8b536a4771a2cef6439, optimization was started
// Traversing the layer tree recursively was chosen for readability, but should be replaced with an optimized approach later
fn recurse_layer_tree(ctx: &DocumentMessageHandler, mut path: Vec<u64>, without_children: &mut Vec<Vec<LayerId>>, selected: bool) {
if let Ok(folder) = ctx.graphene_document.folder(&path) {
for child in folder.list_layers() {
path.push(*child);
let selected_or_parent_selected = selected || ctx.selected_layers_contains(&path);
let selected_without_any_parent_selected = !selected && ctx.selected_layers_contains(&path);
if ctx.graphene_document.is_folder(&path) {
if selected_without_any_parent_selected {
without_children.push(path.clone());
}
recurse_layer_tree(ctx, path.clone(), without_children, selected_or_parent_selected);
} else if selected_without_any_parent_selected {
without_children.push(path.clone());
}
path.pop();
}
}
}
let mut without_children: Vec<Vec<LayerId>> = vec![];
recurse_layer_tree(self, vec![], &mut without_children, false);
without_children
}
pub fn selected_layers_contains(&self, path: &[LayerId]) -> bool {
self.layer_metadata.get(path).map(|layer| layer.selected).unwrap_or(false)
}
pub fn selected_visible_layers(&self) -> impl Iterator<Item = &[LayerId]> {
self.selected_layers().filter(|path| match self.graphene_document.layer(path) {
Ok(layer) => layer.visible,
@ -562,12 +593,13 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
responses.push_back(DocumentMessage::SetLayerExpansion(path, true).into());
}
GroupSelectedLayers => {
let selected_layers = self.selected_layers();
let mut new_folder_path: Vec<u64> = self.graphene_document.shallowest_common_folder(self.selected_layers()).unwrap_or(&[]).to_vec();
let common_prefix = self.graphene_document.common_layer_path_prefix(selected_layers);
let (_id, common_prefix) = common_prefix.split_last().unwrap_or((&0, &[]));
// Required for grouping parent folders with their own children
if !new_folder_path.is_empty() && self.selected_layers_contains(&new_folder_path) {
new_folder_path.remove(new_folder_path.len() - 1);
}
let mut new_folder_path = common_prefix.to_vec();
new_folder_path.push(generate_uuid());
responses.push_back(DocumentsMessage::Copy(Clipboard::System).into());
@ -618,10 +650,12 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
}
DeleteSelectedLayers => {
self.backup(responses);
responses.push_front(ToolMessage::DocumentIsDirty.into());
for path in self.selected_layers().map(|path| path.to_vec()) {
for path in self.selected_layers_without_children() {
responses.push_front(DocumentOperation::DeleteLayer { path }.into());
}
responses.push_front(ToolMessage::DocumentIsDirty.into());
}
SetViewMode(mode) => {
self.view_mode = mode;
@ -652,6 +686,7 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
let layer = self.layer_metadata_mut(&selected);
layer.selected = !layer.selected;
responses.push_back(LayerChanged(selected.clone()).into());
responses.push_back(ToolMessage::DocumentIsDirty.into());
} else {
paths.push(selected.clone());
}

22
editor/src/document/document_message_handler.rs
View File

@ -352,15 +352,18 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
responses.push_back(DocumentsMessage::SelectDocument(prev_id).into());
}
Copy(clipboard) => {
let paths = self.active_document().selected_layers_sorted();
self.copy_buffer[clipboard as usize].clear();
for path in paths {
let document = self.active_document();
match (document.graphene_document.layer(&path).map(|t| t.clone()), *document.layer_metadata(&path)) {
// We can't use `self.active_document()` because it counts as an immutable borrow of the entirety of `self`
let active_document = self.documents.get(&self.active_document_id).unwrap();
let copy_buffer = &mut self.copy_buffer;
copy_buffer[clipboard as usize].clear();
for layer_path in active_document.selected_layers_without_children() {
match (active_document.graphene_document.layer(&layer_path).map(|t| t.clone()), *active_document.layer_metadata(&layer_path)) {
(Ok(layer), layer_metadata) => {
self.copy_buffer[clipboard as usize].push(CopyBufferEntry { layer, layer_metadata });
copy_buffer[clipboard as usize].push(CopyBufferEntry { layer, layer_metadata });
}
(Err(e), _) => warn!("Could not access selected layer {:?}: {:?}", path, e),
(Err(e), _) => warn!("Could not access selected layer {:?}: {:?}", layer_path, e),
}
}
}
@ -372,9 +375,9 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
let document = self.active_document();
let shallowest_common_folder = document
.graphene_document
.deepest_common_folder(document.selected_layers())
.shallowest_common_folder(document.selected_layers())
.expect("While pasting, the selected layers did not exist while attempting to find the appropriate folder path for insertion");
responses.push_back(StartTransaction.into());
responses.push_back(
PasteIntoFolder {
clipboard,
@ -383,6 +386,7 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
}
.into(),
);
responses.push_back(CommitTransaction.into());
}
PasteIntoFolder { clipboard, path, insert_index } => {
let paste = |entry: &CopyBufferEntry, responses: &mut VecDeque<_>| {

30
graphene/src/document.rs
View File

@ -61,7 +61,7 @@ impl Document {
pub fn folder(&self, path: &[LayerId]) -> Result<&Folder, DocumentError> {
let mut root = &self.root;
for id in path {
root = root.as_folder()?.layer(*id).ok_or(DocumentError::LayerNotFound)?;
root = root.as_folder()?.layer(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
}
root.as_folder()
}
@ -72,7 +72,7 @@ impl Document {
fn folder_mut(&mut self, path: &[LayerId]) -> Result<&mut Folder, DocumentError> {
let mut root = &mut self.root;
for id in path {
root = root.as_folder_mut()?.layer_mut(*id).ok_or(DocumentError::LayerNotFound)?;
root = root.as_folder_mut()?.layer_mut(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
}
root.as_folder_mut()
}
@ -83,7 +83,7 @@ impl Document {
return Ok(&self.root);
}
let (path, id) = split_path(path)?;
self.folder(path)?.layer(id).ok_or(DocumentError::LayerNotFound)
self.folder(path)?.layer(id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))
}
/// Returns a mutable reference to the layer or folder at the path.
@ -92,10 +92,10 @@ impl Document {
return Ok(&mut self.root);
}
let (path, id) = split_path(path)?;
self.folder_mut(path)?.layer_mut(id).ok_or(DocumentError::LayerNotFound)
self.folder_mut(path)?.layer_mut(id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))
}
pub fn deepest_common_folder<'a>(&self, layers: impl Iterator<Item = &'a [LayerId]>) -> Result<&'a [LayerId], DocumentError> {
pub fn shallowest_common_folder<'a>(&self, layers: impl Iterator<Item = &'a [LayerId]>) -> Result<&'a [LayerId], DocumentError> {
let common_prefix_of_path = self.common_layer_path_prefix(layers);
Ok(match self.layer(common_prefix_of_path)?.data {
@ -113,6 +113,10 @@ impl Document {
.unwrap_or_default()
}
pub fn is_folder(&self, path: &[LayerId]) -> bool {
return self.folder(path).is_ok();
}
// Determines which layer is closer to the root, if path_a return true, if path_b return false
// Answers the question: Is A closer to the root than B?
pub fn layer_closer_to_root(&self, path_a: &[u64], path_b: &[u64]) -> bool {
@ -136,9 +140,9 @@ impl Document {
false
}
// Is the target layer between a <-> b layers, inclusive
// Is the target layer between a <-> b layers, inclusive
pub fn layer_is_between(&self, target: &[u64], path_a: &[u64], path_b: &[u64]) -> bool {
// If the target is a nonsense path, it isn't between
// If the target is the root, it isn't between
if target.is_empty() {
return false;
}
@ -165,12 +169,12 @@ impl Document {
// TODO: appears to be n^2? should we maintain a lookup table?
for id in path {
let pos = root.layer_ids.iter().position(|x| *x == *id).ok_or(DocumentError::LayerNotFound)?;
let pos = root.layer_ids.iter().position(|x| *x == *id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
indices.push(pos);
root = root.folder(*id).ok_or(DocumentError::LayerNotFound)?;
root = root.folder(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
}
indices.push(root.layer_ids.iter().position(|x| *x == layer_id).ok_or(DocumentError::LayerNotFound)?);
indices.push(root.layer_ids.iter().position(|x| *x == layer_id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?);
Ok(indices)
}
@ -264,7 +268,7 @@ impl Document {
let mut root = &mut self.root;
root.cache_dirty = true;
for id in path {
root = root.as_folder_mut()?.layer_mut(*id).ok_or(DocumentError::LayerNotFound)?;
root = root.as_folder_mut()?.layer_mut(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
root.cache_dirty = true;
}
Ok(())
@ -297,7 +301,7 @@ impl Document {
let mut transforms = vec![self.root.transform];
for id in path {
if let Ok(folder) = root.as_folder() {
root = folder.layer(*id).ok_or(DocumentError::LayerNotFound)?;
root = folder.layer(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
}
transforms.push(root.transform);
}
@ -309,7 +313,7 @@ impl Document {
let mut trans = self.root.transform;
for id in path {
if let Ok(folder) = root.as_folder() {
root = folder.layer(*id).ok_or(DocumentError::LayerNotFound)?;
root = folder.layer(*id).ok_or_else(|| DocumentError::LayerNotFound(path.into()))?;
}
trans = trans * root.transform;
}

6
graphene/src/layers/folder.rs
View File

@ -101,8 +101,12 @@ impl Folder {
Some(&mut self.layers[pos])
}
pub fn folder_contains(&self, id: LayerId) -> bool {
self.layer_ids.contains(&id)
}
pub fn position_of_layer(&self, layer_id: LayerId) -> Result<usize, DocumentError> {
self.layer_ids.iter().position(|x| *x == layer_id).ok_or(DocumentError::LayerNotFound)
self.layer_ids.iter().position(|x| *x == layer_id).ok_or_else(|| DocumentError::LayerNotFound([layer_id].into()))
}
pub fn folder(&self, id: LayerId) -> Option<&Folder> {

2
graphene/src/lib.rs
View File

@ -14,7 +14,7 @@ pub type LayerId = u64;
#[derive(Debug, Clone, PartialEq)]
pub enum DocumentError {
LayerNotFound,
LayerNotFound(Vec<LayerId>),
InvalidPath,
IndexOutOfBounds,
NotAFolder,