Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch Issue #1360 #1361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kirevdokimov
Copy link

@kirevdokimov kirevdokimov commented Aug 27, 2024

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • 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
Copy link
Contributor

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

@hrgdavor hrgdavor requested a review from z3dev August 27, 2024 14:23
@z3dev z3dev requested a review from platypii August 29, 2024 23:35
@z3dev
Copy link
Member

z3dev commented Aug 29, 2024

@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
Copy link
Author

kirevdokimov commented Sep 4, 2024

@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
Copy link
Member

z3dev commented Sep 4, 2024

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
Copy link
Contributor

platypii commented Sep 6, 2024

The fix seems fine, but I haven't yet been able to reproduce the original error on master?
#1360 (comment)

@z3dev
Copy link
Member

z3dev commented Nov 9, 2024

@kirevdokimov Can you supply a small test case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants