From e1aaee1918ae4d3f96c4d0f638ba08fe17b3286f Mon Sep 17 00:00:00 2001 From: mfish33 Date: Sat, 18 Dec 2021 12:01:29 -0500 Subject: [PATCH 01/11] removed all use of document indicies --- editor/src/communication/dispatcher.rs | 9 +- editor/src/communication/mod.rs | 12 ++ .../src/document/document_message_handler.rs | 135 ++++++++++-------- .../src/frontend/frontend_message_handler.rs | 13 +- editor/src/lib.rs | 1 + frontend/src/components/workspace/Panel.vue | 27 +--- .../src/components/workspace/Workspace.vue | 19 ++- frontend/src/utilities/documents.ts | 38 ++--- frontend/src/utilities/js-messages.ts | 16 ++- frontend/wasm/src/api.rs | 8 +- 10 files changed, 166 insertions(+), 112 deletions(-) diff --git a/editor/src/communication/dispatcher.rs b/editor/src/communication/dispatcher.rs index a729f9cd46..1969ba4e8d 100644 --- a/editor/src/communication/dispatcher.rs +++ b/editor/src/communication/dispatcher.rs @@ -17,11 +17,15 @@ pub struct Dispatcher { pub responses: Vec, } -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; } log_message(&message); diff --git a/editor/src/communication/mod.rs b/editor/src/communication/mod.rs index 47a881ac67..91f3a05c98 100644 --- a/editor/src/communication/mod.rs +++ b/editor/src/communication/mod.rs @@ -36,3 +36,15 @@ pub fn generate_uuid() -> u64 { } lock.as_mut().map(ChaCha20Rng::next_u64).unwrap() } + +/// Generates a random signed 32 bit integer. This is safe to be used inside a struct that will +/// be going to the frontend. Later this can be changed over to use a u64. wasm-bindgen only +/// supports u64 and i64 as function parameters. When used as struct fields JSON.parse gets called +/// and precision is often lost. +pub fn generate_uuid_js_safe() -> i32 { + let mut lock = RNG.lock(); + if lock.is_none() { + *lock = Some(ChaCha20Rng::seed_from_u64(0)); + } + lock.as_mut().map(|rand| rand.next_u32() as i32).unwrap() +} diff --git a/editor/src/document/document_message_handler.rs b/editor/src/document/document_message_handler.rs index 0fdf30a8a2..1ac95900a0 100644 --- a/editor/src/document/document_message_handler.rs +++ b/editor/src/document/document_message_handler.rs @@ -1,3 +1,4 @@ +use crate::frontend::frontend_message_handler::FrontendDocumentState; use crate::input::InputPreprocessor; use crate::message_prelude::*; use graphene::layers::Layer; @@ -18,8 +19,8 @@ pub enum DocumentsMessage { insert_index: isize, }, Paste, - SelectDocument(usize), - CloseDocument(usize), + SelectDocument(i32), + CloseDocument(i32), #[child] Document(DocumentMessage), CloseActiveDocumentWithConfirmation, @@ -35,22 +36,19 @@ pub enum DocumentsMessage { #[derive(Debug, Clone)] pub struct DocumentsMessageHandler { - documents: HashMap, - document_ids: Vec, - document_id_counter: u64, - active_document_index: usize, + documents: HashMap, + document_ids: Vec, + active_document_id: i32, copy_buffer: Vec, } 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 { @@ -77,21 +75,27 @@ impl DocumentsMessageHandler { } fn load_document(&mut self, new_document: DocumentMessageHandler, responses: &mut VecDeque) { - 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_js_safe(); + 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| FrontendDocumentState { + is_saved: doc.is_saved(), + id: *id, + name: doc.name.clone(), + }) + }) .collect::>(); 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() { @@ -103,18 +107,22 @@ impl DocumentsMessageHandler { pub fn ordered_document_iterator(&self) -> impl Iterator { 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: i32) -> usize { + self.document_ids.iter().position(|id| id == &document_id).expect("Active document is missing from document id's") + } } impl Default for DocumentsMessageHandler { fn default() -> Self { - let mut documents_map: HashMap = HashMap::with_capacity(1); - documents_map.insert(0, DocumentMessageHandler::default()); + let mut documents_map: HashMap = HashMap::with_capacity(1); + let starting_key = generate_uuid_js_safe(); + 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, } } } @@ -125,11 +133,9 @@ impl MessageHandler for DocumentsMessageHa use DocumentsMessage::*; match message { 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() { @@ -137,12 +143,7 @@ impl MessageHandler for DocumentsMessageHa } } CloseActiveDocumentWithConfirmation => { - responses.push_back( - FrontendMessage::DisplayConfirmationToCloseDocument { - document_index: self.active_document_index, - } - .into(), - ); + responses.push_back(FrontendMessage::DisplayConfirmationToCloseDocument { document_id: self.active_document_id }.into()); } CloseAllDocumentsWithConfirmation => { responses.push_back(FrontendMessage::DisplayConfirmationToCloseAllDocuments.into()); @@ -155,38 +156,44 @@ impl MessageHandler 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_js_safe(); + 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| FrontendDocumentState { + is_saved: doc.is_saved(), + id: *id, + name: doc.name.clone(), + }) + }) + .collect::>(); // 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() { @@ -218,17 +225,31 @@ impl MessageHandler 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| FrontendDocumentState { + is_saved: doc.is_saved(), + id: *id, + name: doc.name.clone(), + }) + }) + .collect::>(); 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(); diff --git a/editor/src/frontend/frontend_message_handler.rs b/editor/src/frontend/frontend_message_handler.rs index f8a6d2814a..1ac437868d 100644 --- a/editor/src/frontend/frontend_message_handler.rs +++ b/editor/src/frontend/frontend_message_handler.rs @@ -4,16 +4,23 @@ use crate::tool::tool_options::ToolOptions; use crate::Color; use serde::{Deserialize, Serialize}; +#[derive(PartialEq, Clone, Deserialize, Serialize, Debug)] +pub struct FrontendDocumentState { + pub is_saved: bool, + pub name: String, + pub id: i32, +} + #[impl_message(Message, Frontend)] #[derive(PartialEq, Clone, Deserialize, Serialize, Debug)] pub enum FrontendMessage { DisplayFolderTreeStructure { data_buffer: RawBuffer }, SetActiveTool { tool_name: String, tool_options: Option }, - SetActiveDocument { document_index: usize }, - UpdateOpenDocumentsList { open_documents: Vec<(String, bool)> }, + SetActiveDocument { document_id: i32 }, + UpdateOpenDocumentsList { open_documents: Vec }, DisplayError { title: String, description: String }, DisplayPanic { panic_info: String, title: String, description: String }, - DisplayConfirmationToCloseDocument { document_index: usize }, + DisplayConfirmationToCloseDocument { document_id: i32 }, DisplayConfirmationToCloseAllDocuments, UpdateLayer { data: LayerPanelEntry }, UpdateCanvas { document: String }, diff --git a/editor/src/lib.rs b/editor/src/lib.rs index ff5a8339d5..cca02c50e7 100644 --- a/editor/src/lib.rs +++ b/editor/src/lib.rs @@ -53,6 +53,7 @@ impl Default for Editor { pub mod message_prelude { pub use crate::communication::generate_uuid; + pub use crate::communication::generate_uuid_js_safe; pub use crate::communication::message::{AsMessage, Message, MessageDiscriminant}; pub use crate::communication::{ActionList, MessageHandler}; pub use crate::document::{DocumentMessage, DocumentMessageDiscriminant}; diff --git a/frontend/src/components/workspace/Panel.vue b/frontend/src/components/workspace/Panel.vue index a5a53860f1..3a1077dcf4 100644 --- a/frontend/src/components/workspace/Panel.vue +++ b/frontend/src/components/workspace/Panel.vue @@ -7,26 +7,11 @@ :class="{ active: tabIndex === tabActiveIndex }" v-for="(tabLabel, tabIndex) in tabLabels" :key="tabIndex" - @click.middle=" - (e) => { - e.stopPropagation(); - closeDocumentWithConfirmation(tabIndex); - } - " - @click="panelType === 'Document' && selectDocument(tabIndex)" + @click.middle="(e) => altClickAction && altClickAction(e, tabIndex)" + @click="(e) => clickAction && clickAction(e, tabIndex)" > {{ tabLabel }} - + @@ -169,8 +154,6 @@