Fix export dialogues showing

editor/src/messages/dialog/dialog_message.rs

@@ -8,8 +8,6 @@ pub enum DialogMessage {
 	ExportDialog(ExportDialogMessage),
 	#[child]
 	NewDocumentDialog(NewDocumentDialogMessage),
-	#[child]
-	PreferencesDialog(PreferencesDialogMessage),
 
 	// Messages
 	CloseAllDocumentsWithConfirmation,

editor/src/messages/dialog/dialog_message_handler.rs

@@ -1,4 +1,4 @@
-use super::simple_dialogs::{self, AboutGraphiteDialog, ComingSoonDialog, DemoArtworkDialog, LicensesDialog};
+use super::simple_dialogs::{self, *};
 use crate::messages::layout::utility_types::widget_prelude::*;
 use crate::messages::prelude::*;
 
@@ -13,7 +13,6 @@ pub struct DialogMessageContext<'a> {
 pub struct DialogMessageHandler {
 	export_dialog: ExportDialogMessageHandler,
 	new_document_dialog: NewDocumentDialogMessageHandler,
-	preferences_dialog: PreferencesDialogMessageHandler,
 }
 
 #[message_handler_data]
@@ -24,7 +23,6 @@ impl MessageHandler<DialogMessage, DialogMessageContext<'_>> for DialogMessageHa
 		match message {
 			DialogMessage::ExportDialog(message) => self.export_dialog.process_message(message, responses, ExportDialogMessageContext { portfolio }),
 			DialogMessage::NewDocumentDialog(message) => self.new_document_dialog.process_message(message, responses, ()),
-			DialogMessage::PreferencesDialog(message) => self.preferences_dialog.process_message(message, responses, PreferencesDialogMessageContext { preferences }),
 
 			DialogMessage::CloseAllDocumentsWithConfirmation => {
 				let dialog = simple_dialogs::CloseAllDocumentsDialog {
@@ -33,13 +31,13 @@ impl MessageHandler<DialogMessage, DialogMessageContext<'_>> for DialogMessageHa
 				dialog.send_dialog_to_frontend(responses);
 			}
 			DialogMessage::CloseDialogAndThen { followups } => {
+				// Since this message is "close dialog and then", the closing of the dialogue must happen first.
+				// This is because processing may spawn another dialogue (e.g. the export dialogue may produce an error dialogue).
+				responses.add(FrontendMessage::DisplayDialogDismiss);
+
 				for message in followups.into_iter() {
 					responses.add(message);
 				}
-
-				// This come after followups, so that the followups (which can cause the dialog to open) happen first, then we close it afterwards.
-				// If it comes before, the dialog reopens (and appears to not close at all).
-				responses.add(FrontendMessage::DisplayDialogDismiss);
 			}
 			DialogMessage::DisplayDialogError { title, description } => {
 				let dialog = simple_dialogs::ErrorDialog { title, description };
@@ -105,8 +103,8 @@ impl MessageHandler<DialogMessage, DialogMessageContext<'_>> for DialogMessageHa
 				self.new_document_dialog.send_dialog_to_frontend(responses);
 			}
 			DialogMessage::RequestPreferencesDialog => {
-				self.preferences_dialog = PreferencesDialogMessageHandler {};
+				let dialog = PreferencesDialog { preferences };
-				self.preferences_dialog.send_dialog_to_frontend(responses, preferences);
+				dialog.send_dialog_to_frontend(responses);
 			}
 		}
 	}

editor/src/messages/dialog/export_dialog/export_dialog_message_handler.rs

@@ -43,16 +43,22 @@ impl MessageHandler<ExportDialogMessage, ExportDialogMessageContext<'_>> for Exp
 			ExportDialogMessage::TransparentBackground(transparent_background) => self.transparent_background = transparent_background,
 			ExportDialogMessage::ExportBounds(export_area) => self.bounds = export_area,
 
-			ExportDialogMessage::Submit => responses.add_front(PortfolioMessage::SubmitDocumentExport {
+			ExportDialogMessage::Submit => {
-				file_name: portfolio.active_document().map(|document| document.name.clone()).unwrap_or_default(),
+				responses.add(FrontendMessage::DisplayDialogDismiss);
-				file_type: self.file_type,
+				responses.add(PortfolioMessage::SubmitDocumentExport {
-				scale_factor: self.scale_factor,
+					file_name: portfolio.active_document().map(|document| document.name.clone()).unwrap_or_default(),
-				bounds: self.bounds,
+					file_type: self.file_type,
-				transparent_background: self.file_type != FileType::Jpg && self.transparent_background,
+					scale_factor: self.scale_factor,
-			}),
+					bounds: self.bounds,
+					transparent_background: self.file_type != FileType::Jpg && self.transparent_background,
+				});
+			}
 		}
 
