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

Nix build #6854

Merged
merged 10 commits into from
Dec 19, 2024
Merged

Nix build #6854

merged 10 commits into from
Dec 19, 2024

Conversation

netogallo
Copy link
Contributor

I am building a derivation for NixOS 24.05 of open3d-cpu so it can be easily used in nix. To guarantee reproducibility, Nix cannot fetch resources during build and best practices recommend using packages already present in the nix store. For that reason, I removed the use of deprecated functions from ImGui.

Additionally, Nix allows users to easily modify the build parameters of packages, to that end I included a flag to disable the "ComputeUVAtlas" function as it is a single function that leads to the inclusion of significant dependencies.

Finally, Nix does not install packages in systemwide directories. Instead, each package and version gets installed in an isolated directory and nix uses symlinks and env variables to bring the packages into scope. Therefore, a hardcoded DESKTOP_INSTALL_DIR is not suitable for this environment and must be supplied through a compile time variable. If this variable is not supplied (default), the behavior remains unchanged.

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

These changes are required to eventually update ImGui to version 1.9.6. Furthermore, it adds a CMake flag that allows users to build a smaller package by removing the TriangleMesh::ComptueUVAtlas function which adds many additional dependencies to the project. The function is included by default in order to preserve existing behavior.

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

Allow compiling open3d-cpu python package on NixOs 24.05

  • Remove the use of deprecated functions in ImGui
  • CMake option to exclude uvatlas as dependency as a single function leads to the inclusion of many libraries.
  • CMake option to override the desktop install path.

The derivation for packaging v0.18.0 is available at: https://github.com/netogallo/nix-packages/blob/main/pkgs/python-packages/open3d/open3d-common.nix

It currently applies a similar patch before building.

Ernesto Rodriguez added 2 commits July 4, 2024 08:18
* Remove the use of deprecated functions in ImGui
* CMake option to exclude uvatlas as dependency as a single function
  leads to the inclusion of many libraries.
* CMake option to override the desktop install path
Copy link

update-docs bot commented Jul 4, 2024

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@ssheorey ssheorey requested review from ssheorey and benjaminum July 9, 2024 17:46
Ernesto Rodriguez added 4 commits July 11, 2024 07:47
* Remove the use of deprecated functions in ImGui
* CMake option to exclude uvatlas as dependency as a single function
  leads to the inclusion of many libraries.
* CMake option to override the desktop install path
@ssheorey
Copy link
Member

@netogallo thanks for submitting this PR. Removing deprecated imgui functions is very useful.

About making UVAtlas optional, @benjaminum what do you think? I think this adds extra complexity to the build system without being widely useful, so I would recommend to maintain that in a fork of Open3D.

@ssheorey
Copy link
Member

@netogallo if there is a constraint on fetching resources during build, you can build Open3D once on an internet connected system. All required 3rdparty downloads are stored in the 3rdparty_downloads folder. If you include this in the Open3D source, Open3D can be built without an internet connection. cmake looks in the 3rdparty_downloads folder first before downloading from the internet.

@benjaminum
Copy link
Contributor

@netogallo Thank you for these contributions!
I think being able to override the DESKTOP_INSTALL_DIR is a good idea. Please see the change which allows you to directly set the variable from the cmdline.
Thanks for updating ListView!

About UVAtlas, I removed the build option as we have already quite a large number of build options and likely will expand UV map functionality in the future. Happy to hear from you how we can make it easier to deal with dependencies for Nix.

@benjaminum benjaminum merged commit 2a67419 into isl-org:main Dec 19, 2024
38 of 39 checks passed
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.

3 participants