From b2305932d928719e79cd57fe9dfdcc356da5ab3d Mon Sep 17 00:00:00 2001
From: Rbanh <Rbanh15@gmail.com>
Date: Sat, 6 Jun 2026 06:38:37 -0400
Subject: [PATCH] Expand brush validation coverage

---
 crates/shared/src/brush_math.rs | 94 +++++++++++++++++++++++++++++++++
 docs/editor/brushes.md          |  2 +-
 2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/crates/shared/src/brush_math.rs b/crates/shared/src/brush_math.rs
index 7be1e2a..731b900 100644
--- a/crates/shared/src/brush_math.rs
+++ b/crates/shared/src/brush_math.rs
@@ -3,6 +3,7 @@
 use bevy::prelude::*;
 
 use crate::{BrushDesc, ComponentInstanceId};
+use std::collections::HashMap;
 
 const EPSILON: f32 = 0.0001;
 
@@ -105,6 +106,18 @@ pub fn validate_brush(brush: &BrushDesc) -> BrushValidationReport {
                 "Face area is zero or degenerate.",
             );
         }
+        if let (Some(plane_normal), Some(geometry_normal)) = (
+            face.plane.normal.try_normalize(),
+            polygon_normal_3d(&face.vertices),
+        ) {
+            if plane_normal.dot(geometry_normal) < -0.5 {
+                report.push(
+                    BrushDiagnosticSeverity::Error,
+                    Some(face.id.clone()),
+                    "Face plane normal is inverted relative to its vertices.",
+                );
+            }
+        }
         if !face.uv_scale.is_finite() || face.uv_scale.cmpeq(Vec2::ZERO).any() {
             report.push(
                 BrushDiagnosticSeverity::Warning,
@@ -114,6 +127,15 @@ pub fn validate_brush(brush: &BrushDesc) -> BrushValidationReport {
         }
     }
 
+    let non_manifold_edges = count_non_manifold_edges(brush);
+    if non_manifold_edges > 0 {
+        report.push(
+            BrushDiagnosticSeverity::Error,
+            None,
+            format!("Brush has {non_manifold_edges} open or non-manifold edge(s)."),
+        );
+    }
+
     report
 }
 
@@ -212,6 +234,54 @@ fn polygon_area_3d(vertices: &[Vec3]) -> f32 {
     area
 }
 
+fn polygon_normal_3d(vertices: &[Vec3]) -> Option<Vec3> {
+    if vertices.len() < 3 || !vertices.iter().all(|vertex| vertex.is_finite()) {
+        return None;
+    }
+    let origin = vertices[0];
+    let mut normal = Vec3::ZERO;
+    for index in 1..(vertices.len() - 1) {
+        normal += (vertices[index] - origin).cross(vertices[index + 1] - origin);
+    }
+    normal.try_normalize()
+}
+
+fn count_non_manifold_edges(brush: &BrushDesc) -> usize {
+    let mut edge_counts = HashMap::<(QuantizedVertex, QuantizedVertex), usize>::new();
+    for face in &brush.faces {
+        if face.vertices.len() < 3 || !face.vertices.iter().all(|vertex| vertex.is_finite()) {
+            continue;
+        }
+        for index in 0..face.vertices.len() {
+            let a = QuantizedVertex::from(face.vertices[index]);
+            let b = QuantizedVertex::from(face.vertices[(index + 1) % face.vertices.len()]);
+            if a == b {
+                continue;
+            }
+            let edge = if a <= b { (a, b) } else { (b, a) };
+            *edge_counts.entry(edge).or_default() += 1;
+        }
+    }
+    edge_counts.values().filter(|count| **count != 2).count()
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
+struct QuantizedVertex {
+    x: i64,
+    y: i64,
+    z: i64,
+}
+
+impl From<Vec3> for QuantizedVertex {
+    fn from(value: Vec3) -> Self {
+        Self {
+            x: (value.x / EPSILON).round() as i64,
+            y: (value.y / EPSILON).round() as i64,
+            z: (value.z / EPSILON).round() as i64,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -270,4 +340,28 @@ mod tests {
             .iter()
             .any(|diagnostic| diagnostic.severity == BrushDiagnosticSeverity::Warning));
     }
+
+    #[test]
+    fn reports_inverted_face_normals() {
+        let mut brush = BrushDesc::default();
+        brush.faces[0].plane.normal = -brush.faces[0].plane.normal;
+        let report = validate_brush(&brush);
+        assert!(!report.is_valid());
+        assert!(report
+            .diagnostics
+            .iter()
+            .any(|diagnostic| diagnostic.message.contains("inverted")));
+    }
+
+    #[test]
+    fn reports_open_edges() {
+        let mut brush = BrushDesc::default();
+        brush.faces.pop();
+        let report = validate_brush(&brush);
+        assert!(!report.is_valid());
+        assert!(report
+            .diagnostics
+            .iter()
+            .any(|diagnostic| diagnostic.message.contains("non-manifold")));
+    }
 }
diff --git a/docs/editor/brushes.md b/docs/editor/brushes.md
index 7130d72..b3ca074 100644
--- a/docs/editor/brushes.md
+++ b/docs/editor/brushes.md
@@ -35,7 +35,7 @@ These operations validate selected brushes before applying results and validate
 ## Current Limits
 
 - Only convex authored faces are supported by the mesh builder.
-- Brush diagnostics currently focus on face validity, plane normals, finite vertices, duplicate vertices, degenerate area, and UV scale warnings.
+- Brush diagnostics currently cover face validity, plane normals, inverted face normals, finite vertices, duplicate vertices, degenerate area, open/non-manifold edges, and UV scale warnings.
 - Subtractive brush markers are stored but not automatically evaluated.
 - Clip split application and arbitrary-face CSG remain future roadmap work.
 - Imported source-material subasset refs persist for future material-preserving CSG, but only face refs with loadable source paths can drive brush material hydration today.