-		self.send_dialog_to_frontend(responses);
+		// Don't send the dialogue if the form was already submitted
+		if message != ExportDialogMessage::Submit {
+			self.send_dialog_to_frontend(responses);
+		}
 	}
 
 	advertise_actions! {ExportDialogUpdate;}
@@ -64,15 +70,7 @@ impl DialogLayoutHolder for ExportDialogMessageHandler {
 
 	fn layout_buttons(&self) -> Layout {
 		let widgets = vec![
-			TextButton::new("Export")
+			TextButton::new("Export").emphasized(true).on_update(|_| ExportDialogMessage::Submit.into()).widget_holder(),
-				.emphasized(true)
-				.on_update(|_| {
-					DialogMessage::CloseDialogAndThen {
-						followups: vec![ExportDialogMessage::Submit.into()],
-					}
-					.into()
-				})
-				.widget_holder(),
 			TextButton::new("Cancel").on_update(|_| FrontendMessage::DisplayDialogDismiss.into()).widget_holder(),
 		];
 

editor/src/messages/dialog/mod.rs

@@ -10,7 +10,6 @@ mod dialog_message_handler;
 
 pub mod export_dialog;
 pub mod new_document_dialog;
-pub mod preferences_dialog;
 pub mod simple_dialogs;
 
 #[doc(inline)]

editor/src/messages/dialog/new_document_dialog/new_document_dialog_message_handler.rs

@@ -14,12 +14,16 @@ pub struct NewDocumentDialogMessageHandler {
 #[message_handler_data]
 impl MessageHandler<NewDocumentDialogMessage, ()> for NewDocumentDialogMessageHandler {
 	fn process_message(&mut self, message: NewDocumentDialogMessage, responses: &mut VecDeque<Message>, _: ()) {
+		let mut dismiss = false;
+
 		match message {
 			NewDocumentDialogMessage::Name(name) => self.name = name,
 			NewDocumentDialogMessage::Infinite(infinite) => self.infinite = infinite,
 			NewDocumentDialogMessage::DimensionsX(x) => self.dimensions.x = x as u32,
 			NewDocumentDialogMessage::DimensionsY(y) => self.dimensions.y = y as u32,
 			NewDocumentDialogMessage::Submit => {
+				responses.add(FrontendMessage::DisplayDialogDismiss);
+				dismiss = true;
 				responses.add(PortfolioMessage::NewDocumentWithName { name: self.name.clone() });
 
 				let create_artboard = !self.infinite && self.dimensions.x > 0 && self.dimensions.y > 0;
@@ -41,7 +45,10 @@ impl MessageHandler<NewDocumentDialogMessage, ()> for NewDocumentDialogMessageHa
 			}
 		}
 
-		self.send_dialog_to_frontend(responses);
+		// Don't send the dialogue if the form was already dismissed (this would reopen it)
+		if !dismiss {
+			self.send_dialog_to_frontend(responses);
+		}
 	}
 
 	advertise_actions! {NewDocumentDialogUpdate;}
@@ -53,15 +60,7 @@ impl DialogLayoutHolder for NewDocumentDialogMessageHandler {
 
 	fn layout_buttons(&self) -> Layout {
 		let widgets = vec![
-			TextButton::new("OK")
+			TextButton::new("OK").emphasized(true).on_update(|_| NewDocumentDialogMessage::Submit.into()).widget_holder(),
-				.emphasized(true)
-				.on_update(|_| {
-					DialogMessage::CloseDialogAndThen {
-						followups: vec![NewDocumentDialogMessage::Submit.into()],
-					}
-					.into()
-				})
-				.widget_holder(),
 			TextButton::new("Cancel").on_update(|_| FrontendMessage::DisplayDialogDismiss.into()).widget_holder(),
 		];
 

editor/src/messages/dialog/preferences_dialog/mod.rs

@@ -1,7 +0,0 @@
-mod preferences_dialog_message;
-mod preferences_dialog_message_handler;
-
-#[doc(inline)]
-pub use preferences_dialog_message::{PreferencesDialogMessage, PreferencesDialogMessageDiscriminant};
-#[doc(inline)]
-pub use preferences_dialog_message_handler::{PreferencesDialogMessageContext, PreferencesDialogMessageHandler};

editor/src/messages/dialog/preferences_dialog/preferences_dialog_message.rs

@@ -1,7 +0,0 @@
-use crate::messages::prelude::*;
-
-#[impl_message(Message, DialogMessage, PreferencesDialog)]
-#[derive(Eq, PartialEq, Clone, Debug, serde::Serialize, serde::Deserialize)]
-pub enum PreferencesDialogMessage {
-	Confirm,
-}

editor/src/messages/dialog/simple_dialogs/mod.rs

@@ -5,6 +5,7 @@ mod coming_soon_dialog;
 mod demo_artwork_dialog;
 mod error_dialog;
 mod licenses_dialog;
+mod preferences_dialog;
 
 pub use about_graphite_dialog::AboutGraphiteDialog;
 pub use close_all_documents_dialog::CloseAllDocumentsDialog;
@@ -14,3 +15,4 @@ pub use demo_artwork_dialog::ARTWORK;
 pub use demo_artwork_dialog::DemoArtworkDialog;
 pub use error_dialog::ErrorDialog;
 pub use licenses_dialog::LicensesDialog;
+pub use preferences_dialog::PreferencesDialog;

editor/src/messages/dialog/simple_dialogs/preferences_dialog.rs

@@ -4,38 +4,28 @@ use crate::messages::portfolio::document::utility_types::wires::GraphWireStyle;
 use crate::messages::preferences::SelectionMode;
 use crate::messages::prelude::*;
 
-#[derive(ExtractField)]
+pub struct PreferencesDialog<'a> {
-pub struct PreferencesDialogMessageContext<'a> {
 	pub preferences: &'a PreferencesMessageHandler,
 }
 
-/// A dialog to allow users to customize Graphite editor options
+impl<'a> DialogLayoutHolder for PreferencesDialog<'a> {
-#[derive(Debug, Clone, Default, ExtractField)]
-pub struct PreferencesDialogMessageHandler {}
-
-#[message_handler_data]
-impl MessageHandler<PreferencesDialogMessage, PreferencesDialogMessageContext<'_>> for PreferencesDialogMessageHandler {
-	fn process_message(&mut self, message: PreferencesDialogMessage, responses: &mut VecDeque<Message>, context: PreferencesDialogMessageContext) {
-		let PreferencesDialogMessageContext { preferences } = context;
-
-		match message {
-			PreferencesDialogMessage::Confirm => {}
-		}
-
-		self.send_dialog_to_frontend(responses, preferences);
-	}
-
-	advertise_actions! {PreferencesDialogUpdate;}
-}
-
-// This doesn't actually implement the `DialogLayoutHolder` trait like the other dialog message handlers.
-// That's because we need to give `send_layout` the `preferences` argument, which is not part of the trait.
-// However, it's important to keep the methods in sync with those from the trait for consistency.
-impl PreferencesDialogMessageHandler {
 	const ICON: &'static str = "Settings";
 	const TITLE: &'static str = "Editor Preferences";
 
-	fn layout(&self, preferences: &PreferencesMessageHandler) -> Layout {
+	fn layout_buttons(&self) -> Layout {
+		let widgets = vec![
+			TextButton::new("OK").emphasized(true).on_update(|_| FrontendMessage::DisplayDialogDismiss.into()).widget_holder(),
+			TextButton::new("Reset to Defaults").on_update(|_| PreferencesMessage::ResetToDefaults.into()).widget_holder(),
+		];
+
+		Layout::WidgetLayout(WidgetLayout::new(vec![LayoutGroup::Row { widgets }]))
+	}
+}
+
+impl<'a> LayoutHolder for PreferencesDialog<'a> {
+	fn layout(&self) -> Layout {
+		let preferences = self.preferences;
+
 		// ==========
 		// NAVIGATION
 		// ==========
@@ -217,58 +207,6 @@ impl PreferencesDialogMessageHandler {
 			LayoutGroup::Row { widgets: vector_meshes },
 		]))
 	}
-
-	pub fn send_layout(&self, responses: &mut VecDeque<Message>, layout_target: LayoutTarget, preferences: &PreferencesMessageHandler) {
-		responses.add(LayoutMessage::SendLayout {
-			layout: self.layout(preferences),
-			layout_target,
-		})
-	}
-
-	fn layout_column_2(&self) -> Layout {
-		Layout::default()
-	}
-
-	fn send_layout_column_2(&self, responses: &mut VecDeque<Message>, layout_target: LayoutTarget) {
-		responses.add(LayoutMessage::SendLayout {
-			layout: self.layout_column_2(),
-			layout_target,
-		});
-	}
-
-	fn layout_buttons(&self) -> Layout {
-		let widgets = vec![
-			TextButton::new("OK")
-				.emphasized(true)
-				.on_update(|_| {
-					DialogMessage::CloseDialogAndThen {
-						followups: vec![PreferencesDialogMessage::Confirm.into()],
-					}
-					.into()
-				})
-				.widget_holder(),
-			TextButton::new("Reset to Defaults").on_update(|_| PreferencesMessage::ResetToDefaults.into()).widget_holder(),
-		];
-
-		Layout::WidgetLayout(WidgetLayout::new(vec![LayoutGroup::Row { widgets }]))
-	}
-
-	fn send_layout_buttons(&self, responses: &mut VecDeque<Message>, layout_target: LayoutTarget) {
-		responses.add(LayoutMessage::SendLayout {
-			layout: self.layout_buttons(),
-			layout_target,
-		});
-	}
-
-	pub fn send_dialog_to_frontend(&self, responses: &mut VecDeque<Message>, preferences: &PreferencesMessageHandler) {
-		self.send_layout(responses, LayoutTarget::DialogColumn1, preferences);
-		self.send_layout_column_2(responses, LayoutTarget::DialogColumn2);
-		self.send_layout_buttons(responses, LayoutTarget::DialogButtons);
-		responses.add(FrontendMessage::DisplayDialog {
-			icon: Self::ICON.into(),
-			title: Self::TITLE.into(),
-		});
-	}
 }
 
 /// Maps display values (1-100) to actual zoom rates.

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -662,7 +662,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 					return;
 				}
 
-				let layers_to_move = self.network_interface.shallowest_unique_layers_sorted(&self.selection_network_path);
+				let layers_to_move = self.network_interface.shallowest_unique_layers(&self.selection_network_path).collect::<Vec<_>>();
 				// Offset the index for layers to move that are below another layer to move. For example when moving 1 and 2 between 3 and 4, 2 should be inserted at the same index as 1 since 1 is moved first.
 				let layers_to_move_with_insert_offset = layers_to_move
 					.iter()
@@ -717,7 +717,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 			}
 			DocumentMessage::MoveSelectedLayersToGroup { parent } => {
 				// Group all shallowest unique selected layers in order
-				let all_layers_to_group_sorted = self.network_interface.shallowest_unique_layers_sorted(&self.selection_network_path);
+				let all_layers_to_group_sorted = self.network_interface.shallowest_unique_layers(&self.selection_network_path).collect::<Vec<_>>();
 
 				for layer_to_group in all_layers_to_group_sorted.into_iter().rev() {
 					responses.add(NodeGraphMessage::MoveLayerToStack {

editor/src/messages/portfolio/document/utility_types/network_interface.rs

@@ -1313,46 +1313,18 @@ impl NodeNetworkInterface {
 			.reduce(Quad::combine_bounds)
 	}
 
-	/// Layers excluding ones that are children of other layers in the list.
+	/// Layers excluding ones that are children of other layers in the list in layer tree order.
 	// TODO: Cache this
-	pub fn shallowest_unique_layers(&self, network_path: &[NodeId]) -> impl Iterator<Item = LayerNodeIdentifier> + use<> {
+	// Now allocation free!
-		let mut sorted_layers = if let Some(selected_nodes) = self.selected_nodes_in_nested_network(network_path) {
+	pub fn shallowest_unique_layers(&self, network_path: &[NodeId]) -> ShallowestSelectionIter<'_> {
-			selected_nodes
+		// Avoids the clone and filtering from from the selected_nodes_in_nested_network.
-				.selected_layers(self.document_metadata())
+		let metadata = self.network_metadata(network_path);
-				.map(|layer| {
+		let selection = metadata.and_then(|metadata| metadata.persistent_metadata.selection_undo_history.back());
-					let mut layer_path = layer.ancestors(&self.document_metadata).collect::<Vec<_>>();
+		ShallowestSelectionIter {
-					layer_path.reverse();
+			selection: selection.map_or([].as_slice(), |selection| selection.0.as_slice()),
-					layer_path
+			next: Some(LayerNodeIdentifier::ROOT_PARENT),
-				})
+			metadata: self.document_metadata(),
-				.collect::<Vec<_>>()
-		} else {
-			log::error!("Could not get selected nodes in shallowest_unique_layers");
-			Vec::new()
-		};
-
-		// Sorting here creates groups of similar UUID paths
-		sorted_layers.sort();
-		sorted_layers.dedup_by(|a, b| a.starts_with(b));
-		sorted_layers.into_iter().map(|mut path| {
-			let layer = path.pop().expect("Path should not be empty");
-			assert!(
-				layer != LayerNodeIdentifier::ROOT_PARENT,
-				"The root parent cannot be selected, so it cannot be a shallowest selected layer"
-			);
-			layer
-		})
-	}
-
-	pub fn shallowest_unique_layers_sorted(&self, network_path: &[NodeId]) -> Vec<LayerNodeIdentifier> {
-		let all_layers_to_group = self.shallowest_unique_layers(network_path).collect::<Vec<_>>();
-		// Ensure nodes are grouped in the correct order
-		let mut all_layers_to_group_sorted = Vec::new();
-		for descendant in LayerNodeIdentifier::ROOT_PARENT.descendants(self.document_metadata()) {
-			if all_layers_to_group.contains(&descendant) {
-				all_layers_to_group_sorted.push(descendant);
-			};
 		}
-		all_layers_to_group_sorted
 	}
 
 	/// Ancestor that is shared by all layers and that is deepest (more nested). Default may be the root. Skips selected non-folder, non-artboard layers
@@ -7021,3 +6993,32 @@ pub enum TransactionStatus {
 	#[default]
 	Finished,
 }
+
+/// Iterate through the shallowest selected layers without allocating
+#[derive(Clone)]
+pub struct ShallowestSelectionIter<'a> {
+	next: Option<LayerNodeIdentifier>,
+	selection: &'a [NodeId], // TODO: should be HashSet to avoid duplicates.
+	metadata: &'a DocumentMetadata,
+}
+
+impl Iterator for ShallowestSelectionIter<'_> {
+	type Item = LayerNodeIdentifier;
+
+	fn next(&mut self) -> Option<Self::Item> {
+		while let Some(layer_node) = self.next.take() {
+			// Ignoring the children of this layer, find the next layer that would be displayed in the tree
+			let below_in_tree = || layer_node.ancestors(self.metadata).find_map(|ancestor| ancestor.next_sibling(self.metadata));
+
+			// If the current layer is selected, return it.
+			if layer_node != LayerNodeIdentifier::ROOT_PARENT && self.selection.contains(&layer_node.to_node()) {
+				self.next = below_in_tree(); // Go straight to below and don't look at children
+				return Some(layer_node);
+			}
+			// Go to children or otherwise go to below in the tree
+			self.next = layer_node.first_child(self.metadata).or_else(below_in_tree);
+		}
+
+		None
+	}
+}

editor/src/messages/prelude.rs

@@ -9,7 +9,6 @@ pub use crate::messages::debug::{DebugMessage, DebugMessageDiscriminant, DebugMe
 pub use crate::messages::defer::{DeferMessage, DeferMessageDiscriminant, DeferMessageHandler};
 pub use crate::messages::dialog::export_dialog::{ExportDialogMessage, ExportDialogMessageContext, ExportDialogMessageDiscriminant, ExportDialogMessageHandler};
 pub use crate::messages::dialog::new_document_dialog::{NewDocumentDialogMessage, NewDocumentDialogMessageDiscriminant, NewDocumentDialogMessageHandler};
-pub use crate::messages::dialog::preferences_dialog::{PreferencesDialogMessage, PreferencesDialogMessageContext, PreferencesDialogMessageDiscriminant, PreferencesDialogMessageHandler};
 pub use crate::messages::dialog::{DialogMessage, DialogMessageContext, DialogMessageDiscriminant, DialogMessageHandler};
 pub use crate::messages::frontend::{FrontendMessage, FrontendMessageDiscriminant};
 pub use crate::messages::globals::{GlobalsMessage, GlobalsMessageDiscriminant, GlobalsMessageHandler};