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

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

Open
Nukem9 opened this issue Mar 7, 2024 · 5 comments
Open
Labels
bug Something isn't working ready Changes made and ready for release. Will close when “done”.

Comments

@Nukem9
Copy link
Member

Nukem9 commented Mar 7, 2024

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<ID3D12Resource> 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.
  1. Calling IDXGISwapChain1::ResizeBuffers() suddenly forces ::GetBuffer() to adhere to BufferCount limits from above.
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
Copy link
Collaborator

jake-nv commented Mar 8, 2024

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
Copy link
Collaborator

jake-nv commented Mar 8, 2024

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
Copy link
Collaborator

jake-nv commented Mar 8, 2024

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

@Nukem9
Copy link
Member Author

Nukem9 commented Mar 8, 2024

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
Copy link
Collaborator

jake-nv commented Mar 8, 2024

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.

@jake-nv jake-nv added bug Something isn't working ready Changes made and ready for release. Will close when “done”. labels Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready Changes made and ready for release. Will close when “done”.
Projects
None yet
Development

No branches or pull requests

2 participants