Conversation
5a53a68 to
de22168
Compare
There was a problem hiding this comment.
If you parameterize the Surface by the actual window type, you can avoid the weird 'static mutation and instead just pass around the Surface wrapping an Arc<Window> or something. See the softbuffer version of this PR for more info (rust-windowing/softbuffer#132)
I don't think this would work as well in Wgpu as it did in Softbuffer. Wgpu has a bunch of methods that can create a Maybe I'm missing something here, let me know. |
de22168 to
09fc22c
Compare
|
Rebased on |
|
What about the “ |
My assumption here is that's what @notgull would you be able to comment on that? |
This happens in
This is true; while a |
I tried this idea, but again, Wgpu doesn't make this straightforward. I have to pass down the generic now somehow in all the functions that take I could continue to play around with all this, but would like to hear a word from a member if this is desired in the first place or not. |
09fc22c to
96ec738
Compare
|
Rebased on trunk, no changes. |
|
I think I used dynamic dispatch and |
4c74c15 to
1c692ef
Compare
|
Rebased on master. |
|
So making surface creation safe is something we definitely want, I don't think this is quite the way to do it though. Notably, we only actually need the very surface We should have create_surface be: trait WgpuSurfaceRequirement: HasWindowHandle + HasDisplayHandle {}
pub fn create_surface<'a, W>(&self, window: W) -> Result<Surface<'a>, CreateSurfaceError>
where
W: WgpuSurfaceRequirement + 'a,Note that there isn't actually a generic here. We can store This prevents the generics from infecting user code, and allows the flexibility to store both references and owned values as wgpu-core and below behave like they did before with raw handles everywhere. |
cwfitzgerald
left a comment
There was a problem hiding this comment.
Please re-request a review from me once the changes are addressed to make sure I see it!
|
@cwfitzgerald I'm unsure what exactly your request is. I looked over my changes again, but I don't believe I introduced any generics. Maybe you were just pointing out that we don't want to introduce a generic based on the conversation I had with nogull before? In any case, the current implementation seems to be exactly what you are requesting, including not touching anything else then Apologies for any misunderstanding. |
1c692ef to
ed58ab3
Compare
|
Rebased on master. |
b2afcc2 to
3d57eba
Compare
cwfitzgerald
left a comment
There was a problem hiding this comment.
Code is good, just style and docs.
5aca1cb to
a072070
Compare
|
LGTM after comment! |
3e2d0bf to
c3f847c
Compare
|
Just rebased onto trunk again. |
* doc: as of #4597 surface creation is no longer unsafe * doc: extend documentation of the include_wgsl macro
* doc: as of gfx-rs#4597 surface creation is no longer unsafe * doc: extend documentation of the include_wgsl macro
Now that we use
raw-window-handlev0.6 (#4202), safeSurfacecreation should be possible. This is done by adding a lifetime toSurfacethat is bound to the passedwindow.Additionally I added
Surface::create_surface_from_raw()which still takes aHasWindowHandle + HasDisplayHandlebut remainsunsafeand returns aSurface<'static>.Fixes #1463.