On 3 October 2016 at 00:38, Józef Kucia jkucia@codeweavers.com wrote:
The swapchain can be destroyed by state_unbind_resources() if the only reference to a swapchain is kept by a shader resource view. This happens in d3d10core and d3d11 tests when a device is destroyed in test_swapchain_flip().
I think it may be harder to fix than this. If state_unbind_resources() can end up destroying the (primary) swapchain, it's not safe to call context_acquire() afterwards. Which means it's unsafe to unload resources or even unload a different SRV. The reason destroying the RTVs is currently safe, is that we unloaded resources earlier, so releasing the references to the resources for those views shouldn't acquire a context to destroy e.g. the GL texture again, but that's already more fragile than I'd like. Perhaps we should just delay destroying the context until leaving the last level.
03.10.2016 3:47 PM "Henri Verbeet" hverbeet@gmail.com:
I think it may be harder to fix than this. If state_unbind_resources() can end up destroying the (primary) swapchain, it's not safe to call context_acquire() afterwards. Which means it's unsafe to unload resources or even unload a different SRV. The reason destroying the RTVs is currently safe, is that we unloaded resources earlier, so releasing the references to the resources for those views shouldn't acquire a context to destroy e.g. the GL texture again, but that's already more fragile than I'd like. Perhaps we should just delay destroying the context until leaving the last level.
I think this fix is correct. The device keeps a reference to the implicit swapchain. The implicit swapchain is destroyed just before leaving from wined3d_device_uninit_3d() so it should be safe to call context_acquire().
This problem occurs only for additional swapchains and that's why it happens only in test_swapchain_flip().
On 3 October 2016 at 17:27, Józef Kucia joseph.kucia@gmail.com wrote:
03.10.2016 3:47 PM "Henri Verbeet" hverbeet@gmail.com:
I think it may be harder to fix than this. If state_unbind_resources() can end up destroying the (primary) swapchain, it's not safe to call context_acquire() afterwards. Which means it's unsafe to unload resources or even unload a different SRV. The reason destroying the RTVs is currently safe, is that we unloaded resources earlier, so releasing the references to the resources for those views shouldn't acquire a context to destroy e.g. the GL texture again, but that's already more fragile than I'd like. Perhaps we should just delay destroying the context until leaving the last level.
I think this fix is correct. The device keeps a reference to the implicit swapchain. The implicit swapchain is destroyed just before leaving from wined3d_device_uninit_3d() so it should be safe to call context_acquire().
This problem occurs only for additional swapchains and that's why it happens only in test_swapchain_flip().
You're probably right. (Although that's not to say the code wouldn't benefit from being a little more robust. I think it's hard to reason about.) Is state_unbind_shaders() required? I assume it's because unbinding the shaders needs to happen before destroying the shader backend, but would delaying the context_acquire() call until after the wine_rb_clear() call also work?
On Tue, Oct 4, 2016 at 10:43 AM, Henri Verbeet hverbeet@gmail.com wrote:
On 3 October 2016 at 17:27, Józef Kucia joseph.kucia@gmail.com wrote:
I think this fix is correct. The device keeps a reference to the implicit swapchain. The implicit swapchain is destroyed just before leaving from wined3d_device_uninit_3d() so it should be safe to call context_acquire().
This problem occurs only for additional swapchains and that's why it happens only in test_swapchain_flip().
You're probably right. (Although that's not to say the code wouldn't benefit from being a little more robust. I think it's hard to reason about.) Is state_unbind_shaders() required? I assume it's because unbinding the shaders needs to happen before destroying the shader backend, but would delaying the context_acquire() call until after the wine_rb_clear() call also work?
Yes, delaying context_acquire() would also work.
-
Henri Verbeet -
Józef Kucia