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

README.md improvement for Open3D library installation #4

Merged
merged 4 commits into from
Mar 29, 2024

Conversation

DamienGilliard
Copy link
Collaborator

Hi @9and3 , I tried to install Open3D following the instructions on the README.md but the files behind the link contains pre-built binaries and thus no CMakeLists.txt and cannot be built from source (and after building from source from the latest release I got linker issues ). With the changes proposed, the pre-built binaries are placed at the right place and link properly.

README.md Outdated
Comment on lines 52 to 54
Expand-Archive -DestinationPath 'C:\Users\<Your user name>\Downloads' -LiteralPath "C:\Users\<Your user name>\Downloads\open3d-devel-windows-amd64-0.18.0.zip"
Move-Item -LiteralPath 'C:\Users\<Your user name>\Downloads\open3d-devel-windows-amd64-0.18.0\*' -Destination 'C:\Program Files\Open3D\'
rmdir 'C:\Users\<Your user name>\Downloads\open3d-devel-windows-amd64-0.18.0\'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With what you propose it will work with the current CMakeLists.txt file of DiffCheck because you probably already compiled everything before for Open3d (I remember you told me that you compiled). In this way we are using precompiled binaries and not compiled from source. It's safer for now to keep the compiled from source. But well spotted for the error in the link.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok ! I probably made an error when compiling from source, then. For me unzipping the prebuilt binaries was the only way to make it work, that's why I made this PR. Thanks for the review !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No thanks for letting me know. You made me think now that we should do this better.. I am opening a PR for refactoring the cmake project.

The thing is that you should erase all the Open3D install files of your previous build before recompiling from source.
I'll keep you posted for the refactoring of the CMake project.

@9and3
Copy link
Contributor

9and3 commented Mar 29, 2024

I am merging but do not hesitate to open new branches to propose changes for the installation @DamienGilliard !

@@ -72,7 +72,7 @@ To build and test the project, follow the following steps:
```terminal
cmake/config.bat
cmake/build.bat
./build/bin/diffCheckApp.exe <-- for prototyping
./build/bin/Release/diffCheckApp.exe <-- for prototyping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@9and3 9and3 merged commit ba86fbe into main Mar 29, 2024
1 check failed
@9and3 9and3 added the enhancement New feature or request label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants