Remove all use of document indices (#406)

* removed all use of document indicies

* -add u64 support for wasm bridge

* fixed rust formating

* Cleaned up FrontendDocumentState in js-messages

* Tiny tweaks from code review

* - moved more of closeDocumentWithConfirmation to rust
- updated serde_wasm_bindgen to add feature flag

* changed to upsteam version of serde_wasm_bindgen

* cargo fmt

* -fix event propigation on delete
- Js message change class extention to typedef

* changed another typedef

* cargo fmt

Co-authored-by: Keavon Chambers <[email protected]>

Author: mfish33

Cargo.lock

@@ -17,11 +17,15 @@ pub struct Dispatcher {
 	pub responses: Vec<FrontendMessage>,
 }
 
-const GROUP_MESSAGES: &[MessageDiscriminant] = &[
+// For optimization, these are messages guaranteed to be redundant when repeated
+// The last occurrence of the message in the message queue is sufficient to ensure correctness
+// In addition, these messages do not change any state in the backend (aside from caches)
+const SIDE_EFFECT_FREE_MESSAGES: &[MessageDiscriminant] = &[
 	MessageDiscriminant::Documents(DocumentsMessageDiscriminant::Document(DocumentMessageDiscriminant::RenderDocument)),
 	MessageDiscriminant::Documents(DocumentsMessageDiscriminant::Document(DocumentMessageDiscriminant::FolderChanged)),
 	MessageDiscriminant::Frontend(FrontendMessageDiscriminant::UpdateLayer),
 	MessageDiscriminant::Frontend(FrontendMessageDiscriminant::DisplayFolderTreeStructure),
+	MessageDiscriminant::Frontend(FrontendMessageDiscriminant::UpdateOpenDocumentsList),
 	MessageDiscriminant::Tool(ToolMessageDiscriminant::SelectedLayersChanged),
 ];
 
@@ -31,7 +35,8 @@ impl Dispatcher {
 
 		use Message::*;
 		while let Some(message) = self.messages.pop_front() {
-			if GROUP_MESSAGES.contains(&message.to_discriminant()) && self.messages.contains(&message) {
+			// Skip processing of this message if it will be processed later
+			if SIDE_EFFECT_FREE_MESSAGES.contains(&message.to_discriminant()) && self.messages.contains(&message) {
 				continue;
 			}
 			self.log_message(&message);

editor/src/communication/dispatcher.rs

@@ -1,3 +1,4 @@
+use crate::frontend::frontend_message_handler::FrontendDocumentDetails;
 use crate::input::InputPreprocessor;
 use crate::message_prelude::*;
 use graphene::layers::Layer;
@@ -18,11 +19,12 @@ pub enum DocumentsMessage {
 		insert_index: isize,
 	},
 	Paste,
-	SelectDocument(usize),
-	CloseDocument(usize),
+	SelectDocument(u64),
+	CloseDocument(u64),
 	#[child]
 	Document(DocumentMessage),
 	CloseActiveDocumentWithConfirmation,
+	CloseDocumentWithConfirmation(u64),
 	CloseAllDocumentsWithConfirmation,
 	CloseAllDocuments,
 	RequestAboutGraphiteDialog,
@@ -38,20 +40,17 @@ pub enum DocumentsMessage {
 pub struct DocumentsMessageHandler {
 	documents: HashMap<u64, DocumentMessageHandler>,
 	document_ids: Vec<u64>,
-	document_id_counter: u64,
-	active_document_index: usize,
+	active_document_id: u64,
 	copy_buffer: Vec<Layer>,
 }
 
 impl DocumentsMessageHandler {
 	pub fn active_document(&self) -> &DocumentMessageHandler {
-		let id = self.document_ids[self.active_document_index];
-		self.documents.get(&id).unwrap()
+		self.documents.get(&self.active_document_id).unwrap()
 	}
 
 	pub fn active_document_mut(&mut self) -> &mut DocumentMessageHandler {
-		let id = self.document_ids[self.active_document_index];
-		self.documents.get_mut(&id).unwrap()
+		self.documents.get_mut(&self.active_document_id).unwrap()
 	}
 
 	fn generate_new_document_name(&self) -> String {
@@ -78,21 +77,27 @@ impl DocumentsMessageHandler {
 	}
 
 	fn load_document(&mut self, new_document: DocumentMessageHandler, responses: &mut VecDeque<Message>) {
-		self.document_id_counter += 1;
-		self.active_document_index = self.document_ids.len();
-		self.document_ids.push(self.document_id_counter);
-		self.documents.insert(self.document_id_counter, new_document);
+		let new_id = generate_uuid();
+		self.active_document_id = new_id;
+		self.document_ids.push(new_id);
+		self.documents.insert(new_id, new_document);
 
 		// Send the new list of document tab names
 		let open_documents = self
 			.document_ids
 			.iter()
-			.filter_map(|id| self.documents.get(&id).map(|doc| (doc.name.clone(), doc.is_saved())))
+			.filter_map(|id| {
+				self.documents.get(&id).map(|doc| FrontendDocumentDetails {
+					is_saved: doc.is_saved(),
+					id: *id,
+					name: doc.name.clone(),
+				})
+			})
 			.collect::<Vec<_>>();
 
 		responses.push_back(FrontendMessage::UpdateOpenDocumentsList { open_documents }.into());
 
-		responses.push_back(DocumentsMessage::SelectDocument(self.active_document_index).into());
+		responses.push_back(DocumentsMessage::SelectDocument(self.active_document_id).into());
 		responses.push_back(DocumentMessage::RenderDocument.into());
 		responses.push_back(DocumentMessage::DocumentStructureChanged.into());
 		for layer in self.active_document().layer_data.keys() {
@@ -104,18 +109,23 @@ impl DocumentsMessageHandler {
 	pub fn ordered_document_iterator(&self) -> impl Iterator<Item = &DocumentMessageHandler> {
 		self.document_ids.iter().map(|id| self.documents.get(id).expect("document id was not found in the document hashmap"))
 	}
+
+	fn document_index(&self, document_id: u64) -> usize {
+		self.document_ids.iter().position(|id| id == &document_id).expect("Active document is missing from document ids")
+	}
 }
 
 impl Default for DocumentsMessageHandler {
 	fn default() -> Self {
 		let mut documents_map: HashMap<u64, DocumentMessageHandler> = HashMap::with_capacity(1);
-		documents_map.insert(0, DocumentMessageHandler::default());
+		let starting_key = generate_uuid();
+		documents_map.insert(starting_key, DocumentMessageHandler::default());
+
 		Self {
 			documents: documents_map,
-			document_ids: vec![0],
+			document_ids: vec![starting_key],
 			copy_buffer: vec![],
-			active_document_index: 0,
-			document_id_counter: 0,
+			active_document_id: starting_key,
 		}
 	}
 }
@@ -129,24 +139,27 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
 				responses.push_back(FrontendMessage::DisplayAboutGraphiteDialog.into());
 			}
 			Document(message) => self.active_document_mut().process_action(message, ipp, responses),
-			SelectDocument(index) => {
-				// NOTE: Potentially this will break if we ever exceed 56 bit values due to how the message parsing system works.
-				assert!(index < self.documents.len(), "Tried to select a document that was not initialized");
-				self.active_document_index = index;
-				responses.push_back(FrontendMessage::SetActiveDocument { document_index: index }.into());
+			SelectDocument(id) => {
+				self.active_document_id = id;
+				responses.push_back(FrontendMessage::SetActiveDocument { document_id: id }.into());
 				responses.push_back(RenderDocument.into());
 				responses.push_back(DocumentMessage::DocumentStructureChanged.into());
 				for layer in self.active_document().layer_data.keys() {
 					responses.push_back(DocumentMessage::LayerChanged(layer.clone()).into());
 				}
 			}
 			CloseActiveDocumentWithConfirmation => {
-				responses.push_back(
-					FrontendMessage::DisplayConfirmationToCloseDocument {
-						document_index: self.active_document_index,
-					}
-					.into(),
-				);
+				responses.push_back(DocumentsMessage::CloseDocumentWithConfirmation(self.active_document_id).into());
+			}
+			CloseDocumentWithConfirmation(id) => {
+				let target_document = self.documents.get(&id).unwrap();
+				if target_document.is_saved() {
+					responses.push_back(DocumentsMessage::CloseDocument(id).into());
+				} else {
+					responses.push_back(FrontendMessage::DisplayConfirmationToCloseDocument { document_id: id }.into());
+					// Select the document being closed
+					responses.push_back(DocumentsMessage::SelectDocument(id).into());
+				}
 			}
 			CloseAllDocumentsWithConfirmation => {
 				responses.push_back(FrontendMessage::DisplayConfirmationToCloseAllDocuments.into());
@@ -159,38 +172,44 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
 				// Create a new blank document
 				responses.push_back(NewDocument.into());
 			}
-			CloseDocument(index) => {
-				assert!(index < self.documents.len(), "Tried to close a document that was not initialized");
-				// Get the ID based on the current collection of the documents.
-				let id = self.document_ids[index];
-				// Map the ID to an index and remove the document
+			CloseDocument(id) => {
+				let document_index = self.document_index(id);
 				self.documents.remove(&id);
-				self.document_ids.remove(index);
+				self.document_ids.remove(document_index);
 
 				// Last tab was closed, so create a new blank tab
 				if self.document_ids.is_empty() {
-					self.document_id_counter += 1;
-					self.document_ids.push(self.document_id_counter);
-					self.documents.insert(self.document_id_counter, DocumentMessageHandler::default());
+					let new_id = generate_uuid();
+					self.document_ids.push(new_id);
+					self.documents.insert(new_id, DocumentMessageHandler::default());
 				}
 
-				self.active_document_index = if self.active_document_index >= self.document_ids.len() {
-					self.document_ids.len() - 1
+				self.active_document_id = if id != self.active_document_id {
+					// If we are not closing the active document, stay on it
+					self.active_document_id
+				} else if document_index >= self.document_ids.len() {
+					// If we closed the last document take the one previous (same as last)
+					*self.document_ids.last().unwrap()
 				} else {
-					index
+					// Move to the next tab
+					self.document_ids[document_index]
 				};
 
 				// Send the new list of document tab names
-				let open_documents = self.ordered_document_iterator().map(|doc| (doc.name.clone(), doc.is_saved())).collect();
-
+				let open_documents = self
+					.document_ids
+					.iter()
+					.filter_map(|id| {
+						self.documents.get(&id).map(|doc| FrontendDocumentDetails {
+							is_saved: doc.is_saved(),
+							id: *id,
+							name: doc.name.clone(),
+						})
+					})
+					.collect::<Vec<_>>();
 				// Update the list of new documents on the front end, active tab, and ensure that document renders
 				responses.push_back(FrontendMessage::UpdateOpenDocumentsList { open_documents }.into());
-				responses.push_back(
-					FrontendMessage::SetActiveDocument {
-						document_index: self.active_document_index,
-					}
-					.into(),
-				);
+				responses.push_back(FrontendMessage::SetActiveDocument { document_id: self.active_document_id }.into());
 				responses.push_back(RenderDocument.into());
 				responses.push_back(DocumentMessage::DocumentStructureChanged.into());
 				for layer in self.active_document().layer_data.keys() {
@@ -222,17 +241,31 @@ impl MessageHandler<DocumentsMessage, &InputPreprocessor> for DocumentsMessageHa
 			}
 			UpdateOpenDocumentsList => {
 				// Send the list of document tab names
-				let open_documents = self.ordered_document_iterator().map(|doc| (doc.name.clone(), doc.is_saved())).collect();
+				let open_documents = self
+					.document_ids
+					.iter()
+					.filter_map(|id| {
+						self.documents.get(&id).map(|doc| FrontendDocumentDetails {
+							is_saved: doc.is_saved(),
+							id: *id,
+							name: doc.name.clone(),
+						})
+					})
+					.collect::<Vec<_>>();
 				responses.push_back(FrontendMessage::UpdateOpenDocumentsList { open_documents }.into());
 			}
 			NextDocument => {
-				let next = (self.active_document_index + 1) % self.document_ids.len();
-				responses.push_back(SelectDocument(next).into());
+				let current_index = self.document_index(self.active_document_id);
+				let next_index = (current_index + 1) % self.document_ids.len();
+				let next_id = self.document_ids[next_index];
+				responses.push_back(SelectDocument(next_id).into());
 			}
 			PrevDocument => {
 				let len = self.document_ids.len();
-				let prev = (self.active_document_index + len - 1) % len;
-				responses.push_back(SelectDocument(prev).into());
+				let current_index = self.document_index(self.active_document_id);
+				let prev_index = (current_index + len - 1) % len;
+				let prev_id = self.document_ids[prev_index];
+				responses.push_back(SelectDocument(prev_id).into());
 			}
 			Copy => {
 				let paths = self.active_document().selected_layers_sorted();

editor/src/document/document_message_handler.rs

@@ -2,10 +2,7 @@ use crate::consts::VIEWPORT_ROTATE_SNAP_INTERVAL;
 use glam::{DAffine2, DVec2};
 use graphene::layers::{style::ViewMode, BlendMode, Layer, LayerData as DocumentLayerData, LayerDataType};
 use graphene::LayerId;
-use serde::{
-	ser::{SerializeSeq, SerializeStruct},
-	Deserialize, Serialize,
-};
+use serde::{ser::SerializeStruct, Deserialize, Serialize};
 use std::collections::HashMap;
 use std::fmt;
 
@@ -84,39 +81,11 @@ pub fn layer_panel_entry(layer_data: &LayerData, transform: DAffine2, layer: &La
 		opacity: layer.opacity,
 		layer_type: (&layer.data).into(),
 		layer_data: *layer_data,
-		path: path.into(),
+		path,
 		thumbnail,
 	}
 }
 
-#[derive(Debug, Clone, Deserialize, PartialEq)]
-pub struct Path(Vec<LayerId>);
-
-impl From<Vec<LayerId>> for Path {
-	fn from(iter: Vec<LayerId>) -> Self {
-		Self(iter)
-	}
-}
-impl Serialize for Path {
-	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-	where
-		S: serde::Serializer,
-	{
-		let mut seq = serializer.serialize_seq(Some(self.0.len()))?;
-		for e in self.0.iter() {
-			#[cfg(target_arch = "wasm32")]
-			{
-				// LayerIds are sent as (u32, u32) because json does not support u64s
-				let id = ((e >> 32) as u32, (e << 32 >> 32) as u32);
-				seq.serialize_element(&id)?;
-			}
-			#[cfg(not(target_arch = "wasm32"))]
-			seq.serialize_element(e)?;
-		}
-		seq.end()
-	}
-}
-
 #[derive(Debug, Clone, Deserialize, PartialEq)]
 pub struct RawBuffer(Vec<u8>);
 
@@ -152,7 +121,7 @@ pub struct LayerPanelEntry {
 	pub opacity: f64,
 	pub layer_type: LayerType,
 	pub layer_data: LayerData,
-	pub path: crate::document::layer_panel::Path,
+	pub path: Vec<LayerId>,
 	pub thumbnail: String,
 }