-
Notifications
You must be signed in to change notification settings - Fork 9
Rounded Cube and Face Triangulation #4
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
base: master
Are you sure you want to change the base?
Conversation
function to subdivision.scad
subdivision.scad
Outdated
| [for (i=[start:end]) list[i]] | ||
| ; | ||
|
|
||
| function vsum(v,i=0) = len(v) > i ? v[i] + vsum(v, i+1) : [0,0,0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added vsum to the core library so we shouldn't need it here.
subdivision.scad
Outdated
| ; | ||
|
|
||
|
|
||
| function winding_direction(points,face)= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same as calculating the signed area? Maybe we can modify signed_area to optionally accept a face parameter instead of having two different functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's doing something very similar. (Well, it should be doing something very similar. I think that might be incorrect as submitted.)
subdivision.scad
Outdated
| n > 1 ? subdivide_faces(n-1, [allpoints, faces]) : n > 0 ? [allpoints,faces] : poly; | ||
|
|
||
| //Utility function for splitting circular vectors. | ||
| function range (list,start,end)= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this function is a little too general I think. And I'd call it more of a vector "slice", than a "range".
How about "slice_circular" or "vslice_circular"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, maybe this should go in "utilities" section of functional.scad.
Do you think it would be useful in other circumstances, or do you feel its just a very niche thing that would not be needed again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like a general thing, but I don't have a lot of experience developing this kind of thing.
subdivision.scad
Outdated
| //The order of tests matters - This assumes Test 0 indicates which point is furthest in | ||
| //the ear - and that point will form a diagonal with the tip of the ear. | ||
|
|
||
| function pointinear(points,face,tests,i=0)= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add underscores between words in name. At first glance it looks like you misspelled "linear"
subdivision.scad
Outdated
| [checkpointinear(points[face[i]],tests),i] | ||
| ; | ||
|
|
||
| function checkpointinear (point,tests)= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add underscores between words in name. At first glance it looks like you misspelled "linear"
subdivision.scad
Outdated
| vsum([for(i=[0:count-1]) cross(points[face[(i+1)%count]]-points[face[(i)%count]],points[face[(i+2)%count]]-points[face[(i+1)%count]])],0) | ||
| ; | ||
|
|
||
| function convexvertex(wd,points,face,i=0)= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add underscores between words in name.
roundedcube.scad
Outdated
| @@ -0,0 +1,149 @@ | |||
| use<FunctionalOpenSCAD/functional.scad> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create a separate PR for independent features (rounded cube, vs triangulate)?
When you submit a PR it accounts for ALL differences between my branch and yours. So typically if you have multiple changes you are working on concurrently, you would develope each independent feature in its own branch, then make the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the best way to do that now that I've got both in on my local version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably easiest to delete roundedcube.scad in your master, then we can focus on this PR striclty for the triangulation feature.
Since this PR came from your master branch, any updates to your master will update this PR automatically, until the PR is closed.
Then create a new "rounded_cube" branch. Ideally you would create this branch this from your master at from the commit just before you added triangulation.
git branch branchname <sha1-of-commit>
Your roundedcube.scad will still be in your git history, and you can re-add it in this branch.
Off-by-one error
A rounded cube 'primitive' that uses cube/octahedron duality and face triangulation capability to help with subdivision.