Skip to content

Smoother augment_profile#10

Open
jacekkopecky wants to merge 5 commits intoopenscad:masterfrom
jacekkopecky:smoother-augment-profile
Open

Smoother augment_profile#10
jacekkopecky wants to merge 5 commits intoopenscad:masterfrom
jacekkopecky:smoother-augment-profile

Conversation

@jacekkopecky
Copy link
Copy Markdown

@jacekkopecky jacekkopecky commented Dec 25, 2018

augment_profile in skin.scad inserts new vertices in a profile so that the result has a given number of vertices. Currently, the order of insertions is not specified, and in some cases the new vertices cause distortion.

This profile (in context of extrusion.scad) demonstrates the issue: the cylinder is visibly distorted:

// profiles that have different vertex counts
skin([
	transform(translation([0,0,0]), circle($fn=70,r=1)),
	transform(translation([0,0,1]), circle($fn=75,r=1)),
	transform(translation([0,0,2]), circle($fn=100,r=1)),
]);

This image shows the distortion.
screen shot 2018-12-25 at 14 50 39

The example above is artificial, but the issue comes up when combining profiles that you naturally produce with different vertex counts.

This pull requests introduces insert_extra_vertices_1 which distributes new vertices uniformly around the length of the profile, resulting in much smaller distortions.

I'll be happy to polish the code if need be.

@jacekkopecky
Copy link
Copy Markdown
Author

Hi, do we have any maintainers here please? I know no commits happened in over 3 years, but maybe?

@OskarLinde
Copy link
Copy Markdown

Thanks for submitting. This looks like a reasonable approach. (note that both this and the original code are quite inefficient – there are better ways to do it)

Have you verified that none of the samples degrade by taking this approach?

@jacekkopecky
Copy link
Copy Markdown
Author

Hi, thanks; I'll do that but need a few more weeks before I get to it due to $dayjob.

@jacekkopecky
Copy link
Copy Markdown
Author

Hi, yes, I can now confirm that all the other examples in this repo that use skin work correctly.

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

Successfully merging this pull request may close these issues.

2 participants