Wgpu-rs: Curiosity killed the cat - why does owning the window and surface change everything?

[Abendstolz]

_First off:_ If you read this to the end, I've found a solution to the initial problem. Curiosity is my motivation for this ticket. It may very well be, that this is no issue at all, or just one on my side. In any case I'd like to understand it.

Backstory: I am porting an Vulkano based renderer to wgpu. The examples run on my machine, so I jump right into it and start by porting the triangle example while wrapping everything in a Renderer-struct to replace the Vulkano one.

The code below is like it's looking at the moment on my machine. Nothing is stripped, nothing omitted.

// main.rs

mod wgpu_renderer;

const WINDOW_TITLE: &str = "This is a triumph";

pub fn main()  {
    use winit::{
        event_loop::ControlFlow,
        event,
    };

    let (mut gfx, el) = WgpuRenderEngine::new(WINDOW_TITLE, 1920.0, 1080.0);

    el.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Poll;
        match event {
            event::Event::WindowEvent { event, .. } => match event {
                event::WindowEvent::KeyboardInput {
                    input:
                        event::KeyboardInput {
                            virtual_keycode: Some(event::VirtualKeyCode::Escape),
                            state: event::ElementState::Pressed,
                            ..
                        },
                        ..
                }
                | event::WindowEvent::CloseRequested => {
                    *control_flow = ControlFlow::Exit;
                }
                _ => {}
            },
            event::Event::EventsCleared => gfx.render(),
            _ => (),
        }
    });
}

// wgpu_renderer.rs

use winit::{
    event_loop::EventLoop,
    window,
};

pub struct WgpuRenderEngine {
    device: wgpu::Device,
    queue: wgpu::Queue,
    swap_chain: wgpu::SwapChain,
    dummy_pipeline: wgpu::RenderPipeline,
    dummy_bind_group: wgpu::BindGroup,
}

impl WgpuRenderEngine {
    pub fn new<T>(title: T, width: f64, height: f64) -> (Self, EventLoop<()>)
        where T: Into<String>
    {
        // FIXME get rid of the panics and return an appropriate error instead
        let event_loop = EventLoop::new();

        let size = winit::dpi::LogicalSize::new(width, height);
        let window = window::WindowBuilder::new()
            .with_inner_size(size)
            .with_title(title)
            .build(&event_loop)
            .expect("Failed to create a window");

        let surface = wgpu::Surface::create(&window);

        let adapter = wgpu::Adapter::request(&wgpu::RequestAdapterOptions {
            power_preference: wgpu::PowerPreference::HighPerformance,
            backends: wgpu::BackendBit::PRIMARY
        }).expect("Failed to retrieve an adapter");

        let (device, queue) = adapter.request_device(&wgpu::DeviceDescriptor {
            extensions: wgpu::Extensions {
                anisotropic_filtering: false,
            },
            limits: wgpu::Limits::default(),
        });

        let swap_chain = device.create_swap_chain(
            &surface,
            &wgpu::SwapChainDescriptor {
                usage: wgpu::TextureUsage::OUTPUT_ATTACHMENT,
                format: wgpu::TextureFormat::Bgra8UnormSrgb,
                width: size.width.round() as u32,
                height: size.height.round() as u32,
                present_mode: wgpu::PresentMode::Vsync,
            },
        );

        let (dummy_pipeline, dummy_bind_group) = self::shaders::dummy_render_pipeline(&device);

        let moi = Self {
            device,
            queue,
            swap_chain,
            dummy_pipeline,
            dummy_bind_group,
        };

        (moi, event_loop)
    }

    pub fn render(&mut self) {
        let frame = self.swap_chain.get_next_texture();
        let mut encoder = self.device.create_command_encoder(&wgpu::CommandEncoderDescriptor { todo: 0 });
        {
            let mut rpass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
                color_attachments: &[wgpu::RenderPassColorAttachmentDescriptor {
                    attachment: &frame.view,
                    resolve_target: None,
                    load_op: wgpu::LoadOp::Clear,
                    store_op: wgpu::StoreOp::Store,
                    clear_color: wgpu::Color::GREEN,
                }],
                depth_stencil_attachment: None,
            });
            rpass.set_pipeline(&self.dummy_pipeline);
            rpass.set_bind_group(0, &self.dummy_bind_group, &[]);
            rpass.draw(0 .. 3, 0 .. 1);
        }
        self.queue.submit(&[encoder.finish()]);
    }
}

mod shaders {
    pub fn dummy_render_pipeline(device: &wgpu::Device) -> (wgpu::RenderPipeline, wgpu::BindGroup) {
        let vs = include_bytes!("shader.vert.spv");
        let vs_module = device.create_shader_module(&wgpu::read_spirv(std::io::Cursor::new(&vs[..])).unwrap());

        let fs = include_bytes!("shader.frag.spv");
        let fs_module = device.create_shader_module(&wgpu::read_spirv(std::io::Cursor::new(&fs[..])).unwrap());

        let bind_group_layout = device.create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
            bindings: &[],
        });
        let bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
            layout: &bind_group_layout,
            bindings: &[],
        });
        let pipeline_layout = device.create_pipeline_layout(&wgpu::PipelineLayoutDescriptor {
            bind_group_layouts: &[&bind_group_layout],
        });

        let pipeline = device.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
            layout: &pipeline_layout,
            vertex_stage: wgpu::ProgrammableStageDescriptor {
                module: &vs_module,
                entry_point: "main",
            },
            fragment_stage: Some(wgpu::ProgrammableStageDescriptor {
                module: &fs_module,
                entry_point: "main",
            }),
            rasterization_state: Some(wgpu::RasterizationStateDescriptor {
                front_face: wgpu::FrontFace::Ccw,
                cull_mode: wgpu::CullMode::None,
                depth_bias: 0,
                depth_bias_slope_scale: 0.0,
                depth_bias_clamp: 0.0,
            }),
            primitive_topology: wgpu::PrimitiveTopology::TriangleList,
            color_states: &[wgpu::ColorStateDescriptor {
                format: wgpu::TextureFormat::Bgra8UnormSrgb,
                color_blend: wgpu::BlendDescriptor::REPLACE,
                alpha_blend: wgpu::BlendDescriptor::REPLACE,
                write_mask: wgpu::ColorWrite::ALL,
            }],
            depth_stencil_state: None,
            index_format: wgpu::IndexFormat::Uint16,
            vertex_buffers: &[],
            sample_count: 1,
            sample_mask: !0,
            alpha_to_coverage_enabled: false,
        });

        (pipeline, bind_group)
    }
}

The compiled shaders I copied from https://github.com/gfx-rs/wgpu-rs/tree/master/examples/hello-triangle

Running this, leads to GPU took too much time processing last frames :(:

cargo r --release
warning: unused manifest key: bin.0.include
    Finished release [optimized] target(s) in 0.08s
     Running `target/release/client`
thread 'main' panicked at 'GPU took too much time processing last frames :(', /home/abendstolz/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/78fbbba/wgpu-native/src/swap_chain.rs:150:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

Okay, what now? The example works just fine. I verify this again by replacing my code with it. All works, so I start playing around. This is the version that works:

pub struct WgpuRenderEngine {
    device: wgpu::Device,
    queue: wgpu::Queue,
    swap_chain: wgpu::SwapChain,
    window: winit::window::Window,
    surface: wgpu::Surface,
    dummy_pipeline: wgpu::RenderPipeline,
    dummy_bind_group: wgpu::BindGroup,
}

impl WgpuRenderEngine {
    pub fn new<T>(title: T, width: f64, height: f64) -> (Self, EventLoop<()>)
        where T: Into<String>
    {
        // FIXME get rid of the panics and return an appropriate error instead
        let event_loop = EventLoop::new();

        let size = winit::dpi::LogicalSize::new(width, height);
        let window = window::WindowBuilder::new()
            .with_inner_size(size)
            .with_title(title)
            .build(&event_loop)
            .expect("Failed to create a window");

        let surface = wgpu::Surface::create(&window);

        let adapter = wgpu::Adapter::request(&wgpu::RequestAdapterOptions {
            power_preference: wgpu::PowerPreference::HighPerformance,
            backends: wgpu::BackendBit::PRIMARY
        }).expect("Failed to retrieve an adapter");

        let (device, queue) = adapter.request_device(&wgpu::DeviceDescriptor {
            extensions: wgpu::Extensions {
                anisotropic_filtering: false,
            },
            limits: wgpu::Limits::default(),
        });

        let swap_chain = device.create_swap_chain(
            &surface,
            &wgpu::SwapChainDescriptor {
                usage: wgpu::TextureUsage::OUTPUT_ATTACHMENT,
                format: wgpu::TextureFormat::Bgra8UnormSrgb,
                width: size.width.round() as u32,
                height: size.height.round() as u32,
                present_mode: wgpu::PresentMode::Vsync,
            },
        );

        let (dummy_pipeline, dummy_bind_group) = self::shaders::dummy_render_pipeline(&device);

        let moi = Self {
            device,
            queue,
            swap_chain,
            window,
            surface,
            dummy_pipeline,
            dummy_bind_group,
        };

        (moi, event_loop)
    }

    pub fn render(&mut self) {
        let frame = self.swap_chain.get_next_texture();
        let mut encoder = self.device.create_command_encoder(&wgpu::CommandEncoderDescriptor { todo: 0 });
        {
            let mut rpass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
                color_attachments: &[wgpu::RenderPassColorAttachmentDescriptor {
                    attachment: &frame.view,
                    resolve_target: None,
                    load_op: wgpu::LoadOp::Clear,
                    store_op: wgpu::StoreOp::Store,
                    clear_color: wgpu::Color::GREEN,
                }],
                depth_stencil_attachment: None,
            });
            rpass.set_pipeline(&self.dummy_pipeline);
            rpass.set_bind_group(0, &self.dummy_bind_group, &[]);
            rpass.draw(0 .. 3, 0 .. 1);
        }
        self.queue.submit(&[encoder.finish()]);
    }
}

