@abdullah care to weigh in on this new feature?abdullah wrote: pinged by pinger macro
FYI, PR is https://github.com/FreeCAD/FreeCAD/pull/4420
@abdullah care to weigh in on this new feature?abdullah wrote: pinged by pinger macro
Yes. Believe it or not, I am wanting to look at this PR for over one month (maybe even two months). There is always something getting in the middle. Hopefully this weekend I will be able to. Thanks for reminding meKunda1 wrote: ↑Mon Apr 19, 2021 10:40 am@abdullah care to weigh in on this new feature?abdullah wrote: pinged by pinger macro
FYI, PR is https://github.com/FreeCAD/FreeCAD/pull/4420
First at all, my deepest apology for the unacceptably long delay in merging this PR. Thanks for your patience.tpavlicek wrote: ↑Tue Feb 09, 2021 10:48 pm For this feature I have created a pull request https://github.com/FreeCAD/FreeCAD/pull/4420. Please feel free to test it, comment or report any bugs.
I started a placeholder page at Sketcher Split. Can someone fill in the rest?Kunda1 wrote: ↑Sat Apr 24, 2021 1:10 pm git commit 4d6b1f3
Thanks to everyone involved!
Can we get a wiki documentation page as well ?
Abdullah, thank You for the review a valuable comments. Definitely I have no objections against the changes You have made, I am pretty much sure they are heading in the right way.abdullah wrote: ↑Sat Apr 24, 2021 1:40 pm After the PR merge, I have pushed a second commit with some changes. Most of it is just about renaming to make it easier to understand for others what the functions do. In one instance (swapInvolvedGeometry), I have moved the constraint specific functionality to the Constraint class. This way it is accessible to any client code.
I have not touched the split() function. I think it is possible to implement this function in more generic terms to avoid to have to write geometry specific code for every single geometry (the current trim() functionality may show the way). I am not a fan of the "do while(false)" idiom in c++, I think it is misleading from a code readability point of view (but I know that many others are of a different opinion, specially those coming from a C or Fortran background). In this case it is used to avoid "if" evaluations of the group and break from the loop at any time (through continue). However, addGeometry in practice never actually returns a negative value. The changes of properties could use move syntax instead of having to delete (this is also how trim works). As the code works and I do not have time at this moment, I decided not to touch the internals of the split function, which is something that can be done in the future if judged necessary or appropriate.
There is an issue I detected in my tests, which should be fixed. If an arc having a radius constraint is split, then the radius constraint is copied, while at the same time making the center point coincident and one endpoint. This leads to redundant constraints. My expectation is that this happens for diameter too. Nothing that cannot be solved by a direct deletion (or even by doing nothing at all if the auto-remove redundants mode is enabled).
Thanks!tpavlicek wrote: ↑Sun Apr 25, 2021 5:27 pm Abdullah, thank You for the review a valuable comments. Definitely I have no objections against the changes You have made, I am pretty much sure they are heading in the right way.
As for the comments, I was thinking about adding split() functionality also to the rest of conics (ellipse, parabola, hyperbola), so once I get to it, I will look how trim() is now implemented and refactor the code according to Your suggestions.
Once again thanks, kind regards,
Tomas
I've added mention this to the 0.20 release page but the documentation is still incomplete. I know this feature is still a work in progress nevertheless we should have some documentation explaining how to use it. Someone please step up, thank you!Kunda1 wrote: ↑Sat Apr 24, 2021 9:17 pmI started a placeholder page at Sketcher Split. Can someone fill in the rest?Kunda1 wrote: ↑Sat Apr 24, 2021 1:10 pm git commit 4d6b1f3
Thanks to everyone involved!
Can we get a wiki documentation page as well ?
Also where should Split be in the hierarchy of Sketcher Workbench?