NVIDIAGameWorks / bridge-remix

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

Client may throw a silent exception for some games and crash itself #6

Closed NarutoUA closed 10 months ago

NarutoUA commented 10 months ago

The function uint32_t getBytesFromFormat(const D3DFORMAT& format) throws a silent exception if an unknown format is passed (line 128).

https://github.com/NVIDIAGameWorks/bridge-remix/blob/1a559fa56d2cfcb5558732ece6f828c87cf1acdb/src/util/util_texture_and_volume.h#L118-L129

It's used by calcRowSize func (and some others too): https://github.com/NVIDIAGameWorks/bridge-remix/blob/1a559fa56d2cfcb5558732ece6f828c87cf1acdb/src/util/util_texture_and_volume.h#L140-L144

The calcRowSize function, in turn, is utilized by Direct3DSurface9_LSS::lock: https://github.com/NVIDIAGameWorks/bridge-remix/blob/1a559fa56d2cfcb5558732ece6f828c87cf1acdb/src/client/d3d9_surface.cpp#L191-L196

There is a game called Bounty Bay Online (and possibly other games based on the Storm Engine) that utilizes an internal texture storage format (*.tx), essentially a file with a header + (un?)compressed data: https://github.com/storm-devs/storm-engine/blob/a68f43df197a50c5177f40209abc109f01f4b114/tools/potc-skybox-converter/sky_convert.py#L6-L46

Some textures were overlooked during conversion, and they were shipped as plain image files with the *.tx format, resulting in broken headers, especially the d3dformat. For some reason, ::CreateTexture doesn't fail with invalid d3d format, and the game somehow calls LockRect on it (or it might be an internal call by D3D; I am not sure).

This could also be the case for other games and engines.

As a fix, exception can be removed from getBytesFromFormat (at least in release builds) and Direct3DSurface9_LSS::lock would return false if calcRowSize returns 0

nv-ajaus commented 10 months ago

Hi @NarutoUA , I saw you were talking about this issue on the community discord and had resolved your issues building caused by having a stale checkout. Did your local fix work? I took a look at this and had some concerns about this code needed to be refactored in order to signal up the caller chain correctly, but I was curious to see if you were able to get this game working.

NarutoUA commented 10 months ago

Hi @nv-ajaus , The problem is that dxvk-remix D3D9Device::CreateCubeTexture returns S_OK with even if invalid D3DFORMAT is given. Original d3d9 returns invalid call error. Please check game pseudo-code to understand the issue

bool CreateTextureFromMemory(void* data, uint size, d3dtexture** out)
{
  return CreateTexture(data, size, out) || CreateTextureFromImage(data, size, out);
}

bool CreateTexture(void* data, uint size, d3dtexture** out)
{
  var header = get_header(data); // sometimes data points to .jpg/.tga/.bmp so header is garbage
  if (header.flags & CUBE_TEXTURE) // and header flag could be identified as cube texture
  {
    if (FAILED(pd3dDevice->CreateCubeTexture(header.width, 1, 0, header.format, D3DPOOL_MANAGED, out)) // orig d3d9 returns invalid call err but dxvk-remix returns S_OK
        return false;

     var tdata;
     if (FAILED(*out->LockRect(&tdata)) // crash here before my fix because lockrect threw exception
       return false;

     memcpy(tdata, data+offset, header.size); // crash here after my fix
     *out->UnlockRect();
     return true;
  }
  else
  {
    if (FAILED(pd3dDevice->CreateTexture(header.width, header.height, 1, 0, header.format, D3DPOOL_MANAGED, out))
       return false;
   // ...
}

bool CreateTextureFromImage(void *data, uint size, d3dtexture** out)
{
  return SUCCEEDED(D3DXCreateTextureFromFileInMemory(device, data, size, out));
}

Most of game textures are valid files with correct headers but looks like devs tired of converting textures to game format and added D3DX fallback Valid texture file: image Invalid (plain image): image

I guess you can reproduce it if you call: pd3dDevice->CreateCubeTexture(0xE1FFD8FF, 1, 0, 0x8000000, D3DPOOL_MANAGED, texture);

nv-ajaus commented 10 months ago

Thank you for the update @NarutoUA . I took a quick look at the dxvk-remix code and it looks like the CreateCubeTexture() function (Helper Function That Handles Checking) and at a glance, does attempt to return D3DERR_INVALIDCALL when the format is invalid.

My suspicion is that bridge-remix might be masking this failure by assuming that the call to CreateCubeTexturre() always succeeds, which is an optimization done to improve the performance of the bridge. You can disable this behavior by setting the option sendAllServerResponses = True in the bridge.conf file next to NvRemixBridge.exe in the .trex folder , making sure not to have a # mark before it on the same line.

Additionally, I recently added an improvement ( https://github.com/NVIDIAGameWorks/bridge-remix/commit/2be539a940d3e31a3825c7e2ab14844ec924fbb2) to make Create* functions always return by default, but allowing them to be disabled. This means that this build might correct this issue for you without having to change the sendAllServerResponses` option, if my suspicions are correct.

Can you either try setting the option I mentioned, or trying the bridge build I linked, and seeing if it corrects the issue?

NarutoUA commented 10 months ago

@nv-ajaus thanks for your help. Your bridge build helps with CreateCubeTexture issue, but the game still crashes somewhere else. I would need some time to investigate it but I dont think this crash relates to current issue. I will close it for now.