DynamoDS / Dynamo

Open Source Graphical Programming for Design
https://dynamobim.org
Other
1.71k stars 628 forks source link

Curve.SplitByPoints discards segements of polycurves past the split point #6581

Closed mccrone closed 2 years ago

mccrone commented 8 years ago

If this issue is with Dynamo for Revit, please post your issue on the Dynamo for Revit Issues page.

If this issue is not a bug report or improvement request, please check the Dynamo forum, and start a thread there to discuss your issue.

Dynamo version

1.0

Operating system

Windows 7

What did you do?

polycurvesplit

What did you expect to see?

two poly curves should be returned which if joined back together would create the original polycuve

What did you see instead?

All segments of the polycurve that are wholly past the split point (parameter values are greater) are discarded.

dimven commented 8 years ago

How about the following temporary workaround:

2016-05-10_17-54-35

mccrone commented 8 years ago

@dimven nice! My workaround was a little less elegant: I reversed the curve, split that, and took the first result

kronz commented 7 years ago

@mccrone @dimven @aparajit-pratap I think the bug is a little more subtle. Curve.SplitByPoints is plural, it wants a set of splitting pointS. The function returns the polycurve segments that occur BETWEEN split inputs. So if you add a point at the end of your polycurve, you get the whole collection of Polycurve. The weird thing in this appears to me that it will ALWAYS return the segments that occur between the start of the polycurve and the first point defined. But the result is actually that you will ALWAYS get ALL the reulting polycurves if you add a point at the end of your polycurve 2016-12-16_1025 2016-12-16_1020

kronz commented 7 years ago

So, my question is should the proper behavior be that it ALWAYS assumes that you want the pieces between 0 and 1 parameters on the polycurve, or ONLY return the pieces that exist between defined splitting points

dimven commented 7 years ago

@kronz Seems like this has been improved in 1.2 :)

dimven commented 7 years ago

@kronz actually I take that back. Reran it again on a particular file that was failing for me, and I get the same issue as before. I've serialized the file for easy distribution. You can save as from the below link:

https://raw.githubusercontent.com/dimven/SpringNodes/master/Issues/6581%20polycurve%20split.dyn

I'm not sure if it's because the curve is closed or because of units but I'm getting the following warning on multiple parts of the curve:

dynamosandbox_2016-12-17_00-09-09

I'm trying to cut the curve with different pairs of points.

mccrone commented 7 years ago

@kronz @dimven splitting polyCurves should have the same behavior for splitting singular curves. The problem is that it's not consistent now.

inconsistentsplitting

Dyn: https://www.dropbox.com/s/t33pbc12v96l1lb/polyCurveSplit.dyn?dl=0

kronz commented 7 years ago

thanks @mccrone , that's the clearest description of the issue yet. @aparajit-pratap what do you think, any reason split of a polycurve should be any different than split of an arc, line, ellipse, nurbs?

aparajit-pratap commented 7 years ago

@mccrone @kronz I agree polycurves should behave like the rest of the curves. There are a number of issues with polycurves as a lot of the algorithms were implemented in Dynamo from scratch (ASM doesn't support them). Lev Lipkin had put in a lot of thought and effort to devise these algorithms. I have been tracking the issues and we'll need to allocate some time to revisit these methods. Further some of the curve methods do not have corresponding implementations for polycurves yet, such as Trim methods.

ParametricMonkey commented 7 years ago

@aparajit-pratap any update on this? It would be good to get implemented.

Amoursol commented 4 years ago

Thank you for the submission of this Issue above - We very much do appreciate your time taken to look to improve Dynamo and help it grow and evolve into the future.

In order to better serve the active Dynamo user base and the evolving nature of Dynamo, the Dynamo team has made the decision to close any issue that hasn't had activity since 1st January 2019. This doesn't mean that these issues will not be addressed, but just that they are not being actively worked on as they do not align with our current Dynamo Public Roadmap.

If this issue is still relevant to you and your workflows, please do re-submit in a new Github Issue and link to this closed issue for historical context.

Many thanks, The Dynamo Team

aparajit-pratap commented 2 years ago

Fixed in polycurve feature branch.