From e9510c5fee34df2c5242308bfe4160813e03a3ce Mon Sep 17 00:00:00 2001 From: Elbert Ronnie <103196773+elbertronnie@users.noreply.github.com> Date: Tue, 26 Mar 2024 01:50:30 +0530 Subject: [PATCH] Fix Select tool's scale nudging with multi-layer selection (#1699) * Fix scale nudging for multiple selection * Use message discriminant for filtering where possible * Remove unnecessary parameter from `selected_bounds_document_space` * Fix the error `target.closest is not a function` * Minor cleanup --------- Co-authored-by: Keavon Chambers --- editor/src/dispatcher.rs | 13 ++++-- .../document/document_message_handler.rs | 45 +++++++++++-------- .../utility_types/document_metadata.rs | 4 +- .../tool/tool_messages/select_tool.rs | 2 +- editor/src/node_graph_executor.rs | 4 +- .../src/components/layout/FloatingMenu.svelte | 2 +- 6 files changed, 41 insertions(+), 29 deletions(-) diff --git a/editor/src/dispatcher.rs b/editor/src/dispatcher.rs index e100191d..69b77180 100644 --- a/editor/src/dispatcher.rs +++ b/editor/src/dispatcher.rs @@ -40,7 +40,12 @@ const SIDE_EFFECT_FREE_MESSAGES: &[MessageDiscriminant] = &[ MessageDiscriminant::Frontend(FrontendMessageDiscriminant::UpdateDocumentLayerStructure), MessageDiscriminant::Frontend(FrontendMessageDiscriminant::TriggerFontLoad), ]; -const DEBUG_MESSAGE_BLOCK_LIST: &[&str] = &["AnimationFrame", "PointerMove", "PointerOutsideViewport", "FrameTimeAdvance"]; +const DEBUG_MESSAGE_BLOCK_LIST: &[MessageDiscriminant] = &[ + MessageDiscriminant::Broadcast(BroadcastMessageDiscriminant::TriggerEvent(BroadcastEventDiscriminant::AnimationFrame)), + MessageDiscriminant::InputPreprocessor(InputPreprocessorMessageDiscriminant::FrameTimeAdvance), +]; +// TODO: Find a way to combine these with the list above. We use strings for now since these are the standard variant names used by multiple messages. But having these also type-checked would be best. +const DEBUG_MESSAGE_ENDING_BLOCK_LIST: &[&str] = &["PointerMove", "PointerOutsideViewport"]; impl Dispatcher { pub fn new() -> Self { @@ -223,9 +228,11 @@ impl Dispatcher { /// Logs a message that is about to be executed, /// either as a tree with a discriminant or the entire payload (depending on settings) fn log_message(&self, message: &Message, queues: &[VecDeque], message_logging_verbosity: MessageLoggingVerbosity) { - let message_name = MessageDiscriminant::from(message).local_name(); + let discriminant = MessageDiscriminant::from(message); + let is_blocked = DEBUG_MESSAGE_BLOCK_LIST.iter().any(|&blocked_discriminant| discriminant == blocked_discriminant) + || DEBUG_MESSAGE_ENDING_BLOCK_LIST.iter().any(|blocked_name| discriminant.local_name().ends_with(blocked_name)); - if !DEBUG_MESSAGE_BLOCK_LIST.iter().any(|blocked_name| message_name.ends_with(blocked_name)) { + if !is_blocked { match message_logging_verbosity { MessageLoggingVerbosity::Off => {} MessageLoggingVerbosity::Names => { diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index b1c5e849..b6a29247 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -457,18 +457,23 @@ impl MessageHandler> for DocumentMessag let opposite_corner = ipp.keyboard.key(resize_opposite_corner); let delta = DVec2::new(delta_x, delta_y); - for layer in self.selected_nodes.selected_layers(self.metadata()) { + match ipp.keyboard.key(resize) { // Nudge translation - if !ipp.keyboard.key(resize) { - responses.add(GraphOperationMessage::TransformChange { - layer, - transform: DAffine2::from_translation(delta), - transform_in: TransformIn::Local, - skip_rerender: false, - }); + false => { + for layer in self.selected_nodes.selected_layers(self.metadata()) { + responses.add(GraphOperationMessage::TransformChange { + layer, + transform: DAffine2::from_translation(delta), + transform_in: TransformIn::Local, + skip_rerender: false, + }); + } } // Nudge resize - else if let Some([existing_top_left, existing_bottom_right]) = self.metadata.bounding_box_document(layer) { + true => { + let selected_bounding_box = self.metadata().selected_bounds_document_space(false, &self.selected_nodes); + let Some([existing_top_left, existing_bottom_right]) = selected_bounding_box else { return }; + let size = existing_bottom_right - existing_top_left; let new_size = size + if opposite_corner { -delta } else { delta }; let enlargement_factor = new_size / size; @@ -486,16 +491,18 @@ impl MessageHandler> for DocumentMessag let pivot = DAffine2::from_translation(pivot); let transformation = pivot * scale * pivot.inverse(); - let to = self.metadata().document_to_viewport.inverse() * self.metadata().downstream_transform_to_viewport(layer); - let original_transform = self.metadata().upstream_transform(layer.to_node()); - let new = to.inverse() * transformation * to * original_transform; - responses.add(GraphOperationMessage::TransformSet { - layer, - transform: new, - transform_in: TransformIn::Local, - skip_rerender: false, - }); - }; + for layer in self.selected_nodes.selected_layers(self.metadata()) { + let to = self.metadata().document_to_viewport.inverse() * self.metadata().downstream_transform_to_viewport(layer); + let original_transform = self.metadata().upstream_transform(layer.to_node()); + let new = to.inverse() * transformation * to * original_transform; + responses.add(GraphOperationMessage::TransformSet { + layer, + transform: new, + transform_in: TransformIn::Local, + skip_rerender: false, + }); + } + } } } DocumentMessage::PasteImage { image, mouse } => { diff --git a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs index 55d0489f..d148de1e 100644 --- a/editor/src/messages/portfolio/document/utility_types/document_metadata.rs +++ b/editor/src/messages/portfolio/document/utility_types/document_metadata.rs @@ -279,9 +279,9 @@ impl DocumentMetadata { } /// Calculates the selected layer bounds in document space - pub fn selected_bounds_document_space(&self, include_artboards: bool, metadata: &DocumentMetadata, selected_nodes: &SelectedNodes) -> Option<[DVec2; 2]> { + pub fn selected_bounds_document_space(&self, include_artboards: bool, selected_nodes: &SelectedNodes) -> Option<[DVec2; 2]> { selected_nodes - .selected_layers(metadata) + .selected_layers(self) .filter(|&layer| include_artboards || !self.is_artboard(layer)) .filter_map(|layer| self.bounding_box_document(layer)) .reduce(Quad::combine_bounds) diff --git a/editor/src/messages/tool/tool_messages/select_tool.rs b/editor/src/messages/tool/tool_messages/select_tool.rs index eb3e81af..728a9205 100644 --- a/editor/src/messages/tool/tool_messages/select_tool.rs +++ b/editor/src/messages/tool/tool_messages/select_tool.rs @@ -1031,7 +1031,7 @@ impl Fsm for SelectToolFsmState { HintInfo::arrow_keys("Nudge Selected"), HintInfo::keys([Key::Shift], "10x").prepend_plus(), HintInfo::keys([Key::Alt], "Resize Corner").prepend_plus(), - HintInfo::keys([Key::Control], "Opp. Corner").prepend_plus(), + HintInfo::keys([Key::Control], "Other Corner").prepend_plus(), ]), HintGroup(vec![ HintInfo::keys_and_mouse([Key::Alt], MouseMotion::LmbDrag, "Move Duplicate"), diff --git a/editor/src/node_graph_executor.rs b/editor/src/node_graph_executor.rs index 358da3d3..d6a9cf52 100644 --- a/editor/src/node_graph_executor.rs +++ b/editor/src/node_graph_executor.rs @@ -474,9 +474,7 @@ impl NodeGraphExecutor { // Calculate the bounding box of the region to be exported let bounds = match export_config.bounds { ExportBounds::AllArtwork => document.metadata().document_bounds_document_space(!export_config.transparent_background), - ExportBounds::Selection => document - .metadata() - .selected_bounds_document_space(!export_config.transparent_background, document.metadata(), &document.selected_nodes), + ExportBounds::Selection => document.metadata().selected_bounds_document_space(!export_config.transparent_background, &document.selected_nodes), ExportBounds::Artboard(id) => document.metadata().bounding_box_document(id), } .ok_or_else(|| "No bounding box".to_string())?; diff --git a/frontend/src/components/layout/FloatingMenu.svelte b/frontend/src/components/layout/FloatingMenu.svelte index dc4cb4a1..9dd4f9e5 100644 --- a/frontend/src/components/layout/FloatingMenu.svelte +++ b/frontend/src/components/layout/FloatingMenu.svelte @@ -286,7 +286,7 @@ // Assumes the spawner is a sibling of this FloatingMenu component const ownSpawner: HTMLElement | undefined = self?.parentElement?.querySelector(":scope > [data-floating-menu-spawner]") || undefined; // Get the spawner element containing whatever element the user is hovering over now, if there is one - const targetSpawner: HTMLElement | undefined = target?.closest("[data-floating-menu-spawner]") || undefined; + const targetSpawner: HTMLElement | undefined = target?.closest?.("[data-floating-menu-spawner]") || undefined; // HOVER TRANSFER // Transfer from this open floating menu to a sibling floating menu if the pointer hovers to a valid neighboring floating menu spawner