iShape-Rust / iOverlay

Boolean Operations for 2D Polygons: Supports intersection, union, difference, xor, and self-intersections for all polygon varieties.
https://ishape-rust.github.io/iShape-js/
MIT License
42 stars 4 forks source link

Issue with calculation of `union_rect` in `FloatOverlay::build_graph_with_solver` when `clip_paths` is empty #4

Closed MatchaChoco010 closed 4 months ago

MatchaChoco010 commented 4 months ago

I found a issue when using ShapeType::Subject in FloatOverlay and would like to report it.

I added only ShapeType::Subject paths for FloatOverlay and tried to do graph.extract_shapes with OverlayRule::Subject, but the calculation does not seem to be correct.

overlay.add_paths(&shape, ShapeType::Subject);  
let graph = overlay.build_graph(FillRule::NonZero);
let shapes = graph.extract_shapes(OverlayRule::Subject);

In the library code, src/core/float_overlay.rs calculates union_rect from subj_rect and clip_rect, but there seems to be a problem in this part. When subj_rect and clip_rect are created with F64Rect::with_shape, if empty paths are inserted, rects with -f64::MAX min and max values are created. Calculating union_rect based on these all -f64::MAX rects will result in an incorrect rect, and since PointAdapter is calculated based on it, the process will be incorrect.

To fix the above problem, it seems that if self.subj_paths or self.clip_paths is empty, it is necessary to create a PointAdapter by calculating the rect with only the other one.

More to the point, it might be better to return Result Err if build_graph is called with both self.subj_paths and self.clip_paths empty.

NailxSharipov commented 4 months ago

Good job to find this issue.

Yes correct library will not supposed to work with paths less 3 points. We should do something with it. I saw your issue with fixing F64Rect constructor let me explaining my thoughts there. For this issue a create new empty correctness tests at float_overlay_tests.rs

NailxSharipov commented 4 months ago

fixed in 1.2.3

NailxSharipov commented 4 months ago

+