Skip to content

Commit cbfb93f

Browse files
committed
Refactor rearrange layers
1 parent 5e8a693 commit cbfb93f

File tree

6 files changed

+41
-271
lines changed

6 files changed

+41
-271
lines changed

core/document/src/document.rs

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@ impl Default for Document {
2727
}
2828

2929
fn split_path(path: &[LayerId]) -> Result<(&[LayerId], LayerId), DocumentError> {
30-
let id = path.last().ok_or(DocumentError::InvalidPath)?;
31-
let folder_path = &path[0..path.len() - 1];
32-
Ok((folder_path, *id))
30+
let (id, path) = path.split_last().ok_or(DocumentError::InvalidPath)?;
31+
Ok((path, *id))
3332
}
3433

3534
impl Document {
@@ -222,21 +221,7 @@ impl Document {
222221
pub fn delete(&mut self, path: &[LayerId]) -> Result<(), DocumentError> {
223222
let (path, id) = split_path(path)?;
224223
let _ = self.layer_mut(path).map(|x| x.cache_dirty = true);
225-
self.document_folder_mut(path)?.as_folder_mut()?.remove_layer(id)?;
226-
Ok(())
227-
}
228-
229-
pub fn reorder_layers(&mut self, source_paths: &[Vec<LayerId>], target_path: &[LayerId]) -> Result<(), DocumentError> {
230-
// TODO: Detect when moving between folders and handle properly
231-
232-
let source_layer_ids = source_paths
233-
.iter()
234-
.map(|x| x.last().cloned().ok_or(DocumentError::LayerNotFound))
235-
.collect::<Result<Vec<LayerId>, DocumentError>>()?;
236-
237-
self.root.as_folder_mut()?.reorder_layers(source_layer_ids, *target_path.last().ok_or(DocumentError::LayerNotFound)?)?;
238-
239-
Ok(())
224+
self.document_folder_mut(path)?.as_folder_mut()?.remove_layer(id)
240225
}
241226

242227
pub fn layer_axis_aligned_bounding_box(&self, path: &[LayerId]) -> Result<Option<[DVec2; 2]>, DocumentError> {
@@ -321,10 +306,10 @@ impl Document {
321306
let (path, _) = split_path(path.as_slice()).unwrap_or_else(|_| (&[], 0));
322307
Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.to_vec() }])
323308
}
324-
Operation::PasteLayer { path, layer } => {
309+
Operation::PasteLayer { path, layer, insert_index } => {
325310
let folder = self.folder_mut(path)?;
326311
//FIXME: This clone of layer should be avoided somehow
327-
folder.add_layer(layer.clone(), -1).ok_or(DocumentError::IndexOutOfBounds)?;
312+
folder.add_layer(layer.clone(), *insert_index).ok_or(DocumentError::IndexOutOfBounds)?;
328313

329314
Some(vec![DocumentResponse::DocumentChanged, DocumentResponse::FolderChanged { path: path.clone() }])
330315
}
@@ -406,11 +391,6 @@ impl Document {
406391
self.mark_as_dirty(path)?;
407392
Some(vec![DocumentResponse::DocumentChanged])
408393
}
409-
Operation::ReorderLayers { source_paths, target_path } => {
410-
self.reorder_layers(source_paths, target_path)?;
411-
412-
Some(vec![DocumentResponse::DocumentChanged])
413-
}
414394
};
415395
if !matches!(
416396
operation,

core/document/src/layers/folder.rs

Lines changed: 0 additions & 207 deletions
Original file line numberDiff line numberDiff line change
@@ -65,72 +65,6 @@ impl Folder {
6565
Ok(())
6666
}
6767

68-
pub fn reorder_layers(&mut self, source_ids: Vec<LayerId>, target_id: LayerId) -> Result<(), DocumentError> {
69-
let source_pos = self.position_of_layer(source_ids[0])?;
70-
let source_pos_end = source_pos + source_ids.len() - 1;
71-
let target_pos = self.position_of_layer(target_id)?;
72-
73-
let mut last_pos = source_pos;
74-
for layer_id in &source_ids[1..] {
75-
let layer_pos = self.position_of_layer(*layer_id)?;
76-
if (layer_pos as i32 - last_pos as i32).abs() > 1 {
77-
// Selection is not contiguous
78-
return Err(DocumentError::NonReorderableSelection);
79-
}
80-
last_pos = layer_pos;
81-
}
82-
83-
if source_pos < target_pos {
84-
// Moving layers up the hierarchy
85-
86-
// Prevent shifting past end
87-
if source_pos_end + 1 >= self.layers.len() {
88-
return Err(DocumentError::NonReorderableSelection);
89-
}
90-
91-
fn reorder_up<T>(arr: &mut Vec<T>, source_pos: usize, source_pos_end: usize, target_pos: usize)
92-
where
93-
T: Clone,
94-
{
95-
*arr = [
96-
&arr[0..source_pos], // Elements before selection
97-
&arr[source_pos_end + 1..=target_pos], // Elements between selection end and target
98-
&arr[source_pos..=source_pos_end], // Selection itself
99-
&arr[target_pos + 1..], // Elements before target
100-
]
101-
.concat();
102-
}
103-
104-
reorder_up(&mut self.layers, source_pos, source_pos_end, target_pos);
105-
reorder_up(&mut self.layer_ids, source_pos, source_pos_end, target_pos);
106-
} else {
107-
// Moving layers down the hierarchy
108-
109-
// Prevent shifting past end
110-
if source_pos == 0 {
111-
return Err(DocumentError::NonReorderableSelection);
112-
}
113-
114-
fn reorder_down<T>(arr: &mut Vec<T>, source_pos: usize, source_pos_end: usize, target_pos: usize)
115-
where
116-
T: Clone,
117-
{
118-
*arr = [
119-
&arr[0..target_pos], // Elements before target
120-
&arr[source_pos..=source_pos_end], // Selection itself
121-
&arr[target_pos..source_pos], // Elements between selection and target
122-
&arr[source_pos_end + 1..], // Elements before selection
123-
]
124-
.concat();
125-
}
126-
127-
reorder_down(&mut self.layers, source_pos, source_pos_end, target_pos);
128-
reorder_down(&mut self.layer_ids, source_pos, source_pos_end, target_pos);
129-
}
130-
131-
Ok(())
132-
}
133-
13468
/// Returns a list of layers in the folder
13569
pub fn list_layers(&self) -> &[LayerId] {
13670
self.layer_ids.as_slice()
@@ -209,144 +143,3 @@ impl Default for Folder {
209143
}
210144
}
211145
}
212-
213-
#[cfg(test)]
214-
mod test {
215-
use glam::{DAffine2, DVec2};
216-
217-
use crate::layers::{style::PathStyle, Ellipse, Layer, LayerDataTypes, Line, PolyLine, Rect, Shape};
218-
219-
use super::Folder;
220-
221-
#[test]
222-
fn reorder_layers() {
223-
let mut folder = Folder::default();
224-
225-
let identity_transform = DAffine2::IDENTITY.to_cols_array();
226-
folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0);
227-
folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1);
228-
folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2);
229-
folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3);
230-
folder.add_layer(
231-
Layer::new(LayerDataTypes::PolyLine(PolyLine::new(vec![DVec2::ZERO, DVec2::ONE])), identity_transform, PathStyle::default()),
232-
4,
233-
);
234-
235-
assert_eq!(folder.layer_ids[0], 0);
236-
assert_eq!(folder.layer_ids[1], 1);
237-
assert_eq!(folder.layer_ids[2], 2);
238-
assert_eq!(folder.layer_ids[3], 3);
239-
assert_eq!(folder.layer_ids[4], 4);
240-
241-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
242-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
243-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
244-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
245-
assert!(matches!(folder.layer(4).unwrap().data, LayerDataTypes::PolyLine(_)));
246-
247-
assert_eq!(folder.layer_ids.len(), 5);
248-
assert_eq!(folder.layers.len(), 5);
249-
250-
folder.reorder_layers(vec![0, 1], 2).unwrap();
251-
252-
assert_eq!(folder.layer_ids[0], 2);
253-
// Moved layers
254-
assert_eq!(folder.layer_ids[1], 0);
255-
assert_eq!(folder.layer_ids[2], 1);
256-
257-
assert_eq!(folder.layer_ids[3], 3);
258-
assert_eq!(folder.layer_ids[4], 4);
259-
260-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
261-
// Moved layers
262-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
263-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
264-
265-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
266-
assert!(matches!(folder.layer(4).unwrap().data, LayerDataTypes::PolyLine(_)));
267-
268-
assert_eq!(folder.layer_ids.len(), 5);
269-
assert_eq!(folder.layers.len(), 5);
270-
}
271-
272-
#[test]
273-
fn reorder_layer_to_top() {
274-
let mut folder = Folder::default();
275-
276-
let identity_transform = DAffine2::IDENTITY.to_cols_array();
277-
folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0);
278-
folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1);
279-
folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2);
280-
folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3);
281-
282-
assert_eq!(folder.layer_ids[0], 0);
283-
assert_eq!(folder.layer_ids[1], 1);
284-
assert_eq!(folder.layer_ids[2], 2);
285-
assert_eq!(folder.layer_ids[3], 3);
286-
287-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
288-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
289-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
290-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
291-
292-
assert_eq!(folder.layer_ids.len(), 4);
293-
assert_eq!(folder.layers.len(), 4);
294-
295-
folder.reorder_layers(vec![1], 3).unwrap();
296-
297-
assert_eq!(folder.layer_ids[0], 0);
298-
assert_eq!(folder.layer_ids[1], 2);
299-
assert_eq!(folder.layer_ids[2], 3);
300-
// Moved layer
301-
assert_eq!(folder.layer_ids[3], 1);
302-
303-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
304-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
305-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
306-
// Moved layer
307-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
308-
309-
assert_eq!(folder.layer_ids.len(), 4);
310-
assert_eq!(folder.layers.len(), 4);
311-
}
312-
313-
#[test]
314-
fn reorder_non_contiguous_selection() {
315-
let mut folder = Folder::default();
316-
317-
let identity_transform = DAffine2::IDENTITY.to_cols_array();
318-
folder.add_layer(Layer::new(LayerDataTypes::Shape(Shape::new(true, 3)), identity_transform, PathStyle::default()), 0);
319-
folder.add_layer(Layer::new(LayerDataTypes::Rect(Rect::default()), identity_transform, PathStyle::default()), 1);
320-
folder.add_layer(Layer::new(LayerDataTypes::Ellipse(Ellipse::default()), identity_transform, PathStyle::default()), 2);
321-
folder.add_layer(Layer::new(LayerDataTypes::Line(Line::default()), identity_transform, PathStyle::default()), 3);
322-
323-
assert_eq!(folder.layer_ids[0], 0);
324-
assert_eq!(folder.layer_ids[1], 1);
325-
assert_eq!(folder.layer_ids[2], 2);
326-
assert_eq!(folder.layer_ids[3], 3);
327-
328-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
329-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
330-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
331-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
332-
333-
assert_eq!(folder.layer_ids.len(), 4);
334-
assert_eq!(folder.layers.len(), 4);
335-
336-
folder.reorder_layers(vec![0, 2], 3).expect_err("Non-contiguous selections can't be reordered");
337-
338-
// Expect identical state
339-
assert_eq!(folder.layer_ids[0], 0);
340-
assert_eq!(folder.layer_ids[1], 1);
341-
assert_eq!(folder.layer_ids[2], 2);
342-
assert_eq!(folder.layer_ids[3], 3);
343-
344-
assert!(matches!(folder.layer(0).unwrap().data, LayerDataTypes::Shape(_)));
345-
assert!(matches!(folder.layer(1).unwrap().data, LayerDataTypes::Rect(_)));
346-
assert!(matches!(folder.layer(2).unwrap().data, LayerDataTypes::Ellipse(_)));
347-
assert!(matches!(folder.layer(3).unwrap().data, LayerDataTypes::Line(_)));
348-
349-
assert_eq!(folder.layer_ids.len(), 4);
350-
assert_eq!(folder.layers.len(), 4);
351-
}
352-
}

