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

Wire.trim doesn't really work #676

Closed
MatthiasJ1 opened this issue Aug 27, 2024 · 3 comments
Closed

Wire.trim doesn't really work #676

MatthiasJ1 opened this issue Aug 27, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@MatthiasJ1
Copy link
Contributor

Anything but the simplest case fails. For example

w = Rectangle(1,1).wire()
w = w.trim(0.5, 1)

gives RuntimeError: Invalid trim result.

Changing the range to (0, 0.5) doesn't error but gives a wrong result. Trimming a wire composed of many small edges (which works everywhere else) results in floating precision errors.

Also, ideally something like wire.trim(0.5, 1.5) should return the same wire with FirstParameter/LastParameter changed for closed wires (this currently works for trimming a circle wire but I haven't been able to get any other wire to trim for further testing).

@gumyr gumyr added the bug Something isn't working label Aug 29, 2024
@gumyr gumyr added this to the Not Gating Release 1.0.0 milestone Aug 29, 2024
gumyr added a commit that referenced this issue Aug 29, 2024
@gumyr
Copy link
Owner

gumyr commented Aug 29, 2024

This problem is mostly fixed now. There is still strange behavior on Arm based Macs with this test case:

        p = RegularPolygon(10, 20).wire()
        t7 = p.trim(0.1, 0.2)
        self.assertAlmostEqual(p.length * 0.1, t7.length, 5)

which works on all other systems. Looks like there is precision problem with one of the points on the Wire is not on the appropriate Edge.

@MatthiasJ1 is this the "many small edges" problem you noted and are you testing with an Arm based Mac?

@jdegenstein
Copy link
Collaborator

Arm behavior is fixed now in a recent commit yesterday.

@gumyr gumyr closed this as completed Aug 30, 2024
@MatthiasJ1
Copy link
Contributor Author

Sorry for the delay, just confirmed this is working, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants