Fix layer deletion bugs when tools are in use (#684)

* Added test case of layer delete bug

* Fixed crash in `PenTool`

Updated the document `MessageHandler` to cancel all active tools if the
layer is deleted.  This prevents the tools from crashing due to the
layer being pulled from under them.

* Moved Abort into pre-graphene DeleteLayer message

* Renamed test case for clarity

* Moved tool crash tests to the `tools` module

* Added `test-case` to the dev dependencies

* Added crash test case for all tools

* Ran cargo fmt

Co-authored-by: otdavies <oliver@psyfer.io>
Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
Alexi 2022-06-24 09:35:07 -05:00 committed by Keavon Chambers
parent 5016abd971
commit 91219cbe64
5 changed files with 89 additions and 1 deletions

53
Cargo.lock generated
View file

@ -146,6 +146,7 @@ dependencies = [
"serde",
"serde_json",
"spin",
"test-case",
"thiserror",
]
@ -267,6 +268,30 @@ version = "0.2.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eb9f9e6e233e5c4a35559a617bf40a4ec447db2e84c20b55a6f83167b7e57872"
[[package]]
name = "proc-macro-error"
version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c"
dependencies = [
"proc-macro-error-attr",
"proc-macro2",
"quote",
"syn",
"version_check",
]
[[package]]
name = "proc-macro-error-attr"
version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869"
dependencies = [
"proc-macro2",
"quote",
"version_check",
]
[[package]]
name = "proc-macro2"
version = "1.0.37"
@ -441,6 +466,28 @@ dependencies = [
"winapi-util",
]
[[package]]
name = "test-case"
version = "2.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "196e8a70562e252cc51eaaaee3ecddc39803d9b7fd4a772b7c7dae7cdf42a859"
dependencies = [
"test-case-macros",
]
[[package]]
name = "test-case-macros"
version = "2.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8dd461f47ade621665c9f4e44b20449341769911c253275dc5cb03726cbb852c"
dependencies = [
"cfg-if",
"proc-macro-error",
"proc-macro2",
"quote",
"syn",
]
[[package]]
name = "thiserror"
version = "1.0.30"
@ -497,6 +544,12 @@ version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3"
[[package]]
name = "version_check"
version = "0.9.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
[[package]]
name = "wasm-bindgen"
version = "0.2.80"

View file

@ -32,3 +32,4 @@ package = "graphite-graphene"
[dev-dependencies]
env_logger = "0.8.4"
test-case = "2.1"

View file

@ -991,6 +991,7 @@ impl MessageHandler<DocumentMessage, (&InputPreprocessorMessageHandler, &FontCac
}
DeleteLayer { layer_path } => {
responses.push_front(DocumentOperation::DeleteLayer { path: layer_path.clone() }.into());
responses.push_front(ToolMessage::AbortCurrentTool.into());
responses.push_back(PropertiesPanelMessage::CheckSelectedWasDeleted { path: layer_path }.into());
}
DeleteSelectedLayers => {

View file

@ -199,7 +199,11 @@ impl<'a> Selected<'a> {
pub fn new(original_transforms: &'a mut OriginalTransforms, pivot: &'a mut DVec2, selected: &'a [&'a Vec<LayerId>], responses: &'a mut VecDeque<Message>, document: &'a Document) -> Self {
for path in selected {
if !original_transforms.contains_key(*path) {
original_transforms.insert(path.to_vec(), document.layer(path).unwrap().transform);
if let Ok(layer) = document.layer(path) {
original_transforms.insert(path.to_vec(), layer.transform);
} else {
log::warn!("Didn't find a layer for {:?}", path);
}
}
}
Self {

View file

@ -4,3 +4,32 @@ pub mod tool_message;
pub mod tool_message_handler;
pub mod tools;
pub mod vector_editor;
#[cfg(test)]
mod tool_crash_on_layer_delete_tests {
use crate::communication::set_uuid_seed;
use crate::misc::test_utils::EditorTestUtils;
use crate::viewport_tools::tool::ToolType;
use crate::{DocumentMessage, Editor};
use test_case::test_case;
#[test_case(ToolType::Pen ; "while using pen tool")]
#[test_case(ToolType::Freehand ; "while using freehand tool")]
#[test_case(ToolType::Spline ; "while using spline tool")]
#[test_case(ToolType::Line ; "while using line tool")]
#[test_case(ToolType::Rectangle ; "while using rectangle tool")]
#[test_case(ToolType::Ellipse ; "while using ellipse tool")]
#[test_case(ToolType::Shape ; "while using shape tool")]
#[test_case(ToolType::Path ; "while using path tool")]
fn should_not_crash_when_layer_is_deleted(tool: ToolType) {
set_uuid_seed(0);
let mut test_editor = Editor::new();
test_editor.select_tool(tool);
test_editor.lmb_mousedown(0.0, 0.0);
test_editor.move_mouse(100.0, 100.0);
test_editor.handle_message(DocumentMessage::DeleteSelectedLayers);
}
}