Remove unsafe code and clean up the code base in general (#1263)

* Remove unsafe code

* Make node graph test syncronous

* Add miri step to ci

* Remove unsafe from node graph evaluation

* Replace operation pseudo_hash with hash based on discriminant

* Fix test

* Move memo module to core and make it safe

* Fix formatting

* Remove unused stuff from gstd

* Use safe casting for creating key variants

* Fix memo node types

* Fix ref node

* "fix" ub

* Use correct input types for ExtractImageFrame

* Fix types for async nodes

* Fix missing implementation

* Manually override output type for async nodes

* Fix types for EditorApi

* Fix output type for WasmSurfaceHandle

* Remove unused miri.yml

* Fix incorrect type for cache node

node-graph/gcore/src/application_io.rs

@@ -36,34 +36,42 @@ unsafe impl StaticType for SurfaceFrame {
 	type Static = SurfaceFrame;
 }
 
+impl<S> From<SurfaceHandleFrame<S>> for SurfaceFrame {
+	fn from(x: SurfaceHandleFrame<S>) -> Self {
+		Self {
+			surface_id: x.surface_handle.surface_id,
+			transform: x.transform,
+		}
+	}
+}
+
 #[derive(Clone)]
-pub struct SurfaceHandle<'a, Surface> {
+pub struct SurfaceHandle<Surface> {
 	pub surface_id: SurfaceId,
 	pub surface: Surface,
-	application_io: &'a dyn ApplicationIo<Surface = Surface>,
 }
 
-unsafe impl<T: 'static> StaticType for SurfaceHandle<'_, T> {
-	type Static = SurfaceHandle<'static, T>;
+unsafe impl<T: 'static> StaticType for SurfaceHandle<T> {
+	type Static = SurfaceHandle<T>;
 }
 
 #[derive(Clone)]
-pub struct SurfaceHandleFrame<'a, Surface> {
-	pub surface_handle: Arc<SurfaceHandle<'a, Surface>>,
+pub struct SurfaceHandleFrame<Surface> {
+	pub surface_handle: Arc<SurfaceHandle<Surface>>,
 	pub transform: DAffine2,
 }
 
-unsafe impl<T: 'static> StaticType for SurfaceHandleFrame<'_, T> {
-	type Static = SurfaceHandleFrame<'static, T>;
+unsafe impl<T: 'static> StaticType for SurfaceHandleFrame<T> {
+	type Static = SurfaceHandleFrame<T>;
 }
 
-impl<T> Transform for SurfaceHandleFrame<'_, T> {
+impl<T> Transform for SurfaceHandleFrame<T> {
 	fn transform(&self) -> DAffine2 {
 		self.transform
 	}
 }
 
-impl<T> TransformMut for SurfaceHandleFrame<'_, T> {
+impl<T> TransformMut for SurfaceHandleFrame<T> {
 	fn transform_mut(&mut self) -> &mut DAffine2 {
 		&mut self.transform
 	}
@@ -142,10 +150,10 @@ impl<'a, T> AsRef<EditorApi<'a, T>> for EditorApi<'a, T> {
 #[derive(Debug, Clone, Copy, Default)]
 pub struct ExtractImageFrame;
 
-impl<'a: 'input, 'input, T> Node<'input, &'a EditorApi<'a, T>> for ExtractImageFrame {
+impl<'a: 'input, 'input, T> Node<'input, EditorApi<'a, T>> for ExtractImageFrame {
 	type Output = ImageFrame<Color>;
-	fn eval(&'input self, editor_api: &'a EditorApi<'a, T>) -> Self::Output {
-		editor_api.image_frame.clone().unwrap_or(ImageFrame::identity())
+	fn eval(&'input self, editor_api: EditorApi<'a, T>) -> Self::Output {
+		editor_api.image_frame.unwrap_or(ImageFrame::identity())
 	}
 }
 

node-graph/gcore/src/application_io/wasm_application_io.rs

@@ -1,8 +1,8 @@
-use std::{cell::RefCell, collections::HashMap, sync::Mutex};
+use std::{cell::RefCell, collections::HashMap};
 
 use super::{ApplicationIo, SurfaceHandle, SurfaceHandleFrame, SurfaceId};
 use crate::{
-	raster::{color::SRGBA8, ImageFrame, Pixel},
+	raster::{color::SRGBA8, ImageFrame},
 	Node,
 };
 use alloc::sync::Arc;
@@ -35,7 +35,7 @@ impl ApplicationIo for WasmApplicationIo {
 	type Surface = CanvasRenderingContext2d;
 
 	fn create_surface(&self) -> SurfaceHandle<Self::Surface> {
-		let mut wrapper = || {
+		let wrapper = || {
 			let document = window().expect("should have a window in this context").document().expect("window should have a document");
 
 			let canvas: HtmlCanvasElement = document.create_element("canvas")?.dyn_into::<HtmlCanvasElement>()?;
@@ -53,7 +53,7 @@ impl ApplicationIo for WasmApplicationIo {
 			let image_canvases_key = JsValue::from_str("imageCanvases");
 
 			let mut canvases = Reflect::get(&window, &image_canvases_key);
-			if let Err(e) = canvases {
+			if let Err(_) = canvases {
 				Reflect::set(&JsValue::from(web_sys::window().unwrap()), &image_canvases_key, &Object::new()).unwrap();
 				canvases = Reflect::get(&window, &image_canvases_key);
 			}
@@ -66,11 +66,7 @@ impl ApplicationIo for WasmApplicationIo {
 
 			// Use Reflect API to set property
 			Reflect::set(&canvases, &js_key, &js_value)?;
-			Ok::<_, JsValue>(SurfaceHandle {
-				surface_id: id,
-				surface: context,
-				application_io: self,
-			})
+			Ok::<_, JsValue>(SurfaceHandle { surface_id: id, surface: context })
 		};
 
 		wrapper().expect("should be able to set canvas in global scope")
@@ -99,13 +95,13 @@ impl ApplicationIo for WasmApplicationIo {
 	}
 }
 
-pub type WasmSurfaceHandle<'a> = SurfaceHandle<'a, CanvasRenderingContext2d>;
-pub type WasmSurfaceHandleFrame<'a> = SurfaceHandleFrame<'a, CanvasRenderingContext2d>;
+pub type WasmSurfaceHandle = SurfaceHandle<CanvasRenderingContext2d>;
+pub type WasmSurfaceHandleFrame = SurfaceHandleFrame<CanvasRenderingContext2d>;
 
 pub struct CreateSurfaceNode {}
 
 #[node_macro::node_fn(CreateSurfaceNode)]
-fn create_surface_node<'a: 'input>(editor: &'a WasmEditorApi<'a>) -> Arc<SurfaceHandle<'a, CanvasRenderingContext2d>> {
+fn create_surface_node<'a: 'input>(editor: WasmEditorApi<'a>) -> Arc<SurfaceHandle<CanvasRenderingContext2d>> {
 	editor.application_io.create_surface().into()
 }
 
@@ -114,7 +110,7 @@ pub struct DrawImageFrameNode<Surface> {
 }
 
 #[node_macro::node_fn(DrawImageFrameNode)]
-async fn draw_image_frame_node<'a: 'input>(image: ImageFrame<SRGBA8>, surface_handle: Arc<SurfaceHandle<'a, CanvasRenderingContext2d>>) -> SurfaceHandleFrame<'a, CanvasRenderingContext2d> {
+async fn draw_image_frame_node<'a: 'input>(image: ImageFrame<SRGBA8>, surface_handle: Arc<SurfaceHandle<CanvasRenderingContext2d>>) -> SurfaceHandleFrame<CanvasRenderingContext2d> {
 	let image_data = image.image.data;
 	let array: Clamped<&[u8]> = Clamped(bytemuck::cast_slice(image_data.as_slice()));
 	if image.image.width > 0 && image.image.height > 0 {

node-graph/gcore/src/lib.rs

@@ -20,6 +20,7 @@ pub mod value;
 #[cfg(feature = "gpu")]
 pub mod gpu;
 
+pub mod memo;
 pub mod storage;
 
 pub mod raster;
@@ -44,7 +45,7 @@ pub use raster::Color;
 pub trait Node<'i, Input: 'i>: 'i {
 	type Output: 'i;
 	fn eval(&'i self, input: Input) -> Self::Output;
-	fn reset(self: Pin<&mut Self>) {}
+	fn reset(&self) {}
 	#[cfg(feature = "std")]
 	fn serialize(&self) -> Option<std::sync::Arc<dyn core::any::Any>> {
 		log::warn!("Node::serialize not implemented for {}", core::any::type_name::<Self>());

node-graph/gcore/src/memo.rs

@@ -0,0 +1,158 @@
+use crate::Node;
+use core::future::Future;
+
+#[cfg(feature = "alloc")]
+use alloc::sync::Arc;
+use core::cell::Cell;
+use core::pin::Pin;
+use std::marker::PhantomData;
+
+// Caches the output of a given Node and acts as a proxy
+#[derive(Default)]
+pub struct MemoNode<T, CachedNode> {
+	cache: Cell<Option<T>>,
+	node: CachedNode,
+}
+impl<'i, 'o: 'i, T: 'i + Clone + 'o, CachedNode: 'i> Node<'i, ()> for MemoNode<T, CachedNode>
+where
+	CachedNode: for<'any_input> Node<'any_input, ()>,
+	for<'a> <CachedNode as Node<'a, ()>>::Output: core::future::Future<Output = T> + 'a,
+{
+	// TODO: This should return a reference to the cached cached_value
+	// but that requires a lot of lifetime magic <- This was suggested by copilot but is pretty acurate xD
+	type Output = Pin<Box<dyn Future<Output = T> + 'i>>;
+	fn eval(&'i self, input: ()) -> Self::Output {
+		Box::pin(async move {
+			if let Some(cached_value) = self.cache.take() {
+				self.cache.set(Some(cached_value.clone()));
+				cached_value
+			} else {
+				let value = self.node.eval(input).await;
+				self.cache.set(Some(value.clone()));
+				value
+			}
+		})
+	}
+
+	fn reset(&self) {
+		self.cache.set(None);
+	}
+}
+
+impl<T, CachedNode> MemoNode<T, CachedNode> {
+	pub const fn new(node: CachedNode) -> MemoNode<T, CachedNode> {
+		MemoNode { cache: Cell::new(None), node }
+	}
+}
+
+#[cfg(feature = "alloc")]
+/// Caches the output of the last graph evaluation for introspection
+#[derive(Default)]
+pub struct MonitorNode<T> {
+	output: Cell<Option<Arc<T>>>,
+}
+
+#[cfg(feature = "alloc")]
+impl<'i, T: 'static + Clone> Node<'i, T> for MonitorNode<T> {
+	type Output = T;
+	fn eval(&'i self, input: T) -> Self::Output {
+		self.output.set(Some(Arc::new(input.clone())));
+		input
+	}
+
+	fn serialize(&self) -> Option<Arc<dyn core::any::Any>> {
+		let out = self.output.take();
+		self.output.set(out.clone());
+		(out).as_ref().map(|output| output.clone() as Arc<dyn core::any::Any>)
+	}
+}
+
+#[cfg(feature = "alloc")]
+impl<T> MonitorNode<T> {
+	pub const fn new() -> MonitorNode<T> {
+		MonitorNode { output: Cell::new(None) }
+	}
+}
+
+// Caches the output of a given Node and acts as a proxy
+/// It provides two modes of operation, it can either be set
+/// when calling the node with a `Some<T>` variant or the last
+/// value that was added is returned when calling it with `None`
+#[derive(Default)]
+pub struct LetNode<T> {
+	// We have to use an append only data structure to make sure the references
+	// to the cache entries are always valid
+	// TODO: We only ever access the last value so there is not really a reason for us
+	// to store the previous entries. This should be reworked in the future
+	cache: Cell<Option<T>>,
+}
+impl<'i, T: 'i + Clone> Node<'i, Option<T>> for LetNode<T> {
+	type Output = T;
+	fn eval(&'i self, input: Option<T>) -> Self::Output {
+		if let Some(input) = input {
+			self.cache.set(Some(input.clone()));
+			input
+		} else {
+			let value = self.cache.take();
+			self.cache.set(value.clone());
+			value.expect("LetNode was not initialized. This can happen if you try to evaluate a node that depends on the EditorApi in the node_registry")
+		}
+	}
+	fn reset(&self) {
+		self.cache.set(None);
+	}
+}
+
+impl<T> std::marker::Unpin for LetNode<T> {}
+
+impl<T> LetNode<T> {
+	pub fn new() -> LetNode<T> {
+		LetNode { cache: Default::default() }
+	}
+}
+
+/// Caches the output of a given Node and acts as a proxy
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
+pub struct EndLetNode<Input> {
+	input: Input,
+}
+impl<'i, T: 'i, Input> Node<'i, T> for EndLetNode<Input>
+where
+	Input: Node<'i, ()>,
+{
+	type Output = <Input>::Output;
+	fn eval(&'i self, _: T) -> Self::Output {
+		let result = self.input.eval(());
+		result
+	}
+}
+
+impl<Input> EndLetNode<Input> {
+	pub const fn new(input: Input) -> EndLetNode<Input> {
+		EndLetNode { input }
+	}
+}
+
+pub use crate::ops::SomeNode as InitNode;
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
+pub struct RefNode<T, Let> {
+	let_node: Let,
+	_t: PhantomData<T>,
+}
+
+impl<'i, T: 'i, Let> Node<'i, ()> for RefNode<T, Let>
+where
+	Let: for<'a> Node<'a, Option<T>>,
+{
+	type Output = <Let as Node<'i, Option<T>>>::Output;
+	fn eval(&'i self, _: ()) -> Self::Output {
+		self.let_node.eval(None)
+	}
+}
+
+impl<Let, T> RefNode<T, Let> {
+	pub const fn new(let_node: Let) -> RefNode<T, Let> {
+		RefNode { let_node, _t: PhantomData }
+	}
+}

node-graph/gcore/src/storage.rs

@@ -1,22 +1,10 @@
 use crate::Node;
 
-use core::cell::RefMut;
-use core::marker::PhantomData;
 use core::ops::{Deref, DerefMut, Index, IndexMut};
 
 pub struct SetNode<Storage> {
 	storage: Storage,
 }
-/*
-#[node_macro::node_fn(SetNode)]
-fn set_node<_T, _I, A: 'input>(input: (_T, _I), mut storage: RefMut<'input, A>)
-where
-	A: DerefMut,
-	A::Target: IndexMut<_I, Output = _T>,
-{
-	let (value, index) = input;
-	*storage.deref_mut().index_mut(index).deref_mut() = value;
-}*/
 impl<'input, T: 'input, I: 'input, A: 'input + 'input, S0: 'input> Node<'input, (T, I)> for SetNode<S0>
 where
 	A: DerefMut,
@@ -94,7 +82,7 @@ where
 
 #[cfg(test)]
 mod test {
-	use crate::value::{CopiedNode, OnceCellNode, RefCellMutNode, UnsafeMutValueNode, ValueNode};
+	use crate::value::{CopiedNode, OnceCellNode};
 	use crate::Node;
 
 	use super::*;

node-graph/gcore/src/structural.rs

@@ -172,6 +172,7 @@ mod test {
 	}
 
 	#[test]
+	#[allow(clippy::unit_cmp)]
 	fn test_apply() {
 		let mut array = [1, 2, 3];
 		let slice = &mut array;

node-graph/gcore/src/text.rs

@@ -15,7 +15,7 @@ pub struct TextGenerator<Text, FontName, Size> {
 }
 
 #[node_fn(TextGenerator)]
-fn generate_text<'a: 'input>(editor: &'a EditorApi<'a>, text: String, font_name: Font, font_size: f64) -> crate::vector::VectorData {
+fn generate_text<'a: 'input>(editor: EditorApi<'a>, text: String, font_name: Font, font_size: f64) -> crate::vector::VectorData {
 	let buzz_face = editor.font_cache.get(&font_name).map(|data| load_face(data));
 	crate::vector::VectorData::from_subpaths(to_path(&text, buzz_face, font_size, None))
 }

node-graph/gcore/src/types.rs

@@ -56,15 +56,11 @@ macro_rules! generic {
 
 #[macro_export]
 macro_rules! fn_type {
-	($input:ty, $output:ty) => {
-		Type::Fn(Box::new(concrete!($input)), Box::new(concrete!($output)))
+	($type:ty) => {
+		Type::Fn(Box::new(concrete!(())), Box::new(concrete!($type)))
 	};
-}
-
-#[macro_export]
-macro_rules! value_fn {
-	($output:ty) => {
-		Type::Fn(Box::new(concrete!(())), Box::new(concrete!($output)))
+	($in_type:ty, $type:ty) => {
+		Type::Fn(Box::new(concrete!(($in_type))), Box::new(concrete!($type)))
 	};
 }
 
@@ -109,6 +105,7 @@ pub enum Type {
 	Generic(Cow<'static, str>),
 	Concrete(TypeDescriptor),
 	Fn(Box<Type>, Box<Type>),
+	Future(Box<Type>),
 }
 
 impl Type {
@@ -169,6 +166,7 @@ impl Type {
 			Self::Generic(_) => None,
 			Self::Concrete(ty) => Some(ty.size),
 			Self::Fn(_, _) => None,
+			Self::Future(_) => None,
 		}
 	}
 
@@ -177,6 +175,7 @@ impl Type {
 			Self::Generic(_) => None,
 			Self::Concrete(ty) => Some(ty.align),
 			Self::Fn(_, _) => None,
+			Self::Future(_) => None,
 		}
 	}
 }
@@ -190,6 +189,7 @@ impl core::fmt::Debug for Type {
 			#[cfg(not(feature = "type_id_logging"))]
 			Self::Concrete(arg0) => write!(f, "Concrete({})", arg0.name),
 			Self::Fn(arg0, arg1) => write!(f, "({:?} -> {:?})", arg0, arg1),
+			Self::Future(arg0) => write!(f, "Future({:?})", arg0),
 		}
 	}
 }
@@ -200,6 +200,7 @@ impl std::fmt::Display for Type {
 			Type::Generic(name) => write!(f, "{}", name),
 			Type::Concrete(ty) => write!(f, "{}", ty.name),
 			Type::Fn(input, output) => write!(f, "({} -> {})", input, output),
+			Type::Future(ty) => write!(f, "Future<{}>", ty),
 		}
 	}
 }

node-graph/gcore/src/value.rs

@@ -1,7 +1,6 @@
 use crate::Node;
 
 use core::{
-	borrow::BorrowMut,
 	cell::{Cell, RefCell, RefMut},
 	marker::PhantomData,
 };
@@ -47,10 +46,7 @@ impl<'i, T: 'i> Node<'i, ()> for RefCellMutNode<T> {
 	type Output = RefMut<'i, T>;
 	#[inline(always)]
 	fn eval(&'i self, _input: ()) -> Self::Output {
-		#[cfg(not(target_arch = "spirv"))]
 		let a = self.0.borrow_mut();
-		#[cfg(target_arch = "spirv")]
-		let a = unsafe { self.0.try_borrow_mut().unwrap_unchecked() };
 		a
 	}
 }
@@ -60,24 +56,6 @@ impl<T> RefCellMutNode<T> {
 		RefCellMutNode(RefCell::new(value))
 	}
 }
-/// #Safety: Never use this as it is unsound.
-#[derive(Default, Debug)]
-pub struct UnsafeMutValueNode<T>(pub T);
-
-/// #Safety: Never use this as it is unsound.
-impl<'i, T: 'i> Node<'i, ()> for UnsafeMutValueNode<T> {
-	type Output = &'i mut T;
-	#[inline(always)]
-	fn eval(&'i self, _input: ()) -> Self::Output {
-		unsafe { &mut *(&self.0 as &T as *const T as *mut T) }
-	}
-}
-
-impl<T> UnsafeMutValueNode<T> {
-	pub const fn new(value: T) -> UnsafeMutValueNode<T> {
-		UnsafeMutValueNode(value)
-	}
-}
 
 #[derive(Default)]
 pub struct OnceCellNode<T>(pub Cell<T>);