core/document/src/operation.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ pub enum Operation {
5151
PasteLayer {
5252
layer: Layer,
5353
path: Vec<LayerId>,
54+
insert_index: isize,
5455
},
5556
AddFolder {
5657
path: Vec<LayerId>,
@@ -76,8 +77,4 @@ pub enum Operation {
7677
path: Vec<LayerId>,
7778
color: Color,
7879
},
79-
ReorderLayers {
80-
source_paths: Vec<Vec<LayerId>>,
81-
target_path: Vec<LayerId>,
82-
},
8380
}

core/editor/src/communication/dispatcher.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ mod test {
123123

124124
let document_before_copy = editor.dispatcher.document_message_handler.active_document().document.clone();
125125
editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap();
126-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
126+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
127127
let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone();
128128

129129
let layers_before_copy = document_before_copy.root.as_folder().unwrap().layers();
@@ -156,7 +156,7 @@ mod test {
156156

157157
editor.handle_message(Message::Document(DocumentMessage::SelectLayers(vec![vec![shape_id]]))).unwrap();
158158
editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap();
159-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
159+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
160160

161161
let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone();
162162

@@ -220,8 +220,8 @@ mod test {
220220

221221
editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap();
222222
editor.handle_message(Message::Document(DocumentMessage::DeleteSelectedLayers)).unwrap();
223-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
224-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
223+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
224+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
225225

226226
let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone();
227227

@@ -282,8 +282,8 @@ mod test {
282282
editor.handle_message(Message::Document(DocumentMessage::CopySelectedLayers)).unwrap();
283283
editor.handle_message(Message::Document(DocumentMessage::DeleteSelectedLayers)).unwrap();
284284
editor.draw_rect(0, 800, 12, 200);
285-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
286-
editor.handle_message(Message::Document(DocumentMessage::PasteLayers)).unwrap();
285+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
286+
editor.handle_message(Message::Document(DocumentMessage::PasteLayers { path: vec![], insert_index: -1 })).unwrap();
287287

288288
let document_after_copy = editor.dispatcher.document_message_handler.active_document().document.clone();
289289

0 commit comments

Comments
 (0)