DynamoDS / Dynamo

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

Loft Form Fails to Create Surfaces from Single Loops of Curves #265

Closed s-overall closed 11 years ago

s-overall commented 11 years ago

This has been quite a source of angst for me lately. When working with a loop of planar curves, the Loft Form node always fails to create single surfaces in the massing environment. I've used quite a few different setups to try to make it work, but so far I haven't had any luck.

error1 error2

LevL commented 11 years ago

I think list of curves is interpreted by Loft Form and loft between profiles each consisting of one curve. Thus Loft Form fails. I tried to use List of 1 List of Curves, which I believe should be interpreted as one profile and make surface. However the list still got flatten and Loft Form fails. I am looking into that. Meanwhile please consider workaround of making loft form between 2 profiles (2 lists) or extrusion and getting desired face by using node "Faces of Solid Along Line". Thanks.

LevL commented 11 years ago

I pushed fix for the case of multiple profiles and for the case of profile(s) with two or more curves.

If list of curves is the input it is treated as loft through curves, not as curves forming one profile.

In other cases please use lists of lists.

For multiple profiles and for case of profiles with two or more curves use list of lists even if this is list of 1 list like in posted example.

Thanks for alerting for this bug!

s-overall commented 11 years ago

Hey, the fix works as expected to create single surfaces, but if I have a list of lists of planar curves and I want to loft single surfaces on each loop instead of lofting a solid, there doesn't seem to be a way to do it. I would have expected to be able to do it the same way as with a single surface, but using a map node, but it isn't quite working, as seen below:

loft surface bug

s-overall commented 11 years ago

On second thought, maybe I don't need the list node there, but the loft form node is still returning an error saying "cannot loft form"

LevL commented 11 years ago

I see: because list of lists is considered as list of curve loops, some functionality with lacing lists got disabled. Unfortunately we could not have this both ways. I think we need to move Loft Node to use list of Curve Loops to avoid this confusion.

Steell commented 11 years ago

@s-overall The reason the Map node isn't working in that sample is because you're not passing a Function into the first argument of map. Instead, Dynamo is is trying to pass the List node as a function to the Loft Form node, which is causing the error (the list input expects a list, not a function).

To fix this, you can either:

  1. Use the Compose node to accomplish what you'd like: connect the List node to the first input and then the Loft Form node (with nothing connected to the list port) to the second input. The Compose node taks a function f(x) and a function g(x) and returns a function that performs g(f(x))--pass the input to the first function, then pass the result to the second function.
  2. Collapse the Loft Form node, the List node, and the two Boolean nodes into new Custom Node, with 1 Input node that's connected to the the list port of the Loft Form node.

