Allow disabling waiting for latency waitable object #7400
Conversation
`wait_for_latency_object` method that allows waiting manually for the waitable handle
wgpu-hal/src/dx12/mod.rs
Outdated
| pub unsafe fn wait_for_frame_latency_object( | ||
| &self, | ||
| timeout: Option<std::time::Duration>, | ||
| ) -> Result<bool, crate::SurfaceError> { |
There was a problem hiding this comment.
This feels weird being pub
There was a problem hiding this comment.
My idea was that you don't need to expose the actual waitable handle this way. So wgpu can still handle the configuration of the swap chain and creation of the waitable object but we can abstract away the actual unsafe syscalls.
There was a problem hiding this comment.
I think for this particular use case (user demands a certain usage of the waitable object) - it's reasonable to expose the raw handle so they can do whatever they want to do with it (maybe wait for multiple of them at once or something similarly funky.
There was a problem hiding this comment.
Yes, that's fine! I'll expose the waitable object and make the wait method private.
Yes, I thought that's the best way to deal with this. There's no documented downside to setting this flag, but with my (very limited) DX12 experience, that's not to say there are no side effects at all. So it might be better to at least give users an option to disable it. |
|
Oh yes, very true, I wouldn't expect there to be any side effects for d3d12. |
| pub unsafe fn waitable_handle(&self) -> Option<Foundation::HANDLE> { | ||
| let swapchain = self.swap_chain.read(); | ||
| if let Some(swap_chain) = swapchain.as_ref() { | ||
| return swap_chain.waitable.clone(); |
There was a problem hiding this comment.
This implies that Handles are refcounted, but they are Copy aliases around *const c_void - we should return it by copy
There was a problem hiding this comment.
so just drop the clone()?
| } | ||
|
|
||
| unsafe impl Send for Surface {} | ||
| unsafe impl Sync for Surface {} | ||
|
|
||
| impl Surface { | ||
| /// Returns the waitable handle of the swapchain, if it exists. |
There was a problem hiding this comment.
We should also define the lifetime of the handle
There was a problem hiding this comment.
Define the lifetime in a SAFETEY comment or as an actual Rust lifetime?
There was a problem hiding this comment.
Just as a safety comment, saying it's valid until the surface is reconfigured or unconfigured or whatever
|
Looks good, some small nits, then g2g |
This PR allows you to skip the frame latency waitable mechanism used for the DX12 backend by adding a new option to
Dx12BackendOptions. The options now also carry over to the DX12Surface, and a waitable object is only obtained whenuse_latency_waitable_object == true. The swap chain still gets created with theDXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECTflag, because I don't think there are any downsides and it lets users still obtain a waitable object themselves.Not sure if this is the best way to do this or the right place for this setting, so let me know what you think. Happy to also change the docs.