Closed dsjolie closed 10 years ago
Compiling to an out-of-source build directory is not the same thing as installing. This patch makes "make install" and similar work properly, which I think is nicer when you really want to use OsgOculus as an external library in another project (which I'm moving to now).
It's a bit of a preference, but if you don't want to install these instructions shouldn't hurt you.
Yes, I suspected as much. Then we keep both.
I see that you install the shaders to a folder named shared. Is that an arbitrary foldername? If so, I would rather rename it to shaders or something similar.
Great!
No, it's not arbitrary. "share" (not "shared") is a standard folder name for "data files" on unix systems, in parallel to bin, lib, and include. (E.g., try "ls /usr/local").
A related issue, however, is that it would probably be good practice to add subfolders at least for include files and data files. I'm inclined to leave that for later but I wouldn't mind sorting it out now.
For example, we might install includes to "include/OsgOculus" and the shaders to "share/OsgOculus/shaders". This should probably be related to a namespace in the code as well, at least in the long run.
Note that this (adding subfolders, not required for this patch) implies a matching structure in the source project, so that include files are always included as "#include <OsgOculus/whatever.h>" or similar. (osgOculus perhaps?)
Yes, maybe the files should be organized in a more OpenSceneGraph like structure. For example the following folders could be used:
Another option would be to move the shaders into the source files and compile them into the library. Then no file lookup would be needed.
I have moved the shaders inside the OculusDevice class. So there is no longer any need to copy these files separately. 49de90c71925bacf07cea21800361f772fe9c4e4
I have also added a install script which is similar to what you have suggested, aside from the shader copy. 903a56464b56e2a899d8e9172564ff376dd5b0fd
And I have added your name to the list of contributers. fad4c4e6f3ee412b7338e43641b45c409b7833eb
Since this pretty much cover what was included in this pull request, I will close the request without merging it.
Sounds great! I'll add a comment to the install commit.
Why is this needed? I thought this was covered by: https://github.com/bjornblissing/osgoculusviewer/blob/4e55ef1b8d0ea28a399e3e0902c00f232cb285da/src/CMakeLists.txt#L104-L112
Or maybe the above passage should be removed and yours inserted instead?