Patch Issue #1360 - Githubissues

jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.

https://openjscad.xyz/

MIT License

2.65k stars 514 forks source link

Patch Issue #1360 #1361

Open kirevdokimov opened 1 month ago

kirevdokimov commented 1 month ago

All Submissions:

[x] Have you followed the guidelines in our Contributing document?
[x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
[ ] Does your submission pass tests? (did not run tests yet)

It works smoothly for a sample case from the issue and for my complex real case

hrgdavor commented 1 month ago

I am not very good at reviewing, but I did run tests locally, and they passed

z3dev commented 1 month ago

@kirevdokimov The changes are straight forward, and probably fix the special case. But I'm wondering if there's another issue with the algorithm, or the union is returning some strange polygons.

im not sure that I'll have time to research further, so I added @platypii to the review.

kirevdokimov commented 4 weeks ago

@z3dev Retesselation works perfectly fine with this one issue. It is not a specific case. There are cases with union, subtract, and anything else that uses retesselation. The only problem with commenting retesselation out from these functions is that some triangles are left in the air.

I will let you know if I find something else, but this issue is obvious, easy to fix, and does not affect anything else badly.

z3dev commented 4 weeks ago

The only problem with commenting retesselation out from these functions is that some triangles are left in the air.

I was suspecting something like this. Hopefully, @platypii will have some time to investigate as the simple design is producing a nice test case.

FYI, the fix looks fine.

platypii commented 3 weeks ago

The fix seems fine, but I haven't yet been able to reproduce the original error on master? https://github.com/jscad/OpenJSCAD.org/issues/1360#issuecomment-2329728567