fix(visual_math): handle case where the expression to compute contain…

…s a loop (#17)
tguichaoua · Nov 25, 2024 · 6083b28 · 6083b28
1 parent a2ae802
commit 6083b28
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 51 deletions.
diff --git a/examples/visual_math/src/app/graph.rs b/examples/visual_math/src/app/graph.rs
@@ -13,6 +13,17 @@ use crate::graph::{
 
 /* -------------------------------------------------------------------------- */
 
+/// The result of the computation of the expression from the selected node of the graph.
+#[derive(serde::Serialize, serde::Deserialize)]
+pub enum ExprResult {
+    /// There is no expression computed, yet.
+    None,
+    /// The computed expression.
+    Expr(Expr),
+    /// The computation of the expression fails due to a loop in the expression.
+    LoopError,
+}
+
 /// The adapter for the math graph that will render into the visual editor.
 #[derive(serde::Serialize, serde::Deserialize)]
 pub struct GraphApp {
@@ -27,7 +38,7 @@ pub struct GraphApp {
     /// Whether or not we need to rebuild the expr.
     may_need_to_rebuild_expr: bool,
     /// The last built expression.
-    expr: Option<Expr>,
+    expr: ExprResult,
 }
 
 /// An expression from the graph.
@@ -53,7 +64,7 @@ impl GraphApp {
             selected_node: None,
 
             may_need_to_rebuild_expr: false,
-            expr: None,
+            expr: ExprResult::None,
         }
     }
 
@@ -66,23 +77,23 @@ impl GraphApp {
     /// Get the expression of the currently selected node, if any.
     ///
     /// Rebuild the expression if dirty.
-    pub fn rebuild_and_get_expr(&mut self) -> Option<&Expr> {
+    pub fn rebuild_and_get_expr(&mut self) -> &ExprResult {
         self.rebuild_expr();
 
-        self.expr.as_ref()
+        &self.expr
     }
 
     /// Get the expression of the currently selected node, if any.
     ///
     /// Rebuild the expression if dirty, and force the recomputation of the value.
-    pub fn rebuild_recompute_and_get_expr(&mut self) -> Option<&Expr> {
+    pub fn rebuild_recompute_and_get_expr(&mut self) -> &ExprResult {
         if !self.rebuild_expr() {
-            if let Some(Expr { expr, value, .. }) = self.expr.as_mut() {
+            if let ExprResult::Expr(Expr { expr, value, .. }) = &mut self.expr {
                 *value = compute_value(expr, &self.graph);
             }
         }
 
-        self.expr.as_ref()
+        &self.expr
     }
 
     /// Rebuild the expression if dirty.
@@ -93,31 +104,37 @@ impl GraphApp {
             if let Some(selected_socket_id) = self.selected_node {
                 let expr = self.graph.build_expr_from(selected_socket_id);
 
-                if let Ok(expr) = expr {
-                    let formula = expr
-                        .display(|input_id, f| {
-                            if let Some(input) = self.graph.get_input(input_id) {
-                                f.write_str(input.name())
-                            } else {
-                                f.write_str("?")
-                            }
-                        })
-                        .to_string();
-
-                    let value = compute_value(&expr, &self.graph);
-
-                    self.expr = Some(Expr {
-                        expr,
-                        formula,
-                        value,
-                    });
-                    self.may_need_to_rebuild_expr = false;
-
-                    return true;
+                match expr {
+                    Ok(expr) => {
+                        let formula = expr
+                            .display(|input_id, f| {
+                                if let Some(input) = self.graph.get_input(input_id) {
+                                    f.write_str(input.name())
+                                } else {
+                                    f.write_str("?")
+                                }
+                            })
+                            .to_string();
+
+                        let value = compute_value(&expr, &self.graph);
+
+                        self.expr = ExprResult::Expr(Expr {
+                            expr,
+                            formula,
+                            value,
+                        });
+                        self.may_need_to_rebuild_expr = false;
+
+                        return true;
+                    }
+                    Err(crate::graph::BuildExprError::NodeNotFound) => {
+                        self.selected_node = None;
+                        self.expr = ExprResult::None;
+                    }
+                    Err(crate::graph::BuildExprError::Loop) => {
+                        self.expr = ExprResult::LoopError;
+                    }
                 }
-
-                self.selected_node = None;
-                self.expr = None;
             }
         }
 

diff --git a/examples/visual_math/src/app/mod.rs b/examples/visual_math/src/app/mod.rs
@@ -114,8 +114,14 @@ impl App {
             self.graph.rebuild_and_get_expr()
         };
 
-        if let Some(expr) = expr {
-            ui.label(format!("{} = {}", expr.formula, expr.value));
+        match expr {
+            graph::ExprResult::None => {}
+            graph::ExprResult::Expr(expr) => {
+                ui.label(format!("{} = {}", expr.formula, expr.value));
+            }
+            graph::ExprResult::LoopError => {
+                ui.colored_label(egui::Color32::RED, "The expr contains a loop!");
+            }
         }
     }
 

diff --git a/examples/visual_math/src/graph/expr.rs b/examples/visual_math/src/graph/expr.rs
@@ -1,5 +1,6 @@
 //! Math expression.
 
+use std::collections::HashSet;
 use std::fmt::{Debug, Display};
 use std::ops::{Add, Div, Mul, Neg, Sub};
 
@@ -27,17 +28,20 @@ pub enum Expr {
 }
 
 /// An error that can occurs when the building of an [`Expr`] fails.
-pub struct BuildExprError(());
-
-impl Debug for BuildExprError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str("BuildExprError")
-    }
+#[derive(Debug)]
+pub enum BuildExprError {
+    /// The provided node id has not been found in the graph.
+    NodeNotFound,
+    /// The graph contains a loop.
+    Loop,
 }
 
 impl Display for BuildExprError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str("The provided socket didn't not exists in the graph")
+        match self {
+            BuildExprError::NodeNotFound => f.write_str("node not found"),
+            BuildExprError::Loop => f.write_str("the expression contains a loop"),
+        }
     }
 }
 
@@ -46,32 +50,56 @@ impl std::error::Error for BuildExprError {}
 impl Graph {
     /// Build the [`Expr`] from the specified node.
     pub fn build_expr_from(&self, node_id: NodeId) -> Result<Expr, BuildExprError> {
-        self.build_expr_from_inner(Some(node_id))
+        let mut tracelog = HashSet::new();
+
+        self.build_expr_from_inner(&mut tracelog, Some(node_id))
     }
 
     /// Build the [`Expr`] from the specified node.
-    fn build_expr_from_inner(&self, node_id: Option<NodeId>) -> Result<Expr, BuildExprError> {
+    fn build_expr_from_inner(
+        &self,
+        resolution_stack: &mut HashSet<OpNodeId>,
+        node_id: Option<NodeId>,
+    ) -> Result<Expr, BuildExprError> {
         let Some(node_id) = node_id else {
             return Ok(Expr::Unconnected);
         };
 
         match node_id {
             NodeId::Op(node_id) => {
                 let Some(node) = self.get_op_node(node_id) else {
-                    return Err(BuildExprError(()));
+                    return Err(BuildExprError::NodeNotFound);
                 };
 
-                match node.op() {
-                    Op::Unary(UnaryOp::Neg) => self.build_expr_unary(node.id(), Expr::Neg),
-                    Op::Binary(BinaryOp::Add) => self.build_expr_binary(node.id(), Expr::Add),
-                    Op::Binary(BinaryOp::Sub) => self.build_expr_binary(node.id(), Expr::Sub),
-                    Op::Binary(BinaryOp::Mul) => self.build_expr_binary(node.id(), Expr::Mul),
-                    Op::Binary(BinaryOp::Div) => self.build_expr_binary(node.id(), Expr::Div),
+                if !resolution_stack.insert(node_id) {
+                    return Err(BuildExprError::Loop);
                 }
+
+                let expr = match node.op() {
+                    Op::Unary(UnaryOp::Neg) => {
+                        self.build_expr_unary(resolution_stack, node.id(), Expr::Neg)
+                    }
+                    Op::Binary(BinaryOp::Add) => {
+                        self.build_expr_binary(resolution_stack, node.id(), Expr::Add)
+                    }
+                    Op::Binary(BinaryOp::Sub) => {
+                        self.build_expr_binary(resolution_stack, node.id(), Expr::Sub)
+                    }
+                    Op::Binary(BinaryOp::Mul) => {
+                        self.build_expr_binary(resolution_stack, node.id(), Expr::Mul)
+                    }
+                    Op::Binary(BinaryOp::Div) => {
+                        self.build_expr_binary(resolution_stack, node.id(), Expr::Div)
+                    }
+                };
+
+                resolution_stack.remove(&node_id);
+
+                expr
             }
             NodeId::Input(node_id) => {
                 let Some(node) = self.get_input(node_id) else {
-                    return Err(BuildExprError(()));
+                    return Err(BuildExprError::NodeNotFound);
                 };
 
                 Ok(Expr::Input(node.id()))
@@ -82,11 +110,13 @@ impl Graph {
     /// Build an unary operation expression.
     fn build_expr_unary(
         &self,
+        resolution_stack: &mut HashSet<OpNodeId>,
         node_id: OpNodeId,
         expr: impl FnOnce(Box<Expr>) -> Expr,
     ) -> Result<Expr, BuildExprError> {
         Ok(expr(Box::new(
             self.build_expr_from_inner(
+                resolution_stack,
                 self.connections
                     .get(node_id.input_socket_id(SocketIndex::A))
                     .map(|socket_id| socket_id.node_id),
@@ -97,19 +127,22 @@ impl Graph {
     /// Build an binary operation expression.
     fn build_expr_binary(
         &self,
+        resolution_stack: &mut HashSet<OpNodeId>,
         node_id: OpNodeId,
         expr: impl FnOnce(Box<Expr>, Box<Expr>) -> Expr,
     ) -> Result<Expr, BuildExprError> {
         Ok(expr(
             Box::new(
                 self.build_expr_from_inner(
+                    resolution_stack,
                     self.connections
                         .get(node_id.input_socket_id(SocketIndex::A))
                         .map(|socket_id| socket_id.node_id),
                 )?,
             ),
             Box::new(
                 self.build_expr_from_inner(
+                    resolution_stack,
                     self.connections
                         .get(node_id.input_socket_id(SocketIndex::B))
                         .map(|socket_id| socket_id.node_id),

diff --git a/examples/visual_math/src/graph/mod.rs b/examples/visual_math/src/graph/mod.rs
@@ -7,7 +7,7 @@ mod input;
 mod node;
 
 pub use self::connections::Connections;
-pub use self::expr::Expr;
+pub use self::expr::{BuildExprError, Expr};
 pub use self::id::{
     InputId, InputSocketId, IntoOutputSocketId, NodeId, OpNodeId, OutputSocketId, SocketId,
 };