RenderKit / ospray

An Open, Scalable, Portable, Ray Tracing Based Rendering Engine for High-Fidelity Visualization
http://ospray.org
Apache License 2.0
1k stars 182 forks source link

library: open module relative to libospray's location #312

Closed mathstuf closed 5 years ago

mathstuf commented 5 years ago

See #311

mathstuf commented 5 years ago

Cc: @demarle

demarle commented 5 years ago

I give it a thumbs up.

jeffamstutz commented 5 years ago

Thanks for contributing this! I'll work on a v1.8.4 release today and get this out ASAP (if everything checks out). FWIW, this is already on the release-1.8.x branch if you'd like to consume it on your end immediately.

jeffamstutz commented 5 years ago

Can you confirm that this does actually fix the issue you were seeing before I cut the v1.8.4 release?

mathstuf commented 5 years ago

With the patch applied, our macOS binaries are much happier now (also worked on my Linux machine). Windows was untested.

jeffamstutz commented 5 years ago

Good enough for me, thanks!

johguenther commented 5 years ago

Thanks for the quick workaround. I still like to put up some thoughts for discussion:

mathstuf commented 5 years ago

if an additional/specific path to search libraries is used, that should come as "last resort", because otherwise users cannot override this (e.g. with LD_PRELOAD, or maybe also with lmod modules)

What is the use case for custom modules? What are the facilities for providing ones own OSPRay backend plugin?

why not use RPATH (at least on Linux and OSX)?

This would work today with an rpath (or runpath) of $ORIGIN. I'm not sure how this would generalize with libospcommon and libospray being in different prefixes and libospray wanting its plugins to be found relative to it (DT_RUNPATH is the New Way™ and is not inherited like DT_RPATH is nevermind what would happen if two libraries in different prefixes end up loading libospcommon since ld.so would elide the second loading).

The VTK way where a symbol name (Windows) or symbol pointer (dladdr) is given to use as the lookup key would work because then the caller of libospcommon could then say "look up relative to the library providing X" where X is something like the _ospray_anchor function I added.

johguenther commented 5 years ago

Thanks for the feedback. In a future release we like libospcommon to be a bit more independent of OSPRay, i.e., not prefixing a path in library.cpp. Instead, this functionality could move into ospLoadModule, i.e., OSPRay will provide a path to look for plugins, like you suggested.