Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Delaunay triangulation, Voronoi diagram generation, and Lloyd's relaxation nodes to the vector library, leveraging the delaunator crate. It also updates path-closing logic to prevent duplicate closing anchors for near-coincident endpoints. The review feedback highlights critical issues to address: correcting the perpendicular vector calculation in voronoi.rs to ensure unbounded cells project outward, clamping the relaxation iterations to prevent hangs from infinite or extremely large values, and enforcing a minimum threshold on the mesh weld tolerance to avoid division-by-zero errors on extremely small bounds.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let p0 = sites[previous]; | ||
| let p1 = sites[current]; | ||
| // Perpendicular to the hull edge `previous -> current`, pointing away from the hull interior. | ||
| let perpendicular = DVec2::new(p0.y - p1.y, p1.x - p0.x); |
There was a problem hiding this comment.
The perpendicular vector is calculated as DVec2::new(p0.y - p1.y, p1.x - p0.x). For a counter-clockwise hull, this points inward (towards the interior of the hull) instead of outward. This causes the unbounded Voronoi cells to be projected inward, resulting in completely incorrect cell geometry and clipping near the boundary. It should be changed to DVec2::new(p1.y - p0.y, p0.x - p1.x) to point outward.
| let perpendicular = DVec2::new(p0.y - p1.y, p1.x - p0.x); | |
| let perpendicular = DVec2::new(p1.y - p0.y, p0.x - p1.x); |
| let iterations = iterations.max(0.); | ||
| let whole_steps = iterations.floor(); |
There was a problem hiding this comment.
If iterations is extremely large or f64::INFINITY, whole_steps as u32 will saturate to u32::MAX (4.2 billion), causing the relaxation loop to run for a very long time and completely freeze/hang the editor. We should clamp iterations to a sensible maximum (e.g., 100.0) to prevent denial of service and ensure real-time performance, and also handle NaN values safely.
| let iterations = iterations.max(0.); | |
| let whole_steps = iterations.floor(); | |
| let iterations = if iterations.is_nan() { 0. } else { iterations.clamp(0., 100.) }; let whole_steps = iterations.floor(); |
| } | ||
|
|
||
| let diagonal = (max - min).length(); | ||
| if diagonal.is_finite() && diagonal > 0. { diagonal * 1e-6 } else { 1e-6 } |
There was a problem hiding this comment.
If the diagonal of the bounding box is extremely small, diagonal * 1e-6 can underflow to 0.0. A tolerance of 0.0 will cause division-by-zero in replace_with_polygons when calculating the quantization key, leading to NaN or Infinity coordinates that saturate to i64::MAX/i64::MIN and weld all vertices into a single point. Clamping the tolerance to a safe minimum like 1e-12 prevents this.
| if diagonal.is_finite() && diagonal > 0. { diagonal * 1e-6 } else { 1e-6 } | |
| if diagonal.is_finite() && diagonal > 0. { (diagonal * 1e-6).max(1e-12) } else { 1e-6 } |
No description provided.