Simplify the implementation of the message buffering (#2123)

* Simplify the implementation of the message buffering

* Add assert to ensure list is empty

Author: 0HyperCube

editor/src/application.rs

@@ -16,7 +16,7 @@ impl Editor {
 	}
 
 	pub fn handle_message<T: Into<Message>>(&mut self, message: T) -> Vec<FrontendMessage> {
-		self.dispatcher.handle_message(message);
+		self.dispatcher.handle_message(message, true);
 
 		std::mem::take(&mut self.dispatcher.responses)
 	}

editor/src/dispatcher.rs

@@ -63,20 +63,33 @@ impl Dispatcher {
 		}
 	}
 
-	pub fn handle_message<T: Into<Message>>(&mut self, message: T) {
-		self.message_queues.push(VecDeque::from_iter([message.into()]));
-		while let Some(message) = self.message_queues.last_mut().and_then(VecDeque::pop_front) {
-			// Do not buffer the EndBuffer message
-			if !matches!(message, Message::EndBuffer(_)) {
-				if let Some(buffered_queue) = &mut self.buffered_queue {
-					// Store each message in a deque so that its children are added before future messages
-					let mut message_deque = VecDeque::new();
-					message_deque.push_back(message);
-					buffered_queue.push(message_deque);
-					continue;
-				}
+	/// Add a message to a queue so that it can be executed.
+	/// If `process_after_all_current` is set, all currently queued messages (including children) will be processed first.
+	/// If not set, it (and its children) will be processed as soon as possible.
+	pub fn schedule_execution(message_queues: &mut Vec<VecDeque<Message>>, process_after_all_current: bool, messages: impl IntoIterator<Item = Message>) {
+		match message_queues.first_mut() {
+			// If there are currently messages being processed and we are processing after them, add to the end of the first queue
+			Some(queue) if process_after_all_current => queue.extend(messages),
+			// In all other cases, make a new inner queue and add our message there
+			_ => message_queues.push(VecDeque::from_iter(messages)),
+		}
+	}
+
+	pub fn handle_message<T: Into<Message>>(&mut self, message: T, process_after_all_current: bool) {
+		let message = message.into();
+		// Add all aditional messages to the buffer if it exists (except from the end buffer message)
+		if !matches!(message, Message::EndBuffer(_)) {
+			if let Some(buffered_queue) = &mut self.buffered_queue {
+				Self::schedule_execution(buffered_queue, true, [message]);
+
+				return;
 			}
+		}
+
+		// If we are not maintaining the buffer, simply add to the current queue
+		Self::schedule_execution(&mut self.message_queues, process_after_all_current, [message]);
 
+		while let Some(message) = self.message_queues.last_mut().and_then(VecDeque::pop_front) {
 			// Skip processing of this message if it will be processed later (at the end of the shallowest level queue)
 			if SIDE_EFFECT_FREE_MESSAGES.contains(&message.to_discriminant()) {
 				let already_in_queue = self.message_queues.first().filter(|queue| queue.contains(&message)).is_some();
@@ -92,7 +105,7 @@ impl Dispatcher {
 				}
 			}
 
-			// Print the message at a verbosity level of `log`
+			// Print the message at a verbosity level of `info`
 			self.log_message(&message, &self.message_queues, self.message_handlers.debug_message_handler.message_logging_verbosity);
 
 			// Create a new queue for the child messages
@@ -104,9 +117,11 @@ impl Dispatcher {
 					self.buffered_queue = Some(std::mem::take(&mut self.message_queues));
 				}
 				Message::EndBuffer(render_metadata) => {
-					// The buffered vec is added before the metadata messages, because the end of the vec is processed first
+					// Assign the message queue to the currently buffered queue
 					if let Some(buffered_queue) = self.buffered_queue.take() {
-						self.message_queues.extend(buffered_queue);
+						self.cleanup_queues(false);
+						assert!(self.message_queues.is_empty(), "message queues are always empty when ending a buffer");
+						self.message_queues = buffered_queue;
 					};
 
 					let graphene_std::renderer::RenderMetadata {
@@ -115,14 +130,13 @@ impl Dispatcher {
 						clip_targets,
 					} = render_metadata;
 
-					let mut update_upstream_transform = VecDeque::new();
-					update_upstream_transform.push_back(DocumentMessage::UpdateUpstreamTransforms { upstream_transforms: footprints }.into());
-					self.message_queues.push(update_upstream_transform);
-
-					let mut update_click_targets = VecDeque::new();
-					update_click_targets.push_back(DocumentMessage::UpdateClickTargets { click_targets }.into());
-					update_click_targets.push_back(DocumentMessage::UpdateClipTargets { clip_targets }.into());
-					self.message_queues.push(update_click_targets);
+					// Run these update state messages immediately
+					let messages = [
+						DocumentMessage::UpdateUpstreamTransforms { upstream_transforms: footprints },
+						DocumentMessage::UpdateClickTargets { click_targets },
+						DocumentMessage::UpdateClipTargets { clip_targets },
+					];
+					Self::schedule_execution(&mut self.message_queues, false, messages.map(Message::from));
 				}
 				Message::NoOp => {}
 				Message::Init => {
@@ -141,7 +155,7 @@ impl Dispatcher {
 					});
 				}
 				Message::Batched(messages) => {
-					messages.iter().for_each(|message| self.handle_message(message.to_owned()));
+					messages.iter().for_each(|message| self.handle_message(message.to_owned(), false));
 				}
 				Message::Broadcast(message) => self.message_handlers.broadcast_message_handler.process_message(message, &mut queue, ()),
 				Message::Debug(message) => {

editor/src/messages/tool/tool_messages/select_tool.rs

@@ -912,6 +912,7 @@ impl Fsm for SelectToolFsmState {
 				responses.add(DocumentMessage::EndTransaction);
 
 				if !tool_data.has_dragged && input.keyboard.key(remove_from_selection) && tool_data.layer_selected_on_start.is_none() {
+					// When you click on the layer with remove from selection key (shift) pressed, we deselect all nodes that are children.
 					let quad = tool_data.selection_quad();
 					let intersection = document.intersect_quad_no_artboards(quad, input);
 
@@ -942,6 +943,7 @@ impl Fsm for SelectToolFsmState {
 						});
 					}
 				} else if let Some(selecting_layer) = tool_data.select_single_layer.take() {
+					// Previously, we may have had many layers selected. If the user clicks without dragging, we should just select the one layer that has been clicked.
 					if !tool_data.has_dragged {
 						if selecting_layer == LayerNodeIdentifier::ROOT_PARENT {
 							log::error!("selecting_layer should not be ROOT_PARENT");
@@ -1207,6 +1209,9 @@ fn drag_deepest_manipulation(responses: &mut VecDeque<Message>, selected: Vec<La
 	});
 }
 
+/// Called when you double click on the layer of the shallowest layer.
+/// If possible, the direct sibling of an old selected layer is the new selected layer.
+/// Otherwise, the first non-parent ancestor is selected.
 fn edit_layer_shallowest_manipulation(document: &DocumentMessageHandler, layer: LayerNodeIdentifier, responses: &mut VecDeque<Message>) {
 	let Some(new_selected) = layer.ancestors(document.metadata()).filter(not_artboard(document)).find(|ancestor| {
 		ancestor
@@ -1224,6 +1229,8 @@ fn edit_layer_shallowest_manipulation(document: &DocumentMessageHandler, layer:
 	responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![new_selected.to_node()] });
 }
 
+/// Called when a double click on a layer in deep select mode.
+/// If the layer is text, the text tool is selected.
 fn edit_layer_deepest_manipulation(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface, responses: &mut VecDeque<Message>) {
 	if is_layer_fed_by_node_of_name(layer, network_interface, "Text") {
 		responses.add_front(ToolMessage::ActivateTool { tool_type: ToolType::Text });