Fix non-driving constraints parameters after solving not being updated
This PR implements few methods to fix issue #5 (Pulling updated values for non-driving constraints)
I've added few Typescript helpers so it becomes easier to implement parameter offsets in geom_params.ts.
Also added a generic handler for updating constraints values, this is helpful since different constraints have different properties that they should be updated (l2l_angle_ll have angle, coordinate_x have x, etc).
There is a for loop to update each of those properties, however I didn't spent too much time finding a constraint that would apply this case.
Let me know if you think this is overengineering.
- Added unit tests.
- Bump
vitestto current latest version (getting segmentation fault error on project's version)
Constraints implemented:
- [X] CoordinateX
- [X] CoordinateY
- [X] L2LAngle_LL
- [X] L2LAngle_PPPP
- [X] P2PAngle
- [X] P2PDistance
- [X] CircleRadius
- [X] ArcRadius
Constraints not working
Those constraints keep changing other primitives and constraints's values (driving: false being "ignored")
I'll open another issue for those.
- [ ] CircleDiameter (Behaves like CircleRadius,
diameter = radiusinstead ofdiameter = radius * 2) - [ ] ArcDiameter (Behaves like ArcRadius,
diameter = radiusinstead ofdiameter = radius * 2) - [ ] C2CDistance (
driving: falsestill changes geometry) - [ ] C2LDistance (
driving: falsestill changes geometry) - [ ] P2CDistance (
driving: falsestill changes geometry) - [ ] ArcLength (
driving: falsestill changes geometry)
Considering the buggy constraints, I'm gonna look at it later - from the first sight it seems like bugs in planegcs.
Hello, sorry for the delay. However - I've looked into the issue and I realized there is a problem in the parameter offset lookup.
Currently, the p-parameters (for planegcs) of a constraint are only added when they do not reference an existing primitive property or a sketch parameter. Therefore, the parameter offsets of a constraint aren't always the same.
Example: When pushing a constraint with two parameters param1: { o_id: '1', prop: 'x' }, param2: 100, only the second param is pushed as p-param to the planegcs, therefore its offset is 0. Thus, the constant object constraint_properties_and_offsets you added is not valid, because the offsets can vary.
I'll try to implement it using an extra dynamic map object. Sorry, should've realized this earlier.
Ok, I changed the offsets lookup implementation. However, there are still issues with the constraints you mentioned. I also found another bug in the wrapper when passing the primitive numbers, this is fixed now.
The CircleDiameter and ArcDiameter constraints are internally implemented using the Proportional constraint. This works properly when one of the parameters is fixed (e.g. sketch param - see the added test in the last commit). When the parameter isn't fixed (which is by default for the geometries), then the constraint isn't updated properly (it is updated as if the ratio is 1). I would need to dig further to find out why this is so.
In FreeCAD the nondriving circle diameter works ok, so there might be something I'm missing.
Considering the constraints that influence the geometry even when non-driving, I assume they are incorrectly implemented. Also I haven't found them inside FreeCAD, so no way how to check/compare.
Hey! Thanks for that, makes total sense!
I've also found some issue in l2l_angle_ll, but let me first test with the updated changes so I can describe it better. But basically it is not respecting the length of the lines, it acts as if the lines extends infinitely thus causing "wrong" readings.
I will check the behaviour in FreeCad to confirm
Ok, let me know. But I would guess the constraint should regard the lines as lines and not line segments (-> not regard the line length)..?
Yes I thought about that too, but it behaves different than in FreeCAD and it is really confusing. But stepping that aside, I'm getting a lot of issues now with this new changes on simple constraints and i.e. dragging lines which worked fine before.
RuntimeError: Out of bounds memory access (evaluating 'invoker(fn, thisWired, arg0Wired)')
I'll investigate further but if you know what it might be off the top of your head 🤔
Hmm.. gonna look at it.
Ok don't worry much for now, I traced it back to a P2P_Angle constraint being created initially with angle: 0 (or close to 0) and messing up with everything and throwing that error like crazy.
Ok, I still don't understand completely though, so is it a bug in the wrapper that should be fixed?
I'm not sure yet, I can replicate it somewhat easily, pretty sure it is related to P2P_Angle when it is created with an angle that doesn't matches the correct one (e.g. two lines 45 degrees) for the first iteration when driving: true, it seems like it updates correctly X and Y for one of the lines, then anything else I do just breaks everything.
My workaround for now is: create the P2P_Angle with driving: false, solve so I can get the current value (the whole reason this PR was created), then create the constraint with that value, now everything works as expected.
I'll try to write an unit test showcasing this
Found the issue, my implementation relies on clear_data then re-pushing everything to do dragging and sketching operations, as we have this new nondriving_constraint_params_order map, it wasn't being cleared between iterations and it was growing up each step for every property in the constraints. Clearing it solved it
Ah, ok ;) thanks for the fix. Have you explored the issue with l2l_angle_ll yet?
Hi, yes actually, just checked. It is not an issue with planegcs/wrapper itself, I thought PlaneGCS would automatically pick start/end points of lines to correctly account for lines's starting and ending so that the angle is "correct" intuitively.
But I found that FreeCAD does this way before PlaneGCS, so it is all good! 😄
Ok so I guess the state is now that we can merge this PR and I'll keep the changes in the README.md about the constraints that don't work properly now as nondriving.
hey @thes01 can we merge this one? 😄
Yea I'm sorry, will do by tomorrow.