Rearrange layers refactor (#281)

* Keep selection during reordering

* Fix paste layer selection

* Remove junk from layer matadata

* Add function to get non_selected_layers

* Cleanup

* Add tests
This commit is contained in:
TrueDoctor
2021-07-24 00:54:30 +02:00
committed by GitHub
parent d30d44eb70
commit be668b36ab
12 changed files with 155 additions and 299 deletions

View File

@@ -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<LayerId>], 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::<Result<Vec<LayerId>, 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<Option<[DVec2; 2]>, DocumentError> {
@@ -268,6 +251,16 @@ impl Document {
Ok(())
}
fn working_paths(&mut self) -> Vec<Vec<LayerId>> {
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<Option<Vec<DocumentResponse>>, DocumentError> {
@@ -276,19 +269,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,
@@ -301,7 +294,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,
@@ -315,20 +308,29 @@ 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)?;
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 } => {
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)?;
let id = folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?;
let full_path = [path.clone(), vec![id]].concat();
Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.clone() }])
Some(vec![
DocumentResponse::DocumentChanged,
DocumentResponse::CreatedLayer { path: full_path },
DocumentResponse::FolderChanged { path: path.clone() },
])
}
Operation::DuplicateLayer { path } => {
let layer = self.layer(&path)?.clone();
@@ -343,11 +345,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();
@@ -365,18 +369,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<LayerId> = vec![];
std::mem::swap(&mut path, &mut self.work_mount_path);
@@ -384,7 +393,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);
@@ -416,11 +424,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,

View File

@@ -65,72 +65,6 @@ impl Folder {
Ok(())
}
pub fn reorder_layers(&mut self, source_ids: Vec<LayerId>, 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<T>(arr: &mut Vec<T>, 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<T>(arr: &mut Vec<T>, 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);
}
}

View File

@@ -51,6 +51,7 @@ pub enum Operation {
PasteLayer {
layer: Layer,
path: Vec<LayerId>,
insert_index: isize,
},
AddFolder {
path: Vec<LayerId>,
@@ -80,8 +81,4 @@ pub enum Operation {
path: Vec<LayerId>,
color: Color,
},
ReorderLayers {
source_paths: Vec<Vec<LayerId>>,
target_path: Vec<LayerId>,
},
}

View File

@@ -7,7 +7,8 @@ use std::fmt;
pub enum DocumentResponse {
DocumentChanged,
FolderChanged { path: Vec<LayerId> },
SelectLayer { path: Vec<LayerId> },
CreatedLayer { path: Vec<LayerId> },
DeletedLayer { path: Vec<LayerId> },
}
impl fmt::Display for DocumentResponse {
@@ -15,7 +16,8 @@ impl fmt::Display for DocumentResponse {
let name = match self {
DocumentResponse::DocumentChanged { .. } => "DocumentChanged",
DocumentResponse::FolderChanged { .. } => "FolderChanged",
DocumentResponse::SelectLayer { .. } => "SelectLayer",
DocumentResponse::CreatedLayer { .. } => "SelectLayer",
DocumentResponse::DeletedLayer { .. } => "DeleteLayer",
};
formatter.write_str(name)