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

Fix loading of renderdoc library #140

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Conversation

ebkalderon
Copy link
Owner

@ebkalderon ebkalderon commented Jan 27, 2023

Fixed

  • Use RTLD_NOLOAD when loading librenderdoc.so on Unix.
  • Use GetModuleHandle when loading renderdoc.dll on Windows.

The upstream documentation states that the RenderDoc library must only be loaded with libloading if the module is already present and gracefully handle its absence otherwise. This means using the above loading strategies on Unix and Windows instead of libloading::Library::new().

Closes #128.

It turns out this crate has been loading the RenderDoc API incorrectly:

> To do this you'll use your platforms dynamic library functions to see
> if the library is open already - e.g. `GetModuleHandle` on Windows, or
> or `dlopen` with the `RTLD_NOW | RTLD_NOLOAD` flags if available on
> *nix systems. On most platforms you can just search for the module
> name - `renderdoc.dll` on Windows, or `librenderdoc.so` on Linux, or
> `libVkLayer_GLES_RenderDoc.so` on Android should be sufficient here,
> so you don’t need to know the path to where RenderDoc is running from.
> This will vary by platform however so consult your platform’s OS
> documentation. Then you can use `GetProcAddress` or `dlsym` to fetch
> the `RENDERDOC_GetAPI` function using the typedef above.

This was reported long ago as an issue, and I'm addressing this now with
this commit.

It does change the runtime behavior of the library, though: the
application no longer attempts to load RenderDoc into memory if not
injected from the outside, so `RenderDoc::new()` will now return `Err`
when previously it would have succeeded.
This commit adds an unstable `ci` feature which reverts back the
previous non-compliant opening method because it enables tests to run in
CI without needing to painstakingly extract the test binary names from
`cargo build --tests --message-format json` and spawn each one in
`renderdoccmd capture $NAME`. Besides, this strategy is impossible
anyway because doctests compiled on-the-fly by `rustdoc` and do not have
external binaries with which to launch with RenderDoc. We also cannot
make the `RTLD_NOLOAD` conditional, i.e. `#[cfg(any(test, doctest))]`
and `#[cfg(not(any(test, doctest)))]`, due to:

rust-lang/rust#67295

As such, this internal crate feature will have to do.
@ebkalderon ebkalderon self-assigned this Jan 27, 2023
@ebkalderon ebkalderon merged commit 4bef272 into master Jan 27, 2023
@ebkalderon ebkalderon deleted the fix-loading-of-renderdoc-lib branch January 27, 2023 10:54
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

Successfully merging this pull request may close these issues.

Load Using GetModuleHandle on win32 and RTLD_NOLOAD on Unix
1 participant