Better naming for SimplePolygon and Polygons

[geoffder]

I was thinking that it might be good to rename them to Contour and Contours respectively. That way they would line up a bit better with the way that they are talked about in the docs, and also convey more clearly that Polygons does not necessarily have any structure or relationship between the contours within (e.g. they can be intersecting and are not ordered into complex polygons in any way).

Does that make sense?

[elalish]

Yeah, that's reasonable, but it still feels like 6 of one, half-dozen of the other. I'd like to avoid breaking changes where possible, so I'd rather avoid a rename here.

Actually, this made me realize something - you swapped Extrude and Revolve to take CrossSection instead of Polygons. For the same reason, I think I'd like to revert that. It's also nice because it allows Manifold and CrossSection to be independent libraries without dependencies on each other. I try to keep the pieces separated that way to the greatest extent reasonable. Same reason SDF doesn't create a Manifold directly.

How does that sound?

[elalish]

Well, I suppose Cylinder and such are still using Circle, so maybe not that independent, but meh.

[geoffder]

Ah, I figured the change to CrossSection was desirable (I think we did talk about it before I did it) since it enforces the inputs to be intersection free (something that is totally up the user otherwise). Also it is better ergonomics to be able to give CrossSections straight to Extrude and Revolve, rather than always having to use ToPolygons.

[geoffder]

In general I'm on the side of maximizing the ergonomics of the API over keeping things seperate in case someone wanted to spare themselves depending on the sublibraries (which are quite small compared to Manifold itself).

Tangentially related, I'm personally looking forward to when module support improves, I might like to take a crack at reorganizing with then and see how much faster it makes the build.

[geoffder]

Sorry to spam, but another thing came to mind that I'll add before I sleep and forget: RE breaking changes with Polygons, we have already recently changed the type by dropping the indexed struct.

If we come to an agreement about names that would be better going forward, at least it isn't something that is too stable. Plus they are just aliases as is, so the old names could be easily deprecated with the new existing alongside for some time.

[elalish]

You make good points. I'm thinking it over; I'll get back to you.

[elalish]

Okay, I agree about keeping Extrude and Revolve taking CrossSections - you're right that forcing removal of self-intersections is important for the ergonomics.

Yes, I did just make a breaking change to Polygon, but I still feel like changing their names now mostly amounts to needless churn - Contours doesn't evoke much difference in meaning to me. I think we'd be better off including clear descriptive docs for those aliases instead.

Thanks for mentioning C++20 modules - I ended up reading up on them. Agreed, that sounds great once the compilers are ready for us (and it sounds like they're pretty close now). That would be ideal, because definitely part of my library design here was focused on keeping the compile time down to something reasonable.

[pca006132]

I tried using pch but got some weird errors. I think the major slowness probably come from thrust, which is a template library. I don't think modules/pch does much to reduce compilation time for templates unfortunately.

[geoffder]

🤔 Now that you mention it, I wonder if switching from thrust to std parallel stuff like you guys have discussed recently would help with the compile times.

[elalish]

Honestly, at least for nvcc, I think they're basically using the same code for both so I doubt it makes much difference.

[geoffder]

Supposedly clang can compile CUDA, and I saw some comments saying the build was faster (though I'm not sure about performance of the code). I've actually never changed my make config to use it over gcc so now I'm curious to try. I seem to remember being recommended it for improved template errors.

[geoffder]

Speaking of build times, I've opened an issue on Clipper2 to ask whether they'd be open to making the build of the Clipper2Z library (which we don't use) optional.

Well, that was a quick turnaround! AngusJohnson/Clipper2#463
I'll have to update our Clipper2 commit and set the new flag then.