Skip to content
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

v3.0 #1024

Merged
merged 7 commits into from
Nov 18, 2024
Merged

v3.0 #1024

merged 7 commits into from
Nov 18, 2024

Conversation

elalish
Copy link
Owner

@elalish elalish commented Nov 5, 2024

This is a huge release! We've packed in all the requested breaking changes to put us on a solid path of API stability going forward. We have removed all of our required dependencies (no more Thrust or GLM!) and dramatically simplified our directory structure and CMake. We have upgraded internally to double precision and added MeshGL64 for those who need to access it. And we have simplified and improved the API in many ways and added a few powerful new features. Please check out our new and improved C++ and TypeScript docs pages!

Breaking Changes

New Features

Bindings

Bug Fixes

Performance Improvements

Build/CI Updates

Tests

Documentation

New Contributors

Full Changelog: v2.5.1...v3.0.0

@elalish elalish self-assigned this Nov 5, 2024
@elalish
Copy link
Owner Author

elalish commented Nov 5, 2024

I think we should probably update the README some more here, but I need to think about it a bit more. Suggestions welcome.

@pca006132 WDYT, are we ready for v3.0 release? Anything we should hold for?

@elalish elalish requested a review from pca006132 November 5, 2024 23:56
@pca006132
Copy link
Collaborator

I think the only potential blocker is the C binding stuff, as that is a breaking change. I think we can just go ahead with the changes if no one opposes to it.

  1. Some of the dependency description is not formatted correctly in the README (' vs `):

    image

  2. We are using nanobind instead of pybind11.

  3. Should probably mention the dependency name instead of its name in Ubuntu ppa (e.g. not libtbb-dev but tbb).

  4. Offline building: Missing FETCHCONTENT_SOURCE_DIR_CLIPPER2.

  5. Can remove the remarks about Emscripten version. We are already using 3.1.61 in our CI.

  6. Should probably remove the part about fuzzing support, as it is not working for now.

  7. Can add a shell command about using of the formatting script, and maybe git precommit hook for it (we should probably use it ourselves...).

  8. Windows shenanigans: Not sure if this is important now, considering we only have one main dll. Probably need someone more familiar with windows to decide. We can keep it for now.

  9. I can probably find some time and write instructions for developing on nix.

@pca006132
Copy link
Collaborator

pca006132 commented Nov 6, 2024

@SovereignShop btw is it possible for you to make the Clojure binding depends on the C binding alone? Also @geoffder. i.e. without the need for a fork. We can try to integrate the changes needed before we release v3.0.

@elalish
Copy link
Owner Author

elalish commented Nov 6, 2024

Okay, SG - we'll hold this release for the C bindings update if you have time to take care of it. I'm not very comfortable with C myself. Meanwhile I'll update the README - thanks for the comments.

@geoffder
Copy link
Collaborator

geoffder commented Nov 6, 2024

@SovereignShop btw is it possible for you to make the Clojure binding depends on the C binding alone? Also @geoffder. i.e. without the need for a fork. We can try to integrate the changes needed before we release v3.0.

Unfortunately my hiatus has meant I haven't found time to update my OCaml Manifold library in some time, but I already wasn't depending on a fork, just an unadulterated git submodule. If I'm understanding your question correctly.

@pca006132
Copy link
Collaborator

@geoffder yeah if you can use the C binding directly already, it is good.

@elalish I will try to find some time to do it, I am stretched really thin recently.

@elalish
Copy link
Owner Author

elalish commented Nov 7, 2024

@pca006132 Thanks, but don't burn yourself out over these C bindings. We can put out v3.0 without it too if it's unreasonable in the near term. I don't have a good sense of how much work it will be.

@elalish elalish mentioned this pull request Nov 7, 2024
@elalish
Copy link
Owner Author

elalish commented Nov 7, 2024

One more question: should I remove the deterministic option for v3.0? I'm concerned it makes the library harder to test, and is it really deterministic even when true? If we remove it, which branch should we keep?

@pca006132
Copy link
Collaborator

  1. Yeah, we can remove it. I think we can just keep the deterministic version, iirc it is fast enough. Not sure if it is fully deterministic, but ideally it should be.
  2. I don't think C binding requires too much work. But it is still some work that I need to find time to do :)

@starseeker
Copy link
Contributor

@elalish so does releasing 3.0 warrant another blog post? ;-)

