openscad / openscad

OpenSCAD - The Programmers Solid 3D CAD Modeller
https://www.openscad.org
Other
6.97k stars 1.21k forks source link

Feature request: allow center= to accept more than a boolean #265

Open cbrinker opened 11 years ago

cbrinker commented 11 years ago

I find myself spending a lot of my modeling time doing something akin to this:

translate([0,0,10/2]) cylinder(r=5, h=10, center=true);

Which suffers if I need to adjust the height of the cylinder to 20 I have to remember to update the translate. It would be nice to accomplish the same with something akin to:

cylinder(r=5, h=10, center=[1,1,0])

Does this already exist and I didn't catch it in the docs?

Thanks, Chris

--- Want to back this issue? **[Post a bounty on it!](https://app.bountysource.com/issues/218761-feature-request-allow-center-to-accept-more-than-a-boolean?utm_campaign=plugin&utm_content=tracker%2F52063&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://app.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F52063&utm_medium=issues&utm_source=github).
gabebuckmaster commented 11 years ago

I'd just like to second this feature request, I find myself doing the same thing often.

kintel commented 11 years ago

This shouldn't be too hard. TODO:

TakeItAndRun commented 11 years ago

h1=10; r1=5; translate([0,0,h1/2]cylinder(r=r1,h=h1,true);

works fine. Where should your vector center point to? If you change the hight of the cylinder you still need to change the vector pointing to the center.

gabebuckmaster commented 11 years ago

It does work, but it makes for a lot of repetitive code. As far as the vector goes, maybe the vector should move the center of the object around, like: [0, 0, 0] = center in the middle of a cube, [1, 0, 0] = center on the (more positive) x face of the cube, [-1, 0, 0] = center on the (more negative) x face of the cube, [1, 1, 0] = center on the (more positive) x-y edge of the cube, etc.

So for example, if you had a cylinder that you wanted to stay centered on the origin in the x and y directions, but have the bottom stay on the x-y plane no matter the height, you might do:

cylinder(r=r1, h=h1, center = [0, 0, -1]);

gabebuckmaster commented 11 years ago

Alternately, the vector could work like: [1, 0, 0] = center in the x direction only, [1, 1, 0] = center only in the x and y directions, etc.

torwag commented 11 years ago

I like this feature request. I had to fight exactly with the same problem. I think that solution would be sufficient and make things not to complex.

[1, 0, 0] = center in the x direction only, [1, 1, 0] = center only in the x and y directions, etc.

Torsten

On 16 April 2013 23:53, gabebuckmaster notifications@github.com wrote:

Alternately, the vector could work like: [1, 0, 0] = center in the x direction only, [1, 1, 0] = center only in the x and y directions, etc.

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/265#issuecomment-16474146 .

kintel commented 11 years ago

The center parameter would in practice be an array of bool (but you can use 0 or 1 as they evaluate as booleans as well). The array could work in the same way as e.g. cube(5) being the same as cube([5,5,5]).

That said, corner cases often decide whether or not this is well enough thought out - or needed at all, so if anyone feels up to going through there relevant cases in https://github.com/openscad/openscad/tree/master/testdata/scad/features and suggest a bunch of tests, it would make it a lot easier to say yes to the feature request (and quickly implementing it).

donbright commented 11 years ago

maybe someone wants to start a bounty on this?

https://www.bountysource.com/#issues/218761-feature-request-allow-center-to-accept-more-than-a-boolean

(bounty source links the bounty directly to the github issue)

ledvinap commented 11 years ago

@kintel : Using cube(5) being equivalent to cube([5,5,5]) could be expanded to:

function bool2real(x) = (x==false?0:(x==true?1:x));

module ccube(size,center=false) {
    if(len(center)==undef) {            
        ccube(size,[center,center,center]);
    } else {
        translate([-size[0]/2*bool2real(center[0]),
                      -size[1]/2*bool2real(center[1]),
                      -size[2]/2*bool2real(center[2])])
            cube(size);
    }
}
GilesBathgate commented 11 years ago

I am wondering if it would be better to drop most of the primitives built in modules all together and define the modules in scad scripts in terms of polyhedron, then the modifications as above can easily be made by the community without code changes and new releases.

For example here is how the current cube module could be defined

module cube(size,center){
 if(len(size)==undef)
  cube([size,size,size],center);

 x=size[0]; y=size[1]; z=size[2];
 x1=center?-x/2:0;
 x2=center?+x/2:x;
 y1=center?-y/2:0;
 y2=center?+y/2:y;
 z1=center?-z/2:0;
 z2=center?+z/2:z;
 polyhedron([
   [x1, y1, z2], //0
   [x2, y1, z2], //1
   [x2, y2, z2], //2
   [x1, y2, z2], //3

   [x1, y2, z1], //4
   [x2, y2, z1], //5
   [x2, y1, z1], //6
   [x1, y1, z1]  //7
 ],[
   [0,1,2,3],
   [7,6,1,0],
   [6,5,2,1],
   [5,4,3,2],
   [4,7,0,3],
   [4,5,6,7]
 ]);

}

cube([10,10,10],center=true);
kintel commented 11 years ago

I agree. Some of these constructs do, however, introduce extra nodes. I plan to optimize that (e.g. don't create group nodes for if statements).

GilesBathgate commented 11 years ago

I've updated it to use less if statements.

ledvinap commented 11 years ago

@kintel I don't know openscad internals, but isn't it possible to discard group {} containing single child and merge direct group descendants into parent group {} ?

GilesBathgate commented 11 years ago

@ledvinap Yes, this is exactly what I do in RapCAD: https://github.com/GilesBathgate/RapCAD/blob/master/src/treeevaluator.cpp#L462

kintel commented 10 years ago

See Chapter 3.3 - Centering of the following document: https://github.com/devlaam/openscad/blob/loop_extension/doc/Loop.pdf?raw=true

@devlaam implemented something similar to what is discussed above by accepting -1, 0 or 1 for each component of a centre vector. I find this functionality interesting but I find the notation confusing as it effectively inverts the truey/falsey values of the existing center parameter.

Perhaps a better way forward is to add a new parameter for describing how the shape is offset from its natural origin and let center=true be a shortcut for specifying that parameter? What would in case such a parameter be called? align, alignment, offset, or simply translate ? I like the idea that it can take any number value and translate the object accordingly (where -1 and 1 makes the object touch the corresponding axis).

..or is this too confusing?

MichaelAtOz commented 10 years ago

I think center=[true|false,true/false,true/false] rather than '0' as the base case. In which case it would need to retain the zero/""/undef=false, non-zero/non-empty-vector=true for compatibility to use variables in the center statement, so the above +1,-1 won't work. So perhaps a keyword instead as you suggest.

I was reading CoffeeSCAD doco recently, it has the above, plus X/Y/Z can be a signed number where the center is placed (which would need a keyword etc) so:

One option could be use of % as a flag, representing either; a. 0-100% proportion of object in the positive side of the axis, so 50% is centered, 0% represents whole object is in negative side, 100% whole object is in positive side, or b. -100%...+100%, 0% is centered, -100% whole object is in the negative (so positive side of object is at axis), +100% whole object is in the positive.

e.g. cube(10, center=[100%,true,false]);

Use of >100% could also be handy.

'align' could be a good keyword (and a range of -1...+1) for the above if you don't like '%', although it is getting fussy, would you allow both center & align, cube(10, center=[true,false,true], align=[0,+1,-1]); ?

'offset' or 'translate' to me implies using dimension values, ie cube(10, offset=[-150,0,0]); although 'translate' is probably more true to form (but more typing).

MichaelAtOz commented 10 years ago

Or if you just want alignment to the edges, ie not proportional, then snap=positive|negative or similar. edit/ or snap=middle too or snap=center?? or cube(10,center=[positive|negative|true|false,...]); /edit

GilesBathgate commented 10 years ago

I personally think the right way to do this is to provide a "center" module such that centering can be applied to any primitive. This actually makes the language simpler and more terse.

cube(10, center=true);

//Just do:
center()cube(10);

cube(10, translate=[-150,0,0]);

//Just do:
translate([-150,0,0])cube(10);

I also like the idea about snap, although I would also expect that to be a snap module, and I am not sure how it would be implemented as yet.

GilesBathgate commented 10 years ago

By the way, naturally I implemented a center module in RapCAD ;)

https://github.com/GilesBathgate/RapCAD/blob/master/src/nodeevaluator.cpp#L354

GilesBathgate commented 10 years ago

With regard to vectors that describe how to center, some people have asked for [True,False,False] others have said [1,0,0] (which would be interpreted as boolean) and someone else asked for [100%,0%,0%]

May I propose that the value (rather than being a percentage) is normalised between 0 and 1, in much the same way as for the scale module. i.e. 0.5 = 50% etc.

Combining this with the center module idea I proposed before we would get:

center([1,0.5.0])mything();

Perhaps people reading this are now wondering what it means to "center" something by 50%, well for example if you have a cube(100); and say center about x by 50% it just means move it down by 25.

nophead commented 10 years ago

Maybe your center() module would be better called align(). That also removes the very annoying American spelling.

On 23 October 2013 10:55, Giles Bathgate notifications@github.com wrote:

With regard to vectors that describe how to center, some people have asked for [True,False,False] others have said [1,0,0](which would be interpreted as boolean) and someone else asked for [100%,0%,0%]

May I propose that the value (rather than being a percentage) is normalised between 0 and 1, in much the same way as for the scale module. i.e. 0.5 = 50% etc.

Combining this with the center module idea I proposed before we would get:

center([1,0.5.0])mything();

Perhaps people reading this are now wondering what it means to "center" something by 50%, well for example if you have a cube(100); and say center about x by 50% it just means move it down by 25.

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/265#issuecomment-26892772 .

GilesBathgate commented 10 years ago

Ooh, good idea, but I think I would want an additional module called align, that allows you to translate an object to its bounding box

Something like

align(left=True,top=True)mything();

Probably needs more thought. but generally I think that would be quite useful. I think its a different thing to centring isn't it?

(p.s. I agree about the annoying american spelling, but what can you do :( )

GilesBathgate commented 10 years ago

Ah! Align could work by taking a vector normalised between -1 and +1 i.e. align([-1,0,1]) would mean top right?

nophead commented 10 years ago

Looks like top left to me?

It could also be a vector between 0 and 1, with 0.5 being the centre. I think that is more natural.

On 23 October 2013 11:29, Giles Bathgate notifications@github.com wrote:

Ah! Align could work by taking a vector normalised between -1 and +1 i.e. align([-1,0,1]) would mean top right?

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/265#issuecomment-26894617 .

GilesBathgate commented 10 years ago

Hrm... Yes that sounds right. Like I said it could do with some more thought and perhaps some use case examples.

For some reason I imagined one might want to do align([-0.5,-0.5,-0.5]) but I am not sure now.

devlaam commented 10 years ago

Haha, nice to hear that i implemented (in my hack) the feature request of cbrinker exactly like he requested without having read this. :) Anyway, i really dislike the way center is implemented at this moment. This is because it behaves differently for a sphere, cylinder or cube. And it takes a lot of text space for minimal functionality. Besides that, i needed something to put the object on each side of the origin, or at the origin on each axis. Modifying center came natural, for it does not break code, and is more orthogonal. So

sphere(3, center=[-1,0,1]);  
cube(3, center=[-1,0,1]);

result in objects the same location (negative x-axis, center y-axis, positive z-axis)

Later on, i also needed something to put an object relative to an other, so i introduced position(). With this keyword, which takes two object arguments, it is possible to relocate one object and put it in relative to an other. So you can let sides (any) join, or put some fixed space between them in any direction. Using this command on one object, it can replace the center keyword, but it is (as i just learned from Marius) somewhat slower. Advantage is it does also work on groups of objects. Example:

position(xmin=0,ymax=1,zmid=2, keep=true)
{ cube(5, center=true);
  sphere(2); }

results in a sphere drawn at the origin (in this case the reference object) and a cube of which the left (x-direction) touching the sphere, the back(y-direction) is shifted 1 position from the sphere and the z-middle is lifted by 2. If the "keep" is set to false (default) the reference object is not drawn. Shifting the sphere (by translate for example) will also shift the cube.

You can all try this out from my loop_extension fork: https://github.com/devlaam/openscad

kintel commented 10 years ago

@GilesBathgate The drawback of a center module is that it requires geometry to be evaluated for the children. If you have multiple children, this requires us to drop down into CGAL, which would be equivalent to a render(). in RapCAD, this is normal as there is no preview mode. In OpenSCAD, preview rendering is a crucial component.

On one hand, I welcome new functionality, but every time we add a CGAL-only component, I feel OpenSCAD gets even more sluggish. Some day we really need to reimplement geometry evaluation using a more performant library, but until then, I'm not sure adding such modules improves the situation too much.

Perhaps it's possible to provide decent previews of such modules though, but I don't have any good ideas..

GilesBathgate commented 10 years ago

@kintel To be honest I had forgotten that all these modules resize(), position(), my suggestion of center() and nopheads suggestion of align() would all rely on CGAL. If you want to extend support for just the cube module to allow more complex centring you could define the modules in scad scripts in terms of polyhedron, as I suggested 4 months ago above.

GilesBathgate commented 10 years ago

FYI: https://github.com/openscad/openscad/commit/3cf6c24d834295eb9f409cece0b9aec8f2296fa2

kintel commented 10 years ago

Yes, resize() has the same weakness. For single primitives, all these operations would be fast though, so it would be cool if there was a way of distinguishing between using them for single primitives vs. a group (which is an implicit union) of objects.

Perhaps we could somehow distinguish between directly generated geometry vs. calculated results, but this sounds like changing some basic concepts..

GilesBathgate commented 10 years ago

Personally I would say... discuss which of the operations is most useful. The most powerful being position(), the most obvious being align() and the most simple being center(). Decide what parameters makes the most sense for each of these.... and then just go ahead and implement them, (or just some of them). Put a caveat in the documentation that they are convenient, but slow. (At least you have caching where RapCAD dosen't yet) If you change to a faster geometry engine in the future then you can remove the caveat ;)

nophead commented 10 years ago

It's still a mystery to me why CGAL is so slow on such simple operations like union on simple models on a computer running billions of instructions per second and with billions of bytes of memory.

On 23 October 2013 17:08, Giles Bathgate notifications@github.com wrote:

Personally I would say... discuss which of the operations is most useful. The most powerful being position(), the most obvious being align() and the most simple being center(). Decide what parameters makes the most sense for each of these.... and then just go ahead and implement them, (or just some of them). Put a caveat in the documentation that they are convenient, but slow. (At least you have caching where RapCAD dosen't yet) If you change to a faster geometry engine in the future then you can remove the caveat ;)

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/265#issuecomment-26918450 .

kintel commented 10 years ago

It's due to the (insane) way of representing solids and numbers. solids are represented as an intersection of a bunch of infinite halfplanes, and numbers are represented as a series of fractions. The halfplane technique (if I understand it right) effectively kills spatial culling of operations, while the fraction technique makes every operation excruciatingly slow.

It's possible to make CGAL use doubles, but when doing so, it just starts crashing when double precision limitations makes results ambiguous.

GilesBathgate commented 10 years ago

@nophead I personally think the people who wrote it are academics who have created something theoretically brilliant, but in practicality over complicated and slow. I am not a big fan of template meta-programming, it gives loads of flexibility i.e the ability to plugin different "kernels", which may be a familiar term, (currently for example OpenSCAD is researching the EPIC kernel) but the cost is that CGAL is not really a library in the normal sense, effectively you get a custom static library that it has to compile every time you write a program that uses it (Which is why openscad takes so long to compile) If you are interested in CGAL, and have an hour in which you wouldn't rather be doing something else you can watch this video about cgal (or skip forward to 37:18 where they talk about Exact Geometric Computation and the speed/precision trade offs)

GilesBathgate commented 10 years ago

@kintel You should definitely watch that video, at least from 37:18 onwards (if you haven't already)

nophead commented 10 years ago

Hmm, not watched the video yet but it seems to me GCAL uses those representations to avoid problems with coincident faces, etc but because we use OpenCSG preview and we convert to floating point STLs we have to avoid coincident faces to not have Z artefacts and non-manifold STLs. I.e. we don't make use of the things CGAL does for us that makes it slow. We could use something much simpler that breaks when things are close to numerical precision because we have bodge factors of about 0.01 everywhere.

On 23 October 2013 20:29, Giles Bathgate notifications@github.com wrote:

@kintel https://github.com/kintel You should definitely watch that video, at least from 37:18 onwards (if you haven't already)

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/265#issuecomment-26937995 .

kintel commented 10 years ago

See #753 for a pull request with a slightly different suggestion. Also see my last comment.

FueCarlos commented 10 years ago

I think using non-booleans in the center vector is a bit confusing. If we want to add the additional control of pushing by a fractional amount into different planes, using a new keyword that is mutually exclusive with "center" might be easier (center could be implemented in terms of the new keyword under the covers). I'd prefer to still have access to center=[true, false, true] as I think it reads very well.

For the finer control, at first I liked -1..1, but then I realized it is a bit redundant... isn't -0.25 the same as +0.75? It's also confusing in that a value of 0.5 means you are translating a 100-unit cube by 25 units (not 50). So I'll throw in a new suggestion :)

  shift=[(0..1), (0..1), (0..1)]

The value indicates what percentage is in the positive plane. 0.5 would mean 50% of the object is in the positive plane, i.e., it is centered. 0 means none of the object is in the positive plane (it is in the negative plane), and 1.0 means it is fully in the positive plane (same as current center=false). This has the bonus that the percentage is relative to the total dimension (instead of dimension/2 as would happen with -1..1).

I'm torn about the treatment of cylinder and center. In request #753 I went with honoring keeping X and Y always centered regardless of the vector, but I'm not sure that was the right thing to do.

kintel commented 10 years ago

Having fine-grained control over the shift might be taking it too far. How about: align="center" | "negative" | "positive" - with the option of align being a vector?

MichaelAtOz commented 10 years ago

center = is used in: square(), cube(), cylinder(), surface(), linear_extrude().

The align/shift/position concept is applicable to more than those, e.g. import(), circle(), sphere() and possibly the poly's; but possibly so does center= if it is extended, e.g. sphere(10,center=[1,1,0]) is just as relevant to the OP's cylinder issue.

So I suggest they are treated as two separate concepts, center and align. In that case the original proposal center=[x,y,z] with x,y,z as booleans is the base case. It is a straight forward evolution of the existing usage, conceptually aligns with other 3-vector usage. Presumably easily implemented.

So; a. Is extending center= to import(), circle(), sphere() relevant? b. Poly's is beyond me, do they have a center that is readily understandable, for irregular shapes? c. For linear_extrude() center=[x,y,x] doesn't seem to apply. ??

Then for align, should that be just a property of an object ie cube(10,align=[center,negative,positive]) or a module as discussed earlier ie align([center,negative,positive]) cube(10)

But as the latter may need CGAL for complex trees, so is it just easier to apply it to a base object because we know the geometry at the time. Would align() sphere() be as easy to optimise as sphere(align=...)? If so align() can be used efficiently for regular objects, or, just like resize() inefficiently for more complex trees.

I fear I'm starting to ramble...

MichaelAtOz commented 10 years ago

Note: Wiki says "A Boolean value is passed as the 'center' argument to the 'cube' and 'cylinder' primitives," & "In all of these contexts, you can actually pass any type of value. Most values, when used in a Boolean context, are equivalent to 'true', but there are a few that are equivalent to 'false'."

When I was fiddling with linear_extrude() the center parameter did not respond to 0/1, perhaps that should be fixed while we're at it. Also just confirmed square() surface() likewise.

FueCarlos commented 10 years ago

The existing center code explicitly checks the type to see if it is boolean in a few places, which is why linear_extrude didn't work with 0/1.

@kintel, I'm okay with the align=[center|positive|negative], however is there already an enum-like parameter in the API? I just took a look through the wiki, and I didn't see a form similar to parameter=[foo|bar|baz]. It would be preferable if these were tokens (like true/false) instead of quoted string so they could be error-checked at parse time and in syntax files.

Are you advocating that align replace center, or that center becomes syntactic sugar over align?

kintel commented 10 years ago

@FueCarlos We don't have enums. For other nodes, e.g. offset(), we've used strings for this purpose. Having some error checking would make sense though.

With the align solution, I think we could effectively make center syntactic sugar over align, or even deprecate center.

ghost commented 9 years ago

Problems like this can be solved by writing a small wrapper module.

module aligned_cube( size, align = [ 0, 0, 0 ] )
 translate(size/2*[[align[0],0,0],[0,align[1],0],[0,0,align[2]]])
  cube( size, center = true );
aligned_cube( size = [ 10, 20, 30 ], align = [ 0, 0, 1 ] );

Took me way too long realize this. Countless times writing 'center=true' until it finally struck me. hehehe

Salamandar commented 7 years ago

Up ;) I was gonna open an issue when I saw this one. This would considerably simplify a LOT of openSCAD files.

ranguli commented 7 years ago

@kintel Should we move this to the ideas for development section of the wiki if it's under consideration, or close the issue otherwise?

kintel commented 7 years ago

@ranguli A writeup on the wiki would indeed be appreciated. Not sure if that warrants closing the issue as the discussion here may be worth keeping, but at least it would anchor the feature request there.

ranguli commented 7 years ago

@kintel Perhaps a "Long-term" or "Reviewed" label might help to clean up some of the older issues that have been examined and aren't ready to be closed but aren't currently being worked on?