Support rearranging layers with hotkeys (#271)

* Support moving single layers

* Fix "Move layer to top/bottom" keybinds

* Rename things named "move" to "reorder"

Fix formatting

* Combine sorted layer helper functions

* Use integer consts for moving layers to front/back

* Fix merge mistake

* Fix some clippy lints

* Fix panic

* Remove "get" prefix from functions

* Bring layer menu items out to sub-menu

* Support moving multiple layers at a time

* Add comment explaining odd keybinding

* Add reordering tests

* Add negative test

* Add new error type

* Add layer position helper, clean up tests

* Make position helper return Result

* Clean up slice iteration

* Simplify source_layer_ids computation

Co-authored-by: Dennis Kobert <dennis@kobert.dev>
This commit is contained in:
AJ Weeks 2021-07-21 10:36:51 +01:00 committed by GitHub
parent 255cdead28
commit a448b36d9e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 319 additions and 13 deletions

View file

@ -126,6 +126,17 @@ const menuEntries: MenuListEntries = [
[
{ label: "Select All", shortcut: ["Ctrl", "A"], action: async () => (await wasm).select_all_layers() },
{ label: "Deselect All", shortcut: ["Ctrl", "Alt", "A"], action: async () => (await wasm).deselect_all_layers() },
{
label: "Order",
children: [
[
{ label: "Raise To Front", shortcut: ["Ctrl", "Shift", "]"], action: async () => (await wasm).reorder_selected_layers(2147483647) },
{ label: "Raise", shortcut: ["Ctrl", "]"], action: async () => (await wasm).reorder_selected_layers(1) },
{ label: "Lower", shortcut: ["Ctrl", "["], action: async () => (await wasm).reorder_selected_layers(-1) },
{ label: "Lower to Back", shortcut: ["Ctrl", "Shift", "["], action: async () => (await wasm).reorder_selected_layers(-2147483648) },
],
],
},
],
],
},

View file