@elalish
Copy link
Owner Author

elalish commented Nov 7, 2024

@elalish so does releasing 3.0 warrant another blog post? ;-)

Yeah - I should write some on the smoothing and SDF work I've done too. All in my copious spare time...

@elalish
Copy link
Owner Author

elalish commented Nov 16, 2024

Okay, I've added some README cleanup and bumped the version to 3.0. I also went through our release checklist and though I'm not sure what change(s) was responsible, our WASM is now about twice as fast for me (running examples on ManifoldCAD.org), so that's a good sign.

The only question now is whether we should wait for #1048 (or some subset of it) before cutting v3.0. @kintel - you have the strongest opinions here, so let me know. I'd like to get this out the door, but not at the cost of usability.

@hzeller
Copy link
Contributor

hzeller commented Nov 17, 2024

I think it would be good if it was possible to query the manifold version in software, e.g. some macros to define MANIFOLD_MAJOR, MANIFOLD_MINOR or other schemes to query the version, probably defined some header like manifold/version.h or similar. It should be macros so that it is possible in downstream applications to keep things compatible with #ifdefs.

Would probably good to have something like this starting with v3.0.

(context: I was just looking how easy it would be to get the version from Manifold to implement openscad/openscad#5432 ... but it is not in any Manifold-provided header, and trying to get things from the build system such as cmake is sketchy at best - right now I only see the version number in CMake. It needs to be in a proper *.h header).

@kintel
Copy link
Contributor

kintel commented Nov 17, 2024

+1: Compile-time version checking would be nice. It doesn't matter much as long as everyone is using manifold from git HEAD, but as we start seeing multiple deployed versions in the wild, we eventually have to make compile-time feature/version checks.

@kintel
Copy link
Contributor

kintel commented Nov 17, 2024

I'd say wait for #1048 - but thinking about it , it really doesn't matter too much - that doesn't change the API, so it could easily be made part of 3.0.1. Just beat the package managers to the punch by rolling out build system fixes fast :)

@pca006132
Copy link
Collaborator

I'd say wait for #1048 - but thinking about it , it really doesn't matter too much - that doesn't change the API, so it could easily be made part of 3.0.1. Just beat the package managers to the punch by rolling out build system fixes fast :)

Not sure if that is a good idea. This will render our 3.0 release as something weird that distros should avoid...

@elalish elalish added this to the v3.0 milestone Nov 18, 2024
@elalish
Copy link
Owner Author

elalish commented Nov 18, 2024

Okay, now's the moment - are we ready to push v3.0? Speak now, or suffer the fate of a point release.

@pca006132
Copy link
Collaborator

I think it is ready.

@elalish elalish merged commit 5d127e5 into master Nov 18, 2024
21 checks passed
@elalish elalish deleted the v3.0 branch November 18, 2024 05:28
@jonathanhogg
Copy link
Contributor

Just wanted to note that there don't seem to be wheels being built for Python 3.13

@pca006132
Copy link
Collaborator

@jonathanhogg This seems to be an issue related to cibuildwheel version. I guess we can update it and run it again, see if pypi is happy with publishing the same version twice with added wheels?

Btw, I think we are targeting stable ABI for cpython, wondering if there is a way to tell python it can reuse the same wheel after 3.12...

@jonathanhogg
Copy link
Contributor

jonathanhogg commented Nov 18, 2024

@pca006132: Yeah, I think cibuildwheel picks up the supported versions from the tags in the pyproject.toml file and then builds a wheel for each one.

PyPI will let you add wheels to a release, but not change any of the original files. So it depends on your CI script as to whether it will barf on the existing ones or silently ignore the upload failures it gets.

I don't know if there's a way of marking a binary wheel as being compatible with more than one version…

Luckily the source build works now ;-)

@pca006132
Copy link
Collaborator

https://github.com/elalish/manifold/actions/runs/11906944291

It builds python 3.13 wheels, but the upload action is having issues. Maybe we should manually upload the missing binaries? Will have a look at how to do that.

@pca006132
Copy link
Collaborator

OK, I downloaded the artifacts and uploaded them manually.

@cartesian-theatrics
Copy link
Contributor

cartesian-theatrics commented Dec 15, 2024

Wow, this is a massively breaking release. Gonna take a long time to support. Glad it sounds like stability is the goal moving forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants