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

weights used can cause geometry problem? #2

Open
schuderer opened this issue Sep 15, 2018 · 5 comments
Open

weights used can cause geometry problem? #2

schuderer opened this issue Sep 15, 2018 · 5 comments

Comments

@schuderer
Copy link

Not sure exactly why this is happening, but when I create a tightly-knotted object, re-import as STL and use it for 2D projections, OpenSCAD crashes with an assertion error. The problem goes away when I change the line
weight = [-1, 8, 8, -1] / 14;
to the weights which Oskar used in his original forum post (http://forum.openscad.org/smooth-3-D-curves-tp7766p7769.html):
weight = [-1, 9, 9, -1] / 16;

Any idea if it is possible that this change has led to the math not checking out completely any more?

@teejaydub
Copy link
Owner

This is a tough one; my research indicated that the only requirement was that the weights are symmetric and they sum to the denominator. And I tweaked Oskar's because I had problems rendering with OpenSCAD as well (but I'm not certain it was the same problem you're having). Also, the "new" weights felt more aesthetically useful - it seemed like the curves went closer to where I intuitively wanted them to go.

It's possible that having the weights sum to a power of 2 is important in some way. I don't think any values there should result in an assertion error in OpenSCAD, though.

My sense is that non-manifold surfaces can result with complicated objects, regardless of this, and I wish I had a great solution to that problem.

Also, I've made enough models with this set of weights that changing them would cause me some trouble. We could add a global option, though - that seems preferable to forking it!

@schuderer
Copy link
Author

schuderer commented Sep 24, 2018

Thanks for looking into this. I agree that making the weights tweakable would be nice. While the problem might be a really spurious thing, you mention tweaking the values for aesthetics, which sounds like another good argument. :)

Now, I wonder what would be the simplest way to actually do that (have been using OpenSCAD for about a month now). I tried just re-defining the weights in my sketch, but unfortunately the lib function does not see/respect that. I didn't want to overwrite your lib function which would have been copy-pasta from the lib save for other weight variables. Maybe there's some weirdness possible with including the lib at the end of the script, but I'm honestly not sure how to change library behaviour from the outside in a language that doesn't know variables. Maybe simply by introducing weight parameters (or one weights array of arrays) with default values?

Edit: I also wanted to say thank you for providing the library! If you're interested in what I'm using it for: I've been using it for 3D-printable data visualization (https://github.com/schuderer/printable-time-series, and applied to James Bond movies here: https://www.thingiverse.com/thing:3097193)

@teejaydub
Copy link
Owner

Yes, I've just experimented a bit, and I can't find a great option either! It's possible if include were recommended instead of use, a variable defined before the include could influence things. But I also haven't been able to reproduce the aesthetic problems I was having with the original weights either.

So, I'm somewhat inclined to just change back to those original weights and see what it breaks. I think I'll do it on a branch for now.

teejaydub added a commit that referenced this issue Oct 9, 2018
@schuderer
Copy link
Author

schuderer commented Oct 11, 2018

Thanks for your answer! I think the easiest way would be to change line 237 in spline.scad from
function interpolateOpen(path, n, i) =
to
function interpolateOpen(path, n, i, weight=weight, weight0=weight0, weight2=weight2) =

It can be used as it was by existing code and uses the default weights when only the parameters path, n and i are given. That way, your code should keep working, and someone else using the library (i.e. me :) ) can optionally provide their own weights. What do you think?

@teejaydub
Copy link
Owner

That would work fine - but it would require all those arguments to be duplicated in most of the functions and modules in spline.scad. That wouldn't be the end of the world, but it's unnecessary if I can't find any reason not to go back to the original weights.

I've got that committed on the beta branch; I'll do a little more testing and then maybe merge it to master and see what comes up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants