Code review, refactoring, and test fixes

Author: Keavon

desktop/src/app.rs

@@ -47,12 +47,11 @@ pub(crate) struct App {
 }
 
 impl App {
-	pub(crate) fn init(wgpu_context: &WgpuContext) {
-		DesktopWrapper::init(wgpu_context, rand::rng().random());
+	pub(crate) fn init() {
 		Window::init();
 	}
 
-	pub(crate) fn new(
+	pub(crate) fn create(
 		cef_context: Box<dyn cef::CefContext>,
 		cef_view_info_sender: Sender<cef::ViewInfoUpdate>,
 		wgpu_context: WgpuContext,
@@ -80,6 +79,8 @@ impl App {
 		let mut persistent_data = PersistentData::default();
 		persistent_data.load_from_disk();
 
+		let desktop_wrapper = DesktopWrapper::create(&wgpu_context.clone(), rand::rng().random());
+
 		Self {
 			render_state: None,
 			wgpu_context,
@@ -93,7 +94,7 @@ impl App {
 			ui_scale: 1.,
 			app_event_receiver,
 			app_event_scheduler,
-			desktop_wrapper: DesktopWrapper::new(),
+			desktop_wrapper,
 			last_ui_update: Instant::now(),
 			cef_context,
 			cef_schedule: Some(Instant::now()),

desktop/src/lib.rs

@@ -66,16 +66,16 @@ pub fn start() {
 		}
 	};
 
-	let wgpu_context = futures::executor::block_on(gpu_context::create_wgpu_context());
-
-	App::init(&wgpu_context);
+	// Must be called before event loop initialization or native window integrations will break
+	App::init();
 
 	let event_loop = EventLoop::new().unwrap();
 	let (app_event_sender, app_event_receiver) = std::sync::mpsc::channel();
 	let app_event_scheduler = event_loop.create_app_event_scheduler(app_event_sender);
 
 	let (cef_view_info_sender, cef_view_info_receiver) = std::sync::mpsc::channel();
 
+	let wgpu_context = futures::executor::block_on(gpu_context::create_wgpu_context());
 	let cef_handler = cef::CefHandler::new(wgpu_context.clone(), app_event_scheduler.clone(), cef_view_info_receiver);
 	let cef_context = match cef_context_builder.initialize(cef_handler, cli.disable_ui_acceleration) {
 		Ok(context) => {
@@ -100,7 +100,7 @@ pub fn start() {
 		}
 	};
 
-	let mut app = App::new(Box::new(cef_context), cef_view_info_sender, wgpu_context, app_event_receiver, app_event_scheduler, cli.files);
+	let mut app = App::create(Box::new(cef_context), cef_view_info_sender, wgpu_context, app_event_receiver, app_event_scheduler, cli.files);
 
 	event_loop.run_app(&mut app).unwrap();
 

desktop/wrapper/src/lib.rs

@@ -27,22 +27,21 @@ pub struct DesktopWrapper {
 }
 
 impl DesktopWrapper {
-	pub fn init(wgpu_context: &WgpuContext, uuid_random_seed: u64) {
+	pub fn create(wgpu_context: &WgpuContext, uuid_random_seed: u64) -> Self {
+		let application_io = WasmApplicationIo::new_with_context(wgpu_context.clone());
+		futures::executor::block_on(graphite_editor::node_graph_executor::replace_application_io(application_io));
+
 		#[cfg(target_os = "windows")]
 		let host = Host::Windows;
 		#[cfg(target_os = "macos")]
 		let host = Host::Mac;
 		#[cfg(target_os = "linux")]
 		let host = Host::Linux;
 		let env = Environment { platform: Platform::Desktop, host };
-		Editor::init(env, uuid_random_seed);
 
-		let application_io = WasmApplicationIo::new_with_context(wgpu_context.clone());
-		futures::executor::block_on(graphite_editor::node_graph_executor::replace_application_io(application_io));
-	}
-
-	pub fn new() -> Self {
-		Self { editor: Editor::new() }
+		Self {
+			editor: Editor::create(env, uuid_random_seed),
+		}
 	}
 
 	pub fn dispatch(&mut self, message: DesktopWrapperMessage) -> Vec<DesktopFrontendMessage> {
@@ -60,12 +59,6 @@ impl DesktopWrapper {
 	}
 }
 
-impl Default for DesktopWrapper {
-	fn default() -> Self {
-		Self::new()
-	}
-}
-
 pub enum NodeGraphExecutionResult {
 	HasRun(Option<wgpu::Texture>),
 	NotRun,

editor/src/application.rs

@@ -8,21 +8,24 @@ pub struct Editor {
 }
 
 impl Editor {
-	pub fn init(environment: Environment, uuid_random_seed: u64) {
+	pub fn create(environment: Environment, uuid_random_seed: u64) -> Self {
 		ENVIRONMENT.set(environment).expect("Editor shoud only be initialized once");
-		graphene_std::uuid::init_rng(uuid_random_seed);
-	}
+		graphene_std::uuid::set_uuid_seed(uuid_random_seed);
 
-	/// `Editor::init` must be called before this function
-	pub fn new() -> Self {
 		Self { dispatcher: Dispatcher::new() }
 	}
 
 	#[cfg(test)]
 	pub(crate) fn new_local_executor() -> (Self, crate::node_graph_executor::NodeRuntime) {
+		let _ = ENVIRONMENT.set(*Editor::environment());
+		graphene_std::uuid::set_uuid_seed(0);
+
 		let (runtime, executor) = crate::node_graph_executor::NodeGraphExecutor::new_with_local_runtime();
-		let dispatcher = Dispatcher::with_executor(executor);
-		(Self { dispatcher }, runtime)
+		let editor = Self {
+			dispatcher: Dispatcher::with_executor(executor),
+		};
+
+		(editor, runtime)
 	}
 
 	pub fn handle_message<T: Into<Message>>(&mut self, message: T) -> Vec<FrontendMessage> {
@@ -35,17 +38,21 @@ impl Editor {
 		self.dispatcher.poll_node_graph_evaluation(responses)
 	}
 }
-impl Default for Editor {
-	fn default() -> Self {
-		Self::new()
-	}
-}
 
 static ENVIRONMENT: OnceLock<Environment> = OnceLock::new();
 impl Editor {
+	#[cfg(not(test))]
 	pub fn environment() -> &'static Environment {
 		ENVIRONMENT.get().expect("Editor environment accessed before initialization")
 	}
+
+	#[cfg(test)]
+	pub fn environment() -> &'static Environment {
+		&Environment {
+			platform: Platform::Desktop,
+			host: Host::Linux,
+		}
+	}
 }
 
 #[derive(Clone, Copy, Debug)]

editor/src/messages/input_preprocessor/input_preprocessor_message_handler.rs

@@ -187,7 +187,6 @@ impl InputPreprocessorMessageHandler {
 mod test {
 	use crate::messages::input_mapper::utility_types::input_keyboard::{Key, ModifierKeys};
 	use crate::messages::input_mapper::utility_types::input_mouse::{EditorMouseState, MouseKeys, ScrollDelta};
-	use crate::messages::portfolio::utility_types::KeyboardPlatformLayout;
 	use crate::messages::prelude::*;
 
 	#[test]

editor/src/test_utils.rs

@@ -1,9 +1,7 @@
 use crate::application::Editor;
-use crate::application::set_uuid_seed;
 use crate::messages::input_mapper::utility_types::input_keyboard::ModifierKeys;
 use crate::messages::input_mapper::utility_types::input_mouse::{EditorMouseState, MouseKeys, ScrollDelta, ViewportPosition};
 use crate::messages::portfolio::document::node_graph::document_node_definitions::DefinitionIdentifier;
-use crate::messages::portfolio::utility_types::Platform;
 use crate::messages::prelude::*;
 use crate::messages::tool::tool_messages::tool_prelude::Key;
 use crate::messages::tool::utility_types::ToolType;
@@ -25,15 +23,11 @@ pub struct EditorTestUtils {
 impl EditorTestUtils {
 	pub fn create() -> Self {
 		let _ = env_logger::builder().is_test(true).try_init();
-		set_uuid_seed(0);
 
 		let (mut editor, runtime) = Editor::new_local_executor();
 
-		// We have to set this directly instead of using `GlobalsMessage::SetPlatform` because race conditions with multiple tests can cause that message handler to set it more than once, which is a failure.
-		// It isn't sufficient to guard the message dispatch here with a check if the once_cell is empty, because that isn't atomic and the time between checking and handling the dispatch can let multiple through.
-		let _ = GLOBAL_PLATFORM.set(Platform::Windows).is_ok();
-
 		editor.handle_message(PortfolioMessage::Init);
+
 		Self { editor, runtime }
 	}
 

frontend/src/editor.ts

@@ -1,7 +1,7 @@
 // import { panicProxy } from "@graphite/utility-functions/panic-proxy";
 
-import type { EditorHandle } from "@graphite/../wasm/pkg/graphite_wasm";
-import init, { wasmMemory, receiveNativeMessage, constructEditor } from "@graphite/../wasm/pkg/graphite_wasm";
+import { EditorHandle } from "@graphite/../wasm/pkg/graphite_wasm";
+import init, { wasmMemory, receiveNativeMessage } from "@graphite/../wasm/pkg/graphite_wasm";
 import { type JsMessageType } from "@graphite/messages";
 import { createSubscriptionRouter, type SubscriptionRouter } from "@graphite/subscription-router";
 import { operatingSystem } from "@graphite/utility-functions/platform";
@@ -45,14 +45,14 @@ export function createEditor(): Editor {
 	const randomSeed = BigInt(randomSeedFloat);
 
 	// Handle: object containing many functions from `editor_api.rs` that are part of the `EditorHandle` struct (generated by wasm-bindgen)
-	const handle: EditorHandle = constructEditor(operatingSystem(), randomSeed, (messageType: JsMessageType, messageData: Record<string, unknown>) => {
+	const handle = EditorHandle.create(operatingSystem(), randomSeed, (messageType: JsMessageType, messageData: Record<string, unknown>) => {
 		// This callback is called by Wasm when a FrontendMessage is received from the Wasm wrapper `EditorHandle`
 		// We pass along the first two arguments then add our own `raw` and `handle` context for the last two arguments
 		subscriptions.handleJsMessage(messageType, messageData, raw, handle);
 	});
 
 	// Subscriptions: allows subscribing to messages in JS that are sent from the Wasm backend
-	const subscriptions: SubscriptionRouter = createSubscriptionRouter();
+	const subscriptions = createSubscriptionRouter();
 
 	// Check if the URL hash fragment has any demo artwork to be loaded
 	(async () => {

frontend/wasm/src/editor_api.rs

@@ -79,45 +79,42 @@ impl EditorHandle {
 	}
 }
 
-#[cfg(not(feature = "native"))]
-#[wasm_bindgen(js_name = constructEditor)]
-pub fn construct_editor(platform: String, uuid_random_seed: u64, frontend_message_handler_callback: js_sys::Function) -> EditorHandle {
-	Editor::init(
-		Environment {
-			platform: Platform::Web,
-			host: match platform.as_str() {
-				"Linux" => Host::Linux,
-				"Mac" => Host::Mac,
-				"Windows" => Host::Windows,
-				_ => unreachable!(),
+#[wasm_bindgen]
+impl EditorHandle {
+	#[cfg(not(feature = "native"))]
+	pub fn create(platform: String, uuid_random_seed: u64, frontend_message_handler_callback: js_sys::Function) -> EditorHandle {
+		let editor = Editor::create(
+			Environment {
+				platform: Platform::Web,
+				host: match platform.as_str() {
+					"Linux" => Host::Linux,
+					"Mac" => Host::Mac,
+					"Windows" => Host::Windows,
+					_ => unreachable!(),
+				},
 			},
-		},
-		uuid_random_seed,
-	);
+			uuid_random_seed,
+		);
 
-	let editor = Editor::new();
-	let editor_handle = EditorHandle { frontend_message_handler_callback };
-	if EDITOR.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor))).is_none() {
-		log::error!("Attempted to initialize the editor more than once");
-	}
-	if EDITOR_HANDLE.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor_handle.clone()))).is_none() {
-		log::error!("Attempted to initialize the editor handle more than once");
+		let editor_handle = EditorHandle { frontend_message_handler_callback };
+		if EDITOR.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor))).is_none() {
+			log::error!("Attempted to initialize the editor more than once");
+		}
+		if EDITOR_HANDLE.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor_handle.clone()))).is_none() {
+			log::error!("Attempted to initialize the editor handle more than once");
+		}
+		editor_handle
 	}
-	editor_handle
-}
 
-#[cfg(feature = "native")]
-#[wasm_bindgen(js_name = constructEditor)]
-pub fn construct_editor(_platform: String, _uuid_random_seed: u64, frontend_message_handler_callback: js_sys::Function) -> EditorHandle {
-	let editor_handle = EditorHandle { frontend_message_handler_callback };
-	if EDITOR_HANDLE.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor_handle.clone()))).is_none() {
-		log::error!("Attempted to initialize the editor handle more than once");
+	#[cfg(feature = "native")]
+	pub fn create(_platform: String, _uuid_random_seed: u64, frontend_message_handler_callback: js_sys::Function) -> EditorHandle {
+		let editor_handle = EditorHandle { frontend_message_handler_callback };
+		if EDITOR_HANDLE.with(|handle| handle.lock().ok().map(|mut guard| *guard = Some(editor_handle.clone()))).is_none() {
+			log::error!("Attempted to initialize the editor handle more than once");
+		}
+		editor_handle
 	}
-	editor_handle
-}
 
-#[wasm_bindgen]
-impl EditorHandle {
 	// Sends a message to the dispatcher in the Editor Backend
 	#[cfg(not(feature = "native"))]
 	fn dispatch<T: Into<Message>>(&self, message: T) {

node-graph/libraries/core-types/src/uuid.rs

@@ -45,19 +45,27 @@ mod u64_string {
 mod uuid_generation {
 	use rand_chacha::ChaCha20Rng;
 	use rand_chacha::rand_core::{RngCore, SeedableRng};
+	use std::cell::Cell;
 	use std::sync::Mutex;
 
 	static RNG: Mutex<Option<ChaCha20Rng>> = Mutex::new(None);
+	thread_local! {
+		pub static UUID_SEED: Cell<Option<u64>> = const { Cell::new(None) };
+	}
 
-	pub fn init_rng(random_seed: u64) {
-		let mut lock = RNG.lock().expect("UUID mutex poisoned");
-		*lock = Some(ChaCha20Rng::seed_from_u64(random_seed));
+	pub fn set_uuid_seed(random_seed: u64) {
+		UUID_SEED.with(|seed| seed.set(Some(random_seed)))
 	}
 
 	pub fn generate_uuid() -> u64 {
 		let Ok(mut lock) = RNG.lock() else { panic!("UUID mutex poisoned") };
-		let rng = lock.as_mut().expect("UUID generator not initialized. Forgot to call `init_rng`?");
-		rng.next_u64()
+		if lock.is_none() {
+			UUID_SEED.with(|seed| {
+				let random_seed = seed.get().unwrap_or(42);
+				*lock = Some(ChaCha20Rng::seed_from_u64(random_seed));
+			})
+		}
+		lock.as_mut().map(ChaCha20Rng::next_u64).expect("UUID mutex poisoned")
 	}
 }