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

Make all modules optional and separate them out in the codebase #119

Open
vadixidav opened this issue Mar 8, 2019 · 12 comments
Open

Make all modules optional and separate them out in the codebase #119

vadixidav opened this issue Mar 8, 2019 · 12 comments

Comments

@vadixidav
Copy link

TL;DR: It should be possible to specify exactly what features you want using cargo, and it should "just work."

Currently there is some intermingling between features2d and xfeatures2d. OpenCV also has several other modules and some have dependencies on each other. This depends on the completion of the sys crate refactor #115. The sys crate will only build the parts of OpenCV requested. This should result in being able to include just the components needed to avoid having to pay royalties to patent encumbered things in opencv_contrib.

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 12, 2019

Moving all contrib features behind contrib feature would be sufficient.

@vadixidav
Copy link
Author

vadixidav commented Mar 12, 2019

That might be a good starting point, but it wouldn't be too hard for me to just put in logic for all the modules based on their dependencies.

@Pzixel
Copy link
Collaborator

Pzixel commented Mar 12, 2019

It just make no sense to me. Having 100+ features doesn't make crate easier to use but the opposite. Having minimal amount of switches that covers all cases is the best. Think of it as of Karnaugh map.

@geoeo
Copy link
Contributor

geoeo commented Mar 30, 2019

Whats the status of this? I just compiled opencv3 from sources and ocr.hpp doesnt seem to exist? I also cant find it in the source repo. This might help these kind of issues

@vadixidav
Copy link
Author

@geoeo It seems @Pzixel might not be interested in accepting changes to enable only specific modules. However if you are interested in messing around with cv-rs where it builds OpenCV for you, you can use my #117 branch. Just point your git line to my repo and set `rev = "refactor". You can also use the system feature to cause it to use the system packages like the current master does. It requires clang to be installed as well though.

Are you interested in making each OpenCV module optional? If you could provide some feedback, it might help determine what happens with this issue.

@geoeo
Copy link
Contributor

geoeo commented Mar 31, 2019

The core issue is different. Its that it seems that the ocr.hpp file has been removed from the core opencv repo. This would have helped to mitigate the failed compilation. But im not 100% sure.

@vadixidav
Copy link
Author

@geoeo Make sure you compile the opencv submodule in this repository. It points to the right build of OpenCV.

@geoeo
Copy link
Contributor

geoeo commented Mar 31, 2019

I thought I only have to do $cargo build
What else do I have to do?

Edit: i see now text is part of the opencv_contrib repo. Which i have to link in cmake explicitly

@geoeo
Copy link
Contributor

geoeo commented Apr 1, 2019

I cant get this folder to compile. I have to install 100 dependencies which are not relevant. I would +1 this if that means anything.

@vadixidav
Copy link
Author

vadixidav commented Apr 1, 2019

@geoeo After I merge the #117 branch, you wont really need any dependencies installed on Linux (except GTK3) or Windows. You can try it out now if you want, but I am still dealing with issues on Ubuntu 16.04 on Travis (so it passes CI). It also statically links OpenCV by default so you don't need to install that either.

@geoeo
Copy link
Contributor

geoeo commented Apr 6, 2019

There's a typo in build.rs line 102

let cargo_rustc_link_search = env::var("OPENCV_LIB").unwrap_or("/use/local/lib".into());

I think it should be

let cargo_rustc_link_search = env::var("OPENCV_LIB").unwrap_or("/usr/local/lib".into());

Didnt want to make a PR for this.

@Pzixel
Copy link
Collaborator

Pzixel commented Apr 7, 2019

Fixed

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

No branches or pull requests

3 participants