jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.68k stars 516 forks source link

API breaking changes in modeling #1079

Open z3dev opened 2 years ago

z3dev commented 2 years ago

I'd like to group together all API breaking changes into a single release. So, if there are any suggestions then now is the time.

Suggestions:

hrgdavor commented 2 years ago

There is axes parameter for ellipsoid that buggs me

Name Type Attributes Default Description
axes Array   an array with three vectors for the x, y and z base vectors

it allows for changing orientation and maybe size of the ellipsoid. I do not really understand it and it is one thing that was thorn in my eye when looking into instancing.

My idea is to have a reusable unit-shpere (size:[1,1,1]) for specific segment count.

When looking at specific segment count, Without the axex paramert it would be trivial to reuse the unit-sphere and produce any sphere or ellipsoid by reusing it and attaching a scale transform. If there is an easy and reliable way to convert the axes into a transform for unit sphere then this parameter is no issue to me.

Disclaimer: this proposal stems from my lack of understanding the effects of the axes parameter.

There are not many direct benefits from instancing right away as most slices do not implement properly instancing in 3mf, but is something I think worth supporting. http://3d.hrg.hr/jscad/three/threejscad2.html?uri=model.logos.js

in the demo there, without instancing regl would choke if grid is larger than 6x6x6 but isntanced rendering works smooth for a very large grid

platypii commented 2 years ago

Unify offset and expand since they seem to do almost the same thing. I would remove one or the other.

platypii commented 2 years ago

I would like a more standard way to convert between geometries.

Some functions which don't exist, but maybe should:

There is a general question of should there be geom2.fromPath2()? Or should there be path2.toGeom2()?

Right now there are places in the code that do:

geom2.fromPoints(path2.toPoints(path))

But this is actually the wrong way to convert to geom2 -- because it doesn’t check for path.isClosed.

This might not actually be a "breaking" change, just adding functions, but I think we could simplify some things if we have a standard pattern to follow.

z3dev commented 2 years ago

Another idea: I would like a more standard way to convert between geometries.

Some functions which don't exist, but maybe should:

True. I think there was hesitation to add these because of one geometry is required to know the internals of another.

There is a general question of should there be geom2.fromPath2()? Or should there be path2.toGeom2()?

I have no preference, but maybe just convert(path2) or convert(geom2) should be fine.

FYI, 2D to 3D is always via extrude functions. And 3D to 2D is via project functions.

But this is actually the wrong way to convert to geom2 -- because it doesn’t check for path.isClosed.

geom2.fromPoints() checks for closure so it doesn't matter.

This might not actually be a "breaking" change, just adding functions, but I think we could simplify some things if we have a standard pattern to follow.

There could just be a set of utility functions that convert between geometries.

const mypaths = convert(geom2)
const mygeom = convert(path2)
z3dev commented 2 years ago

Unify offset and expand since they seem to do almost the same thing. I would remove one or the other.

Sounds fine but a new parameter is required to control the expand. That's why I opted for two functions.

I was browsing around for a good parameter to control the expansion in all directions (current offset) versus one way (current expand).

z3dev commented 2 years ago

There is axes parameter for ellipsoid that buggs me

Me too. This piece of strangeness is used by 3D expand. It would be nice to remove this after 3D expand is rewritten.

platypii commented 2 years ago

Remove functions which do basically the same thing modulo cloning. As long as we treat data structures as immutable, these can be combined:

Related, path2.create should have an option parameter for isClosed instead of defaulting to false. It's weird to assume that the path is not closed when calling create.

hrgdavor commented 2 years ago

I just remember one really big issue I have with modeling and that is segments

OCCT and OpenSCAD have this not because they want to be very fancy, but because reducing precision and number of vertices can help a lot with performance when projects get bigger. Shortening time between making a change and seeing a result is critical in design phase.

I had a rant about it on #899 that I later truncated, so it could be used to discuss the details

z3dev commented 2 years ago

I found some parameter names which are not camelCase. Those should be fixed as well.

z3dev commented 2 years ago

@danmarshall do you have any suggestions? I'm sure there are a few things that you would like to see changed. 😃

briansturgill commented 2 years ago

Just to give an outsider's perspective on Path2. I found it incongruent with Geom2/Geom3. I see no reason why boolean operators, etc. should be used on a "path". So I left it out of CSharpCAD. If I do put it in eventually, my idea was to have a "Path" object that would have methods like arc, bezier, line, etc (a set of primitives that 2D drawing has) and you would "finish/close" it by calling ToGeom2(). Only after converting to Geom2 would it be usable with the rest of the package. I did not do this yet, as it seems to me that it really isn't in the realm of CAD, but I've not made up my mind fully yet.

To clarify... I believe that a Geom2 polygon is the proper way to communicate a 2D drawing into CSharpCAD. I intended to do a similar thing with Text which will use FreeType's polygon outlines to communicate into CSharpCAD.