So, the only difference is that I now save window and surface in the renderer struct. They are not used anywhere so far (although I will need them inside the renderer, so owning them makes perfect sense).

I jump between the two states and it's working <-> not working with the above error. No additional changes whatsoever.

I can only begin to imagine stuff that could happen under the hood, like some unsafe-pointer magic which leads to stuff being dropped, if it's not owned and goes out of scope.

But instead of guessing I would love to understand what's really happening here. Obviously rustc didn't save my ass this time.

Just in case it matters, here is my GPU info (and OS/kernel)

~ glxinfo | grep OpenGL

OpenGL vendor string: X.Org
OpenGL renderer string: AMD Radeon (TM) RX 470 Graphics (POLARIS10, DRM 3.33.0, 5.3.7-arch1-1-ARCH, LLVM 9.0.0)
OpenGL core profile version string: 4.5 (Core Profile) Mesa 19.2.2
OpenGL core profile shading language version string: 4.50
OpenGL core profile context flags: (none)
OpenGL core profile profile mask: core profile
OpenGL core profile extensions:
OpenGL version string: 4.5 (Compatibility Profile) Mesa 19.2.2
OpenGL shading language version string: 4.50
OpenGL context flags: (none)
OpenGL profile mask: compatibility profile
OpenGL extensions:
OpenGL ES profile version string: OpenGL ES 3.2 Mesa 19.2.2
OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.20
OpenGL ES profile extensions:

```Cargo.toml
[dependencies]
...

Graphics

wgpu = { git = "https://github.com/gfx-rs/wgpu-rs", rev = "ed2c67f762970d0099c1e6c6e078fb645afbf964" }
winit = "0.20.0-alpha4"
...
```
If you need additional details, I am happy to provide them and whatever else helps you solve this "mystery" (maybe it's totally obvious to you!)

[sotrh]

A lot of structs that deal with native code implement the Drop trait. This means that when they go out of scope, they'll free up some data on the native end. The Window struct implements Drop, so when it goes out of scope, it cleanups resources that wgpu needs in order to function properly.

As far as surface goes, it doesn't implement Drop, so I'm not sure if it needs to stick around, but I keep it around because you need it to recreate the swap chain for resizes and stuff.

One thing to note is you don't need to own the window. It just needs to stay in scope for as long as you want to use it's resources. I personally pass the window in as a reference, that way the rendering code doesn't depend on it, and there's no unused window field cluttering the struct.

[Abendstolz]

Thanks @sotrh - I should've made it clearer, that I know that it doesn't need to be owned, but stay in scope. After all that's exactly how the example(s) here work as well.

The drop makes sense, the error message could be improved upon from a user's perspective but I have no idea how complicated that would be.

I'll see if I need the window inside the renderer.
For the vulkano based one it was convenient, because all the render logic was abstracted away and I could've easily swapped out winit with sdl2 or glfw. (This code was created a long time before https://crates.io/crates/raw-window-handle was a thing :))