@ -176,7 +176,7 @@ pub fn select_all_layers() -> Result<(), JsValue> {
EDITOR_STATE.with(|editor| editor.borrow_mut().handle_message(DocumentMessage::SelectAllLayers)).map_err(convert_error)
}
/// Select all layers
/// Deselect all layers
#[wasm_bindgen]
pub fn deselect_all_layers() -> Result<(), JsValue> {
EDITOR_STATE
@ -184,6 +184,14 @@ pub fn deselect_all_layers() -> Result<(), JsValue> {
.map_err(convert_error)
}
/// Reorder selected layer
#[wasm_bindgen]
pub fn reorder_selected_layers(delta: i32) -> Result<(), JsValue> {
EDITOR_STATE
.with(|editor| editor.borrow_mut().handle_message(DocumentMessage::ReorderSelectedLayers(delta)))
.map_err(convert_error)
}
/// Export the document
#[wasm_bindgen]
pub fn export_document() -> Result<(), JsValue> {

View file

@ -131,6 +131,10 @@ pub fn translate_key(name: &str) -> Key {
"arrowdown" => KeyArrowDown,
"arrowleft" => KeyArrowLeft,
"arrowright" => KeyArrowRight,
"[" => KeyLeftBracket,
"]" => KeyRightBracket,
"{" => KeyLeftCurlyBracket,
"}" => KeyRightCurlyBracket,
_ => UnknownKey,
}
}

View file

@ -226,6 +226,19 @@ impl Document {
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(())
}
pub fn layer_axis_aligned_bounding_box(&self, path: &[LayerId]) -> Result<Option<[DVec2; 2]>, DocumentError> {
// TODO: Replace with functions of the transform api
if path.is_empty() {
@ -393,6 +406,11 @@ 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

@ -59,12 +59,78 @@ impl Folder {
}
pub fn remove_layer(&mut self, id: LayerId) -> Result<(), DocumentError> {
let pos = self.layer_ids.iter().position(|x| *x == id).ok_or(DocumentError::LayerNotFound)?;
let pos = self.position_of_layer(id)?;
self.layers.remove(pos);
self.layer_ids.remove(pos);
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()
@ -79,15 +145,19 @@ impl Folder {
}
pub fn layer(&self, id: LayerId) -> Option<&Layer> {
let pos = self.layer_ids.iter().position(|x| *x == id)?;
let pos = self.position_of_layer(id).ok()?;
Some(&self.layers[pos])
}
pub fn layer_mut(&mut self, id: LayerId) -> Option<&mut Layer> {
let pos = self.layer_ids.iter().position(|x| *x == id)?;
let pos = self.position_of_layer(id).ok()?;
Some(&mut self.layers[pos])
}
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)
}
pub fn folder(&self, id: LayerId) -> Option<&Folder> {
match self.layer(id) {
Some(Layer {
@ -143,3 +213,144 @@ 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

@ -16,4 +16,5 @@ pub enum DocumentError {
InvalidPath,
IndexOutOfBounds,
NotAFolder,
NonReorderableSelection,
}

View file

@ -76,4 +76,8 @@ pub enum Operation {
path: Vec<LayerId>,
color: Color,
},
ReorderLayers {
source_paths: Vec<Vec<LayerId>>,
target_path: Vec<LayerId>,
},
}

View file

@ -54,6 +54,7 @@ pub enum DocumentMessage {
WheelCanvasZoom,
SetCanvasRotation(f64),
NudgeSelectedLayers(f64, f64),
ReorderSelectedLayers(i32),
}
impl From<DocumentOperation> for DocumentMessage {
@ -136,21 +137,23 @@ impl DocumentMessageHandler {
);
}
/// Returns the paths to the selected layers in order
fn selected_layers_sorted(&self) -> Vec<Vec<LayerId>> {
/// Returns the paths to all layers in order, optionally including only selected layers
fn layers_sorted(&self, only_selected: bool) -> Vec<Vec<LayerId>> {
// Compute the indices for each layer to be able to sort them
// TODO: Replace with drain_filter https://github.com/rust-lang/rust/issues/59618
let mut layers_with_indices: Vec<(Vec<LayerId>, Vec<usize>)> = self
.active_document()
.layer_data
.iter()
.filter_map(|(path, data)| data.selected.then(|| path.clone()))
// 'path.len() > 0' filters out root layer since it has no indices
.filter_map(|(path, data)| (!path.is_empty() && !only_selected || data.selected).then(|| path.clone()))
.filter_map(|path| {
// Currently it is possible that layer_data contains layers that are don't actually exist
// and thus indices_for_path can return an error. We currently skip these layers and log a warning.
// Once this problem is solved this code can be simplified
match self.active_document().document.indices_for_path(&path) {
Err(err) => {
warn!("selected_layers_sorted: Could not get indices for the layer {:?}: {:?}", path, err);
warn!("layers_sorted: Could not get indices for the layer {:?}: {:?}", path, err);
None
}
Ok(indices) => Some((path, indices)),
@ -161,6 +164,16 @@ impl DocumentMessageHandler {
layers_with_indices.sort_by_key(|(_, indices)| indices.clone());
layers_with_indices.into_iter().map(|(path, _)| path).collect()
}
/// Returns the paths to all layers in order
fn all_layers_sorted(&self) -> Vec<Vec<LayerId>> {
self.layers_sorted(false)
}
/// Returns the paths to all selected layers in order
fn selected_layers_sorted(&self) -> Vec<Vec<LayerId>> {
self.layers_sorted(true)
}
}
impl Default for DocumentMessageHandler {
@ -336,20 +349,19 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
responses.extend(self.handle_folder_changed(path));
}
DeleteSelectedLayers => {
// TODO: Replace with drain_filter https://github.com/rust-lang/rust/issues/59618
let paths: Vec<Vec<LayerId>> = self.active_document().layer_data.iter().filter_map(|(path, data)| data.selected.then(|| path.clone())).collect();
let paths = self.selected_layers_sorted();
for path in paths {
self.active_document_mut().layer_data.remove(&path);
responses.push_back(DocumentOperation::DeleteLayer { path }.into())
}
}
DuplicateSelectedLayers => {
for path in self.active_document().layer_data.iter().filter_map(|(path, data)| data.selected.then(|| path.clone())) {
for path in self.selected_layers_sorted() {
responses.push_back(DocumentOperation::DuplicateLayer { path }.into())
}
}
CopySelectedLayers => {
let paths: Vec<Vec<LayerId>> = self.selected_layers_sorted();
let paths = self.selected_layers_sorted();
self.copy_buffer.clear();
for path in paths {
match self.active_document().document.layer(&path).map(|t| t.clone()) {
@ -539,7 +551,7 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
responses.push_back(FrontendMessage::SetCanvasRotation { new_radians: new }.into());
}
NudgeSelectedLayers(x, y) => {
let paths: Vec<Vec<LayerId>> = self.selected_layers_sorted();
let paths = self.selected_layers_sorted();
let delta = {
let root_layer_rotation = self.layerdata_mut(&[]).rotation;
@ -554,6 +566,34 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
responses.push_back(operation.into());
}
}
ReorderSelectedLayers(delta) => {
let selected_layer_paths: Vec<Vec<LayerId>> = self.selected_layers_sorted();
let all_layer_paths = self.all_layers_sorted();
let max_index = all_layer_paths.len() as i64 - 1;
let num_layers_selected = selected_layer_paths.len() as i64;
let mut selected_layer_index = -1;
let mut next_layer_index = -1;
for (i, path) in all_layer_paths.iter().enumerate() {
if *path == selected_layer_paths[0] {
selected_layer_index = i as i32;
// Skip past selection length when moving up
let offset = if delta > 0 { num_layers_selected - 1 } else { 0 };
next_layer_index = (selected_layer_index as i64 + delta as i64 + offset).clamp(0, max_index) as i32;
break;
}
}
if next_layer_index != -1 && next_layer_index != selected_layer_index {
let operation = DocumentOperation::ReorderLayers {
source_paths: selected_layer_paths.clone(),
target_path: all_layer_paths[next_layer_index as usize].to_vec(),
};
responses.push_back(operation.into());
responses.push_back(DocumentMessage::SelectLayers(selected_layer_paths).into());
}
}
message => todo!("document_action_handler does not implement: {}", message.to_discriminant().global_name()),
}
}
@ -589,6 +629,7 @@ impl MessageHandler<DocumentMessage, &InputPreprocessor> for DocumentMessageHand
DuplicateSelectedLayers,
CopySelectedLayers,
NudgeSelectedLayers,
ReorderSelectedLayers,
);
common.extend(select);
}

View file

@ -235,6 +235,10 @@ impl Default for Mapping {
entry! {action=DocumentMessage::NudgeSelectedLayers(NUDGE_AMOUNT, -NUDGE_AMOUNT), key_down=KeyArrowRight, modifiers=[KeyArrowUp]},
entry! {action=DocumentMessage::NudgeSelectedLayers(NUDGE_AMOUNT, NUDGE_AMOUNT), key_down=KeyArrowRight, modifiers=[KeyArrowDown]},
entry! {action=DocumentMessage::NudgeSelectedLayers(NUDGE_AMOUNT, 0.), key_down=KeyArrowRight},
entry! {action=DocumentMessage::ReorderSelectedLayers(i32::MAX), key_down=KeyRightCurlyBracket, modifiers=[KeyControl]}, // TODO: Use KeyRightBracket with ctrl+shift modifiers once input system is fixed
entry! {action=DocumentMessage::ReorderSelectedLayers(1), key_down=KeyRightBracket, modifiers=[KeyControl]},
entry! {action=DocumentMessage::ReorderSelectedLayers(-1), key_down=KeyLeftBracket, modifiers=[KeyControl]},
entry! {action=DocumentMessage::ReorderSelectedLayers(i32::MIN), key_down=KeyLeftCurlyBracket, modifiers=[KeyControl]}, // TODO: Use KeyLeftBracket with ctrl+shift modifiers once input system is fixed
// Global Actions
entry! {action=GlobalMessage::LogInfo, key_down=Key1},
entry! {action=GlobalMessage::LogDebug, key_down=Key2},

View file

@ -69,6 +69,10 @@ pub enum Key {
KeyArrowDown,
KeyArrowLeft,
KeyArrowRight,
KeyLeftBracket,
KeyRightBracket,
KeyLeftCurlyBracket,
KeyRightCurlyBracket,
// This has to be the last element in the enum.
NumKeys,