[WIP][pre-ll] better dx11 resize support, gamma port #1565
Conversation
examples/gamma/main.rs
Outdated
| @@ -50,21 +56,46 @@ const CLEAR_COLOR: [f32; 4] = [0.5, 0.5, 0.5, 1.0]; | |||
|
|
|||
| type SurfaceData = <<ColorFormat as Formatted>::Surface as SurfaceTyped>::DataType; | |||
|
|
|||
| enum Backend { | |||
| #[cfg(not(target_os = "windows"))] | |||
| Gl(glutin::GlWindow, gfx::handle::DepthStencilView<gfx_window_glutin::Resources, DepthFormat>), | |||
There was a problem hiding this comment.
gfx_window_glutin::Resources does not exist, although it could be a reexport of gfx_device_gl::Resources.
There was a problem hiding this comment.
right, should be fixed now
examples/gamma/main.rs
Outdated
| @@ -50,21 +56,46 @@ const CLEAR_COLOR: [f32; 4] = [0.5, 0.5, 0.5, 1.0]; | |||
|
|
|||
| type SurfaceData = <<ColorFormat as Formatted>::Surface as SurfaceTyped>::DataType; | |||
|
|
|||
| enum Backend { | |||
There was a problem hiding this comment.
Why use a single variant enum instead of a tuple struct ?
There was a problem hiding this comment.
that came from my original attempt to make it a run-time switch. Fixed now
| pipe::new() | ||
| ).unwrap(); | ||
|
|
||
| let (mut backend, mut device, mut factory, pso, main_color) = match () { |
There was a problem hiding this comment.
Why not use {} instead of match () { _ => {} } ?
There was a problem hiding this comment.
{} in expressions is not allowed, and copying the whole starting line for both cases would be unfortunate
There was a problem hiding this comment.
Maybe you could use if cfg! together with else?
There was a problem hiding this comment.
Ah the code won't compile, sorry.
| }, | ||
| Err(e) => error!("Resize failed: {}", e), | ||
| } | ||
| //TODO: clean up the old references first! |
There was a problem hiding this comment.
How do you plan to do that ? Some interior mutability might enable the DX11 backend to update the existing reference or to invalidate it. Another possibility could be to have some kind of generation id to avoid reference mutation while still invalidating it.
There was a problem hiding this comment.
Sure, good ideas!
It's not a big priority for me now to make this work universally on Dx11. The client doesn't use gfx_app, they just need a way to read back textures.
| use core::Factory; | ||
| // first, replace the main view pointer with a dummy | ||
| // the code assumes this is the last standing reference, beware! | ||
| *main_color = factory.create_render_target(1, 1).unwrap().2; |
There was a problem hiding this comment.
I'm not sure what's the purpose of this dummy target ? Looking at #1122 makes me believe that it's meant to propagate it to the application resize handling, which is not possible at this crate level. You could drop the given handle and return a new one ?
There was a problem hiding this comment.
You could drop the given handle and return a new one ?
dropping is fine, but then you'd have to do some magic to prevent a double drop when the new one needs to overwrite the old one.
I chose the simplest approach here - overwrite the handle with a dummy target temporarily, so that the old one is freed.
examples/gamma/main.rs
Outdated
| @@ -50,21 +56,46 @@ const CLEAR_COLOR: [f32; 4] = [0.5, 0.5, 0.5, 1.0]; | |||
|
|
|||
| type SurfaceData = <<ColorFormat as Formatted>::Surface as SurfaceTyped>::DataType; | |||
|
|
|||
| enum Backend { | |||
There was a problem hiding this comment.
that came from my original attempt to make it a run-time switch. Fixed now
examples/gamma/main.rs
Outdated
| @@ -50,21 +56,46 @@ const CLEAR_COLOR: [f32; 4] = [0.5, 0.5, 0.5, 1.0]; | |||
|
|
|||
| type SurfaceData = <<ColorFormat as Formatted>::Surface as SurfaceTyped>::DataType; | |||
|
|
|||
| enum Backend { | |||
| #[cfg(not(target_os = "windows"))] | |||
| Gl(glutin::GlWindow, gfx::handle::DepthStencilView<gfx_window_glutin::Resources, DepthFormat>), | |||
There was a problem hiding this comment.
right, should be fixed now
| pipe::new() | ||
| ).unwrap(); | ||
|
|
||
| let (mut backend, mut device, mut factory, pso, main_color) = match () { |
There was a problem hiding this comment.
{} in expressions is not allowed, and copying the whole starting line for both cases would be unfortunate
| }, | ||
| Err(e) => error!("Resize failed: {}", e), | ||
| } | ||
| //TODO: clean up the old references first! |
There was a problem hiding this comment.
Sure, good ideas!
It's not a big priority for me now to make this work universally on Dx11. The client doesn't use gfx_app, they just need a way to read back textures.
| use core::Factory; | ||
| // first, replace the main view pointer with a dummy | ||
| // the code assumes this is the last standing reference, beware! | ||
| *main_color = factory.create_render_target(1, 1).unwrap().2; |
There was a problem hiding this comment.
You could drop the given handle and return a new one ?
dropping is fine, but then you'd have to do some magic to prevent a double drop when the new one needs to overwrite the old one.
I chose the simplest approach here - overwrite the handle with a dummy target temporarily, so that the old one is freed.
|
Shall we close this one? |
|
Yeah... there has been a few hours put into this that I'd not like to see wasted, but nobody is interested in the outcome atm, so no need to leave it hanging here. |
Findings on the way to #1564
RFC!
cc @Bastacyclop @zakorgy