V2 : Add center parameter to cylinderElliptic / cylinder primitives

[z3dev]

While converting some examples to V2, it became apparent that centering of cylinders would simplify the creation of designs. In addition, other primitives have center, so adding centering to cylinders normalizes the parameters to primitives.

cylinderElliptic:

• [X] code complete
• [X] tests complete
• [X] review complete

cylinder: (alias to cylinderElliptic)

• [X] code complete
• [X] tests complete
• [X] review complete

[z3dev]

@kaosat-dev ... I like the idea of removing the center parameter from the primitives. Removing the center parameter will simplify the definitions for each primitive, which will help later in future plans. In addition, the center function as well as the translate functions are easily available.

Thoughts?

[kaosat-dev]

@z3dev as said in hangouts, I do think that is the way forward, BUT is there any case where we need to store the center in the primitives data for any reason (I cannot think of any valid reason to do so)

[z3dev]

I don't think there's any reason to store the center information as part of the primitives definition. We just need to be consistent that all primitives are 'centered' at the axis, always. This is required for symetric rotations anyway.

I think we should consider a few more measurements just to help the users. See Issue #23

• measureCenter() ; current method of finding midpoint between min and max points
• measureMidpoint() ; average of all points
• measureCenteroid ; "centre of gravity" or the "center of mass" see http://paulbourke.net/geometry/polygonmesh/

[kaosat-dev]

@z3dev the default positioning for cubes etc is not centered sadly (thanks to openscad) , but 100% agreed otherwise. In addition to the measurements (I agree with all of them) we also need (I have pre-existing code too)

• align (top , bottom, on a plane etc)
• center (the operation of centering)
• distribute

[z3dev]

Cool. I’ll open new issues for all of the new functionality, one for each. And mark them for V2.

I suggest we just leave center parameters in for now, and write code and examples that use the new functionality. And, if all goes well, the center parameters are removed.

How do you want to proceed?

[kaosat-dev]

So you mean adding an issue for 'removal of center' , closing this pr and remove the centers in a later pass ?

[z3dev]

Correct. Let’s remove center in the cleanup pass. Coming soon?

[z3dev]

Hmm... here’s one reason for keeping center in the primitives. SVG defines circle / ellipses / etc. with a center.

<rect x="80" y="60" width="250" height="250" rx="20"/>

[z3dev]

Autocad DXF also has the concept of center point for some ‘entities’.

20, 30 DXF: Y and Z values of center point

[kaosat-dev]

For svg that is just a position/ translation in the end right ? Dxf i dunno The thing is we do not have concepts of centers for 90% of the primitives (because it does not make sense for most of them). Here it is also only a parameter that is making a shorthand for primitive + translate. So i think our previous discussion about not keeping them is stil valid. (unless we get back into geometry vs transforms, haha :-)

[z3dev]

agreed. now that all primitives are created 'centered on the axis'. we can translate easily.

[kaosat-dev]

@z3dev cool, glad we agree on simplying :D, so what shall we do with this PR ?

[z3dev]

Closing this PR based on discussions. I will save the code until V2 is done and dusted.

[z3dev]

Sorry. I re-opened this pull request as I think there may be a good reason to include 'center' in primitives.

As we move forward from V2, the vision is moving to an 'expressive' set of primitives, which can have several implementations underneath. However, the expressive primitives need to be 'transformable'. In other words, the basic transforms can apply to the expressive primitives, if the position of the expressive primitive is known.

let geometry = circle({radius: 10, center: [-5, 10]})
geometry = geometry.translate([5, 5]) // translate can be applied to the expressive primitive
geometry = geometry.scale([0.5, 2.0]) // scale can be applied to the expressive primitive!

I just wanted to bring this up for discussion before putting this to sleep.

@kaosat-dev Thoughts?

[z3dev]

@kaosat-dev sorry, but I think this should go into V2. From the conceptual standpoint, translation should be separate, but...

From the user point of view, centering a shape is easy, and easy to calculate as well From the performance point, only one function to create and place the shape From the stack point, only one data model, not two

I’d really like this to be part of V2.

[kaosat-dev]

@z3dev Sorry, I should have answered here as well last time and not just on hangouts. I have to dissagree on this, sorry: imho it is not just about adding an extra parameter into ever single primitive, this is really the same discussion as with transforms:

• you need to store the center (which center btw ? the one for the geometry? the one for the transform, more confusion)
• what happens when you apply a transform ? what happens with the center ?
• is it local space, model space, world space ?

From the performance point, only one function to create and place the shape

No, under the hood, it is still multiple operations at some point or another :)

From the stack point, only one data model, not two not sure I understand ?

[z3dev]

Closing as this was PR is part of V2.