NVIDIAGameWorks / bridge-remix

This is the NVIDIA RTX Remix Runtime Bridge repository
MIT License
157 stars 24 forks source link

BridgeApi - RemixApi wrapper for x86 games #12

Closed xoxor4d closed 1 month ago

xoxor4d commented 4 months ago

This PR adds a intermediary api called BridgeApi to the bridge that allows x86 games to use parts of the x64 remix-runtime api.

Overview:

Behavior on init:

Usage:

// somewhere after the api was initialized bridge.RegisterEndSceneCallback(on_endscene);



### Notes:
- ~~I've set client option `client.exposeRemixApi` to `true` by default~~ (now false by default)

- I'm using a mutex to get the device [here](https://github.com/xoxor4d/bridge-remix/blob/a874b5dea53843f5943b88c575a8ee63fb2370ec/src/server/module_processing.cpp#L88). (I don't have much experience working with threads -> not sure if correct)

- I've directly included `remix_c.h` from `dxvk-remix` (should be made a dependency that's pulled?)

- There is no RemixApi version check nor does `bridge_c.h` expose any version

- Sending `const char*` or `wchar_t*` via rpc can result in the string no longer having the null terminator in the correct location. This can lead to strings with "junk data" attached to them. The returned length/size is correct tho. This also happens for `Bridge_DebugMessage`. This leads to special handling in every api case in `server/main.cpp` that`s using strings:

  - [`Api_DebugPrint`](https://github.com/xoxor4d/bridge-remix/blob/a874b5dea53843f5943b88c575a8ee63fb2370ec/src/server/main.cpp#L2714) using `std::string(data, length)` to construct a new string with the proper length
  -  [`Api_CreateOpaqueMaterial`](https://github.com/xoxor4d/bridge-remix/blob/a874b5dea53843f5943b88c575a8ee63fb2370ec/src/server/main.cpp#L2720) using `NVPULL_PATH` -> `std::wstring(data, length)` to construct a new wstring with the proper length

_____

#### Example usage of almost every feature:
- [iw3xo-dev/blob/feature/remix-api/src/components/modules/rtx/rtx_gui.cpp](https://github.com/xoxor4d/iw3xo-dev/blob/feature/remix-api/src/components/modules/rtx/rtx_gui.cpp) (search for `Developer`)
- [iw3xo-dev/blob/feature/remix-api/src/components/modules/rtx/rtx_api.cpp](https://github.com/xoxor4d/iw3xo-dev/blob/feature/remix-api/src/components/modules/rtx/rtx_api.cpp)
xoxor4d commented 4 months ago

@nv-nfreybler Firstly, thanks for taking the time to review this! I've made the required changes and squashed everything into one commit. I've remove almost all traces of bridgeApi as requested with the exception to the bridge_c header file as I'm not sure if the contents will stay in a separate file or if they'll be included within the remix_c header?

nv-nfreybler commented 3 months ago

Thanks for making all these changes! A couple things to note:

  1. Part of the reason for my delay in getting back to you is because we've been implementing a versioning handshake.
  2. When we're ready, the next step of the process will involve taking in your changes so we can do some internal testing, and make further additions. Please be patient while we deliberate on this PR, in addition to our internal set of tasks.

I've remove almost all traces of bridgeApi as requested with the exception to the bridge_c header file as I'm not sure if the contents will stay in a separate file or if they'll be included within the remix_c header?

Thanks for doing that, I know it can be a long and tedious task to rename things. I appreciate your calling it out in particular, as we are actively discussing the bridge_c header file in particular. A decision will likely be reached in our own internal merge commit.

nv-nfreybler commented 2 months ago

Just wanted to post an update here that we're currently actively internally reviewing and working on this PR, for the better part of the last couple weeks, to be honest. We appreciate your patience, and want to assure you that it is not forgotten.

xoxor4d commented 2 months ago

Thank you for the update, really appreciate it! I hope this hasn't caused too much trouble internally 🙂

nv-nfreybler commented 2 months ago

Please see commit 2dd0a0e. Binaries

We overhauled quite a bit from the original PR, mostly to eliminate the code duplication between the original bridge_c.h and remix_c.h. We also wanted 1:1 parity with the standard Remix API, so we went with a singular CreateLight etc as opposed to extension-specific APIs. Additionally we found some memory leaks, and needed to refactor accordingly.

Thank you again for the PR. If you consider this sufficient, please go ahead and close this, or else we can.

xoxor4d commented 1 month ago

I'll now close the PR because the latest builds are running great and everything seems to be ironed out and working as expected. Communication was great and it was a blast working on this. Thanks again to all the people involved! I really appreciate you all and am looking forward to the next PR 🙂