OpenNI / OpenNI2

OpenNI2
Apache License 2.0
432 stars 893 forks source link

Path resolution based on OpenNI2.dll directory (plus xnOSGetDirName() fix) #7

Open tomoto opened 11 years ago

tomoto commented 11 years ago

Hi OpenNI folks,

Here is a patch to address the issue I pointed out in the community at http://community.openni.org/openni/topics/search_path_for_redist_files. With this patch, the path for the drivers and OpenNI.ini will be resolved based on OpenNI2.dll's directory so that the users are no longer required to launch the application from the designated directory. It also allows them to use the shared driver repository just by setting PATH environment variable to its master OpenNI2.dll.

This patch includes three commits as follows. All the details are explained in their commit comments.

I hope this patch to be reviewed (and accommodated if you agree) as early as possible since this is an external behavior change that may impact on the beta users although it is (I believe) a significant improvement.

Please get back to me if you have any questions. Thank you very much, Tomoto

tomoto commented 11 years ago

I may add another commit if I can apply the same method for PS1080.ini. Thanks.

tomoto commented 11 years ago

I have done the work for PS1080.ini and committed. Thanks.

tomoto commented 11 years ago

Benn (piedar) found my code did not compile with gcc (on Linux). We are working on it at https://github.com/tomoto/OpenNI2/pull/1.

tomoto commented 11 years ago

I decided to build my own Linux environment to investigate the problems reported by Benn, and eventually completed the working implementation! My quick testing confirmed the paths were resolved based on the lib*.so's. Please see the commit 86efc7a.

piedar commented 11 years ago

This path resolution branch is working nicely for me on Gentoo Linux x86_64.

tomoto commented 11 years ago

Hi Benn,

Thank you very much for your contribution to find the issue and validate the fix! I really appreciate it!

Tomoto

tomoto commented 11 years ago

I merged this pull request into the develop branch after the peer review by Eddie.

kubat commented 11 years ago

This was merged in, but then immediately overwritten by the following commit. Was this intentional or an accident?

piedar commented 11 years ago

When I asked the same thing, I was told it was an accident. But I think this ended up getting stranded in the develop branch. I wonder what the plan is for merging from there to master...

tomoto commented 11 years ago

I think piedar is right. The patch 4eec88bdb7 was immediately overwritten by b8aae20cf4 by mistake, and then recovered by 743fbda191 but it is still only in the development branch. I hope it will be merged into the master in the merge.