(This is irrelevant to the actual bug, I just want @s-overall to know that what he's trying to do is correct in theory, Dynamo just isn't doing what he thinks it's doing).

LevL commented 11 years ago

On the subject of lists as in-ports. Without shortcuts expected input to Loft Form node is list of lists of ref curves. So you seems to need extra list to make list of Loft Forms. I plan to change back to lacing List of List of List case, so it would be handled as for normal nodes. The shortcut form of input "list of curve elements" is still troublesome as case of List of List of ref curves will still be considered as request for one Loft Form. I'll post when list of list of lists fix is in (if it does not break anything). If making lists of Forns still not working, please email corrected dyn file to lev.lipkin@autodesk.com (inc. fix Stephen suggested for map node). Thanks! I pushed to master handling of list of list of lists a few minutes ago.

LevL commented 11 years ago

I added node "Planar Ref Curve Chain" to represent each profile and changed Loft Form node to accept List of such Chains. That should resolve confusion between lacing and list of lists as one input. The "list" in-port of Loft Node now says:,"A list of profiles for the Loft Form. The recommended way is to use list of Planar Ref Curve Chains, list of lists and list of curves are supported for legacy graphs." Let me know if there are problems with this. Thanks.

s-overall commented 11 years ago

Thanks for all the updates. I was looking at the Planar Ref Curve Chain node this morning, but I can't seem to get it to output anything. I am always getting an error saying "This curve will make the loop not contiguous". I am quite certain that the loop of curves I am using is planar and closed, but they aren't being input in any particular end-to-end order.

Also, if I attach a watch node to the "Planar Ref Curve Chain" node while I am getting this error, Dynamo crashes.

LevL commented 11 years ago

Yes, end-to-end order is important as Curve Loop code is doing checking. Unfortunately there is no flip curve method in current API, so I could not flip the curves based on end matching. Could you match ends and try again please?

I emailed issue with crash, which might be related to new class used or to exception, not sure.

I checked and successful node does not crash Watch node: image

LevL commented 11 years ago

I pushed change a few minutes ago which does checking in Dynamo, it relies on CurveLoop Revit API methods only to get plane of the curve (which is not line). So hope your test could move forward. Thanks!

s-overall commented 11 years ago

Hey, thanks for all the updates, I have been watching this closely. Unfortunately it still isn't working for me. The "Planar Ref Curve Chain" is working as expected, but they aren't being lofted individually with the loft node.

Consider the following workflow: loft_initialcond

Where this is the desired result: desiredresult

Trying to plug the first, single list of curve arrays into the "Loft Form" produces a lofted solid: loft_singlelist

And plugging a list of lists of individual curves into the "Loft Form" node returns an error: loft_listoflists

And mapping the list of lists of individual curves lofts one surface, but fails on the second and returns a different error: loft_listoflistsmap

LevL commented 11 years ago

Could you please email me dyn file which is not working to lev.lipkin@autodesk.com? Thanks.

LevL commented 11 years ago

Pushed into master 2 more fixes: list of lists of planar chains produces list of lofts (at least in my test example) and avoided dictionary exception by testing if key is there or not.

Not sure if moving lines with list of lists preserves ids of forms on rerun.

Hope you could get to this after today's fix. Thanks!

s-overall commented 11 years ago

Awesome, I've had to put this bit of experimentation to the side for a few days. I just tried it out again and it works... BUT when I add more loops to the "Planar Ref Curve Chain" node, it breaks no longer passes through that node until I restart Dynamo. curveadditionerror1 curveadditionerror2

Otherwise, it all seems to be working as expected. I can manipulate the existing curves within Dyanmo and it knows to update the lofted surface in Revit. It is getting close to what I am hoping to achieve now. Thanks for working so hard on this issue.

LevL commented 11 years ago

Good news! Thanks. I wonder if you could email me dyn file with graph which is not working.

s-overall commented 11 years ago

Hmmm.... I might have to get back to you on that. This particular script if interfacing with another program and would be pretty incomprehensible by itself. I'll have to put together another script that can duplicate this error.

LevL commented 11 years ago

Thanks for finding the issue. Just pushed fix into master.

frenji commented 11 years ago

Hi there

is it possible to perform multiple loft surfaces with list of "list of curves" created from points imported from a csv file ?

I was trying to recreate some surfaces made in rhino.....

frenji commented 11 years ago

Please find the images showing what i was trying to do.....

LevL commented 11 years ago

Use Planar Ref Curve Chain node instead of "list of curves". Each of your curves (and each raw of your points) would need to be planar.

If your curves are not planar, consider using "Face Through Points" node enabled only for Vasari 2014.You might also need "Replace Faces" to get Solid with such face, also enabled only for Vasari 2014. Result of those nodes could be open in Revit 2014 (assuming that Solid is put into Freeform element).

Thanks.

frenji commented 11 years ago

Hi Lev, i was using dynamo on revit 2014 and i dont have planar curves...these are open curves interpolated through list of points .....i was able to loft these curves to make one surface, but what i want to do is to loft list of curves to get multiple independent surface ....is there a better way to achieve this ?

frenji commented 11 years ago

The objective behind this exercise was to recreate multiple non planar (non trimmed) surfaces from rhino inside revit...the task involed sending csv files through grasshopper...This was then imported into dynamo and interpolated curves drawn through them, i was able to split them as list of curves for each surface in dynamo, but where i am stuck is when i try to loft these seperated list of curves using the loft form

s-overall commented 11 years ago

Sounds quite a bit like what I have been doing (parsing a csv sent from rhino in order to reconstruct lines and surfaces). I ran a quick test on lofting lists of lists of open curves, and it doesn't seem to work, and lofting a full list of curves is returning funny results, though I admit I don't know exactly what order my curves are in in this example nurbloft1 nurbloft2 nurbloft3

.

LevL commented 11 years ago

I wonder if requirement stated in RevitAPI new Loft Form creation method for chain of curves to be planar is not really being checked. I will check if this requirement is not actually causing exception when violated. And if true will change node to "chain of curves" and remove checks for planarity.

s-overall commented 11 years ago

I do want to add that creating planar surfaces from closed loops is working very nicely now.

LevL commented 11 years ago

Confirmed my guess: API NewLoftForm works for nonplanar profiles. Just pushed to master the removal of the check (5:35pm EST). Node get renamed to "Ref Curve Chain".

However I still think that "Face Through Points" with Vasari 2014 is better fit for the task :-)