Winit renderer initialisation cleanup #2651
Conversation
Replace qualifier with use.
The creation of the renderer might fail, for example if we tried to create an OpenGL renderer on a display that doesn't support OpenGL. We should catch that in the future and fall back to the software renderer.
| let platform_error: Cell<Option<PlatformError>> = Default::default(); | ||
| let mut renderer: once_cell::unsync::OnceCell<Renderer> = Default::default(); | ||
|
|
||
| match Renderer::new(&(self_weak.clone() as _)) { | ||
| Ok(new_renderer) => { | ||
| renderer = once_cell::unsync::OnceCell::with_value(new_renderer) | ||
| } | ||
| Err(err) => platform_error.set(Some(err)), | ||
| }; | ||
|
|
There was a problem hiding this comment.
This is annoyingly ugly, largely AFAICS because the renderer's need a dyn crate::window::WindowAdapter eventually. This includes the public SoftwareRenderer::new() API.
There was a problem hiding this comment.
Yes, i'm afraid this is the only way for now.
rust-lang/libs-team#90
One think that can a bit be improved would be to move the platform_error in the parent scope instead of in the Rc itself.
(See the code example in rust-lang/libs-team#90 (comment) )
There was a problem hiding this comment.
Ah yes, that would be nice. But then all the WinitWindowAdapter would have to support Default, i.e. corelib::api::Window have to go into a OnceCell. I'll give it a try to see how bad the code becomes - if too bad then I'll stick with this version.
Instead leave it out and return Default::default() in Rc::new_cyclic(). The map state handling gets quite ugly, because it needs to go into a OnceCell. On the upside, if we succeed with creating the winit window in new() in the future, then that entire cell goes away.
Prepare for the situation where creating a renderer may fail.