NVIDIAGameWorks / Streamline

Streamline Integration Framework
Other
377 stars 79 forks source link

Proxy swap chain back buffer counts are inconsistent with dlssg loaded #35

Open Nukem9 opened 6 months ago

Nukem9 commented 6 months ago

FWIW, I'm not entirely sure of the extent to which these are bugs or intentional behavior. I didn't see anything mentioned in the documentation. Each code block assumes factory, commandQueue, and swapChain interfaces were created through sl.interposer proxy methods.

  1. IDXGISwapChain1::GetDesc() and IDXGISwapChain1::GetDesc1() return incorrect BufferCount values. ::GetBuffer() doesn't match.
    
    dlssgOptions.mode = sl::DLSSGMode::eOff;
    Assert(slDLSSGSetOptions(0, dlssgOptions) == sl::Result::eOk);                        // dlssg explicitly off, SL feature still enabled

swapChainDesc.BufferCount = 3; factory->CreateSwapChainForHwnd(commandQueue.Get(), hwnd, &swapChainDesc, nullptr, nullptr, &swapChain);

for (uint32_t i = 0; i < swapChainDesc.BufferCount; i++) // swapChainDesc.BufferCount is 3. GetBuffer() succeeds 3 times. { CComPtr buffer; Assert(swapChain->GetBuffer(i, IID_PPV_ARGS(&buffer)) == S_OK); }

DXGI_SWAP_CHAIN_DESC t; Assert(swapChain->GetDesc(&t) == S_OK && t.BufferCount == swapChainDesc.BufferCount); // Assertion fails. t.BufferCount is 2. 2 != 3.


2. Calling IDXGISwapChain1::ResizeBuffers() suddenly forces ::GetBuffer() to adhere to BufferCount limits from above.
```cpp
dlssgOptions.mode = sl::DLSSGMode::eOff;
Assert(slDLSSGSetOptions(0, dlssgOptions) == sl::Result::eOk);                        // dlssg explicitly off, SL feature still enabled

swapChainDesc.BufferCount = 3;
factory->CreateSwapChainForHwnd(commandQueue.Get(), hwnd, &swapChainDesc, nullptr, nullptr, &swapChain);

Assert(swapChain->ResizeBuffers(0, 0, 0, DXGI_FORMAT_UNKNOWN, 0) == S_OK);

for (uint32_t i = 0; i < swapChainDesc.BufferCount; i++)                              // swapChainDesc.BufferCount is 3.
{
    CComPtr<ID3D12Resource> buffer;
    Assert(swapChain->GetBuffer(i, IID_PPV_ARGS(&buffer)) == S_OK);                   // Assertion failure when i == 2.
}
  1. Calling IDXGISwapChain1::ResizeBuffers() causes presentation to fail entirely...? Passing 0s should preserve original values.
    
    dlssgOptions.mode = sl::DLSSGMode::eOff;
    Assert(slDLSSGSetOptions(0, dlssgOptions) == sl::Result::eOk);                        // dlssg explicitly off, SL feature still enabled

factory->CreateSwapChainForHwnd(commandQueue.Get(), hwnd, &swapChainDesc, nullptr, nullptr, &swapChain);

Assert(swapChain->ResizeBuffers(0, 0, 0, DXGI_FORMAT_UNKNOWN, 0) == S_OK);

while (true) Assert(swapChain->Present(0, 0) == S_OK); // Succeeds, yet the screen never updates. Resource creation errors are printed to SL's log.



I ended up treating values from GetDesc() as the "ground truth" and that's enough for 1 & 2. 3 is slightly more annoying because ResizeBuffers() parameters don't seem to be validated. Manually specifying count, width, and height is a workaround. Personally I'd consider these bugs since they manifest even while dlssg is off.
jake-nv commented 6 months ago

Some changes related to correcting the behavior of IDXGISwapChain::GetDesc() and GetBuffer() are in the upcoming release. I think it will fix most (all?) of the... inconsistencies.

jake-nv commented 6 months ago

Picking through the commits I see:

  1. FG buffers will reflect the number requested by app
  2. GetDesc() is hooked to provide the actual buffer count
  3. Improved GetBuffer() error handling
jake-nv commented 6 months ago

Nothing about ResizeBuffers()... that might still need work.

Nukem9 commented 6 months ago

I'm going to hijack this issue a bit and ask: is it worth opening new issues for trivial code? e.g. problems that've shown up while debugging integration, but nothing during typical gameplay. I see external PRs aren't accepted and I'd rather avoid wasting too much SL maintainer time >;)

jake-nv commented 6 months ago

Yeah, we're trying to be better about that. MRs are still a bit of a challenge for reasons I can't get into.

If it's really trivial (like, <10 lines trivial) it's probably better/faster/easier to just open an issue with the repro steps and the change.