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
Lock the d3d10 device during presentation #833
Conversation
Interesting, although I'm not sure if this is really the best way to fix this since this thread safety issue is not necessarily exclusive to D3D10. Can you test if this patch works as well? I think the whole D3D10 thread safety thing needs some work anyway since currently both the D3D11 context and D3D10 device use a mutex, which should be redundant since the D3D10 code itself doesn't really do anything. |
6ac8749
to
769575f
Compare
I changed the PR so D3D10Device just enables Multithread protection on the contexts ID3D11Multithread. |
src/d3d10/d3d10_device.cpp
Outdated
m_multithread(this, !(pDevice->GetCreationFlags() & D3D10_CREATE_DEVICE_SINGLETHREADED)) { | ||
|
||
: m_device(pDevice), m_context(pContext) { | ||
m_context->GetMultithread()->SetMultithreadProtected(!(pDevice->GetCreationFlags() & D3D10_CREATE_DEVICE_SINGLETHREADED)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will enable multithreaded protection and thus cause a ~20% performance hit for all D3D11 apps as well, since the d3d10 device is always created. The correct fix would be to do this in D3D10CoreCreateDevice
.
src/d3d11/d3d11_device.cpp
Outdated
*ppvObject = ref(m_d3d11Device.GetD3D10Multithread()); | ||
ID3D11DeviceContext* context; | ||
m_d3d11Device.GetImmediateContext(&context); | ||
*ppvObject = ref(static_cast<D3D11ImmediateContext*>(context)->GetMultithread()); | ||
return S_OK; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to replace these two lines by return context->QueryInterface(riid, ppvObject);
src/d3d11/d3d11_context.h
Outdated
|
||
D3D10Multithread* GetMultithread() { | ||
return &m_multithread; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be necessary when using QueryInterface
as mentioned in the previous comment.
769575f
to
0bc16e7
Compare
src/d3d10/d3d10_main.cpp
Outdated
Com<ID3D10Multithread> multithread; | ||
if (FAILED(d3d11Device->QueryInterface(__uuidof(ID3D10Multithread), reinterpret_cast<void**>(&multithread)))) { | ||
return E_FAIL; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: for consistency, please don't use { ... }
around a single line of code.
0bc16e7
to
762d723
Compare
This prevents fixes threading issues with D3D10 games when Present() gets called. Fixes doitsujin#567.
762d723
to
f30dafa
Compare
Did you succeed and will JC2 run without problems? |
See my comment in #567. Before those changes the game would always deadlock on the splash screen for me. That's fixed now. |
The D3D10Device uses a mutex to avoid accessing the D3D11ImmediateContext from multiple threads at the same time. The new D3D11Swapchain needs to be locked as well because with D3D10 it's not guaranteed that the game only accesses the ImmediateContext from the render thread.
This fixes Just Cause 2 for me. I haven't done that much testing, just 5 minutes of running around an shooting things.
In theory D3D11VkInterop has the same issue because it also accesses the ImmediateContext.
But that is only used with VR and thus D3D11, right?