Scope guard types

[Imberflur]

The way the project I work on handles state transitions between e.g. pipelines while drawing with wgpu it isn't feasible to put things inside a scope macro. So I made the scope types (see below) which wrap the CommandEncoder/RenderPass and utilize Drop. I was wondering if something like these would be useful in this crate or make more sense in a separate crate or simply created by users as needed?

pub struct Scope<'a, W: ProfilerCommandRecorder> {
    profiler: &'a mut GpuProfiler,
    wgpu_thing: &'a mut W,
}

pub struct OwningScope<'a, W: ProfilerCommandRecorder> {
    profiler: &'a mut GpuProfiler,
    wgpu_thing: W,
}

// Separate type since we can't destructure types that impl Drop :/
pub struct ManualOwningScope<'a, W: ProfilerCommandRecorder> {
    profiler: &'a mut GpuProfiler,
    wgpu_thing: W,
}

impl<'a, W: ProfilerCommandRecorder> Scope<'a, W> {
    pub fn start(
        profiler: &'a mut GpuProfiler,
        wgpu_thing: &'a mut W,
        device: &wgpu::Device,
        label: &str,
    ) -> Self {
        profiler.begin_scope(label, wgpu_thing, device);
        Self {
            profiler,
            wgpu_thing,
        }
    }

    /// Starts a scope nested within this one
    pub fn scope(&mut self, device: &wgpu::Device, label: &str) -> Scope<'_, W> {
        Scope::start(self.profiler, self.wgpu_thing, device, label)
    }
}

impl<'a, W: ProfilerCommandRecorder> OwningScope<'a, W> {
    pub fn start(
        profiler: &'a mut GpuProfiler,
        mut wgpu_thing: W,
        device: &wgpu::Device,
        label: &str,
    ) -> Self {
        profiler.begin_scope(label, &mut wgpu_thing, device);
        Self {
            profiler,
            wgpu_thing,
        }
    }

    /// Starts a scope nested within this one
    pub fn scope(&mut self, device: &wgpu::Device, label: &str) -> Scope<'_, W> {
        Scope::start(self.profiler, &mut self.wgpu_thing, device, label)
    }
}

impl<'a, W: ProfilerCommandRecorder> ManualOwningScope<'a, W> {
    pub fn start(
        profiler: &'a mut GpuProfiler,
        mut wgpu_thing: W,
        device: &wgpu::Device,
        label: &str,
    ) -> Self {
        profiler.begin_scope(label, &mut wgpu_thing, device);
        Self {
            profiler,
            wgpu_thing,
        }
    }

    /// Starts a scope nested within this one
    pub fn scope(&mut self, device: &wgpu::Device, label: &str) -> Scope<'_, W> {
        Scope::start(self.profiler, &mut self.wgpu_thing, device, label)
    }

    /// Ends the scope allowing the extraction of owned the wgpu thing
    /// and the mutable reference to the GpuProfiler
    pub fn end_scope(mut self) -> (W, &'a mut GpuProfiler) {
        self.profiler.end_scope(&mut self.wgpu_thing);
        (self.wgpu_thing, self.profiler)
    }
}
impl<'a> Scope<'a, wgpu::CommandEncoder> {
    /// Start a render pass wrapped in an OwnedScope
    pub fn scoped_render_pass<'b>(
        &'b mut self,
        device: &wgpu::Device,
        label: &str,
        pass_descriptor: &wgpu::RenderPassDescriptor<'b, '_>,
    ) -> OwningScope<'b, wgpu::RenderPass> {
        let render_pass = self.wgpu_thing.begin_render_pass(pass_descriptor);
        OwningScope::start(self.profiler, render_pass, device, label)
    }
}

impl<'a> OwningScope<'a, wgpu::CommandEncoder> {
    /// Start a render pass wrapped in an OwnedScope
    pub fn scoped_render_pass<'b>(
        &'b mut self,
        device: &wgpu::Device,
        label: &str,
        pass_descriptor: &wgpu::RenderPassDescriptor<'b, '_>,
    ) -> OwningScope<'b, wgpu::RenderPass> {
        let render_pass = self.wgpu_thing.begin_render_pass(pass_descriptor);
        OwningScope::start(self.profiler, render_pass, device, label)
    }
}

impl<'a> ManualOwningScope<'a, wgpu::CommandEncoder> {
    /// Start a render pass wrapped in an OwnedScope
    pub fn scoped_render_pass<'b>(
        &'b mut self,
        device: &wgpu::Device,
        label: &str,
        pass_descriptor: &wgpu::RenderPassDescriptor<'b, '_>,
    ) -> OwningScope<'b, wgpu::RenderPass> {
        let render_pass = self.wgpu_thing.begin_render_pass(pass_descriptor);
        OwningScope::start(self.profiler, render_pass, device, label)
    }
}

// Scope
impl<'a, W: ProfilerCommandRecorder> std::ops::Deref for Scope<'a, W> {
    type Target = W;

    fn deref(&self) -> &Self::Target { self.wgpu_thing }
}

impl<'a, W: ProfilerCommandRecorder> std::ops::DerefMut for Scope<'a, W> {
    fn deref_mut(&mut self) -> &mut Self::Target { self.wgpu_thing }
}

impl<'a, W: ProfilerCommandRecorder> Drop for Scope<'a, W> {
    fn drop(&mut self) { self.profiler.end_scope(self.wgpu_thing); }
}

// OwningScope
impl<'a, W: ProfilerCommandRecorder> std::ops::Deref for OwningScope<'a, W> {
    type Target = W;

    fn deref(&self) -> &Self::Target { &self.wgpu_thing }
}

impl<'a, W: ProfilerCommandRecorder> std::ops::DerefMut for OwningScope<'a, W> {
    fn deref_mut(&mut self) -> &mut Self::Target { &mut self.wgpu_thing }
}

impl<'a, W: ProfilerCommandRecorder> Drop for OwningScope<'a, W> {
    fn drop(&mut self) { self.profiler.end_scope(&mut self.wgpu_thing); }
}

// ManualOwningScope
impl<'a, W: ProfilerCommandRecorder> std::ops::Deref for ManualOwningScope<'a, W> {
    type Target = W;

    fn deref(&self) -> &Self::Target { &self.wgpu_thing }
}

impl<'a, W: ProfilerCommandRecorder> std::ops::DerefMut for ManualOwningScope<'a, W> {
    fn deref_mut(&mut self) -> &mut Self::Target { &mut self.wgpu_thing }
}

[Wumpf]

Do I understand correctly that the problem is that the wgpu_profiler! macro expects to have ownership/ability to reference both profiler & wgpu-thing at the end of the scope which may not always be the case? Can you give man example of how it blows up? I'm still not very savy with these lifetime things.

I think I like the general approach much better than the current macro based solution since it's easier to reason about the lifetimes.

Can you clarify a few things for me? I'm not sure I follow yet in the details:

• Again, this about me being bad with lifetimes: scoped_render_pass are needed because they transfer the ownership of the encoder to the pass I suppose. Not sure I understand the magic that allows to transfer the ownership while still having the outer scope alive. Is this working because by explicitly differentiating lifetime 'b, rustc knows that the inner scope doesn't live as long and the encoder is available again afterwards?
• what's the usecase for ManualOwningScope? In that case one might as well do end/start manually
• Could having those two methods on GpuProfiler also solve the issue?

    /// Profiler scope that takes a mutable reference to the wgpu pass/encoder
    pub fn scope<Recorder, F>(&mut self, label: &str, encoder_or_pass: &mut Recorder, device: &wgpu::Device, func: F)
    where
        F: FnOnce(&mut Self, &mut Recorder, &wgpu::Device),
        Recorder: ProfilerCommandRecorder,
    {
        self.begin_scope(label, encoder_or_pass, device);
        func(self, encoder_or_pass, device);
        self.end_scope(encoder_or_pass);
    }

    /// Profiler scope that takes ownership of the wgpu pass/encoder
    pub fn scope_owning<Recorder, F>(&mut self, label: &str, mut encoder_or_pass: Recorder, device: &wgpu::Device, func: F)
    where
        F: FnOnce(&mut Self, Recorder, &wgpu::Device) -> Recorder,
        Recorder: ProfilerCommandRecorder,
    {
        self.begin_scope(label, &mut encoder_or_pass, device);
        let mut encoder_or_pass = func(self, encoder_or_pass, device);
        self.end_scope(&mut encoder_or_pass);
    }

.. and if so would that be better or worse than your suggestion? (I'm honestly not sure, but before we discuss that I'd like to know what that's missing because like I feel this isn't the full story)

In any case I'd like to have something added to the crate here that solves your usecase :). Thank you very much for contributing!

[Imberflur]

So for drawing each frame we generate this transient Drawer struct that owns the CommandEncoder and the SwapChainTexture
(see: https://gitlab.com/veloren/veloren/-/blob/imbris/wgpu-master-rebased/voxygen/src/render/renderer/drawer.rs)

pub struct Drawer<'frame> {
    encoder: Option<ManualOwningScope<'frame, wgpu::CommandEncoder>>,
    borrow: RendererBorrow<'frame>,
    swap_tex: wgpu::SwapChainTexture,
    globals: &'frame GlobalsBindGroup,
}

Then there are specific method on this that e.g. starts a render pass which takes a mutable borrow of the Drawer

    pub fn first_pass(&mut self) -> FirstPassDrawer {
        let encoder = self.encoder.as_mut().unwrap();
        let device = self.borrow.device;
        let mut render_pass =
            encoder.scoped_render_pass(device, "first_pass", &wgpu::RenderPassDescriptor {
                label: Some("first pass"),
                color_attachments: &[wgpu::RenderPassColorAttachmentDescriptor {
                    attachment: &self.borrow.views.tgt_color,
                    resolve_target: None,
                    ops: wgpu::Operations {
                        load: wgpu::LoadOp::Clear(wgpu::Color::TRANSPARENT),
                        store: true,
                    },
                }],
                depth_stencil_attachment: Some(wgpu::RenderPassDepthStencilAttachmentDescriptor {
                    attachment: &self.borrow.views.tgt_depth,
                    depth_ops: Some(wgpu::Operations {
                        load: wgpu::LoadOp::Clear(0.0),
                        store: true,
                    }),
                    stencil_ops: None,
                }),
            });

        render_pass.set_bind_group(0, &self.globals.bind_group, &[]);
        render_pass.set_bind_group(1, &self.borrow.shadow.bind.bind_group, &[]);

        FirstPassDrawer {
            render_pass,
            borrow: &self.borrow,
            globals: self.globals,
        }
    }

this then has methods on it that can be used to enter a certain pipeline state continuing the borrow chain

    pub fn draw_terrain<'data: 'pass>(
        &mut self,
        col_lights: &'data ColLights<terrain::Locals>,
    ) -> TerrainDrawer<'_, 'pass> {
        let mut render_pass = self.render_pass.scope(self.borrow.device, "terrain");

        render_pass.set_pipeline(&self.borrow.pipelines.terrain.pipeline);
        render_pass.set_bind_group(3, &col_lights.bind_group, &[]);

        TerrainDrawer { render_pass }
    }

and this finally has a method for actually drawing something

pub struct TerrainDrawer<'pass_ref, 'pass: 'pass_ref> {
    render_pass: Scope<'pass_ref, wgpu::RenderPass<'pass>>,
}

impl<'pass_ref, 'pass: 'pass_ref> TerrainDrawer<'pass_ref, 'pass> {
    pub fn draw<'data: 'pass>(
        &mut self,
        model: &'data Model<terrain::Vertex>,
        locals: &'data terrain::BoundLocals,
    ) {
        self.render_pass.set_bind_group(2, &locals.bind_group, &[]);
        self.render_pass.set_vertex_buffer(0, model.buf().slice(..));
        self.render_pass.draw(0..model.len() as u32, 0..1)
    }
}

finally Drawer has a Drop impl which submits things to the queue

impl<'frame> Drop for Drawer<'frame> {
    fn drop(&mut self) {
        let (mut encoder, profiler) = self.encoder.take().unwrap().end_scope();
        profiler.resolve_queries(&mut encoder);
        self.borrow.queue.submit(std::iter::once(encoder.finish()));
        profiler
            .end_frame()
            .expect("Gpu profiler error! Maybe there was an unclosed scope?");
    }
}

[Imberflur]

The code that uses this looks like:

        span!(_guard, "render", "<Session as PlayState>::render");
        let mut drawer = match renderer
            .start_recording_frame(self.scene.global_bind_group())
            .unwrap()
        {
            Some(d) => d,
            // Couldn't get swap chain texture this frame
            None => return,
        };

        // Render world
        {
            let client = self.client.borrow();

            let scene_data = SceneData {
                /* snip */
            };

            self.scene.render(
                &mut drawer,
                client.state(),
                client.entity(),
                client.get_tick(),
                &scene_data,
            );
        }

        // Clouds
        drawer.second_pass().draw_clouds();
        // PostProcess and UI
        let mut third_pass = drawer.third_pass();
        third_pass.draw_post_process();
        // Draw the UI to the screen
        self.hud.render(&mut third_pass.draw_ui());
    }

    /// Render the scene using the provided `Drawer`.
    pub fn render<'a>(
        &'a self,
        drawer: &mut Drawer<'a>,
        state: &State,
        player_entity: EcsEntity,
        tick: u64,
        scene_data: &SceneData,
    ) {
        span!(_guard, "render", "Scene::render");
        let sun_dir = scene_data.get_sun_dir();
        let is_daylight = sun_dir.z < 0.0;
        let focus_pos = self.camera.get_focus_pos();
        let cam_pos = self.camera.dependents().cam_pos + focus_pos.map(|e| e.trunc());

        let camera_data = (&self.camera, scene_data.figure_lod_render_distance);

        // would instead have this as an extension.
        if drawer.render_mode().shadow.is_map() && (is_daylight || !self.light_data.is_empty()) {
            if is_daylight {
                if let Some(mut shadow_pass) = drawer.shadow_pass() {
                    // Render terrain directed shadows.
                    self.terrain
                        .render_shadows(&mut shadow_pass.draw_terrain_shadows(), focus_pos);

                    // Render figure directed shadows.
                    self.figure_mgr.render_shadows(
                        &mut shadow_pass.draw_figure_shadows(),
                        state,
                        tick,
                        camera_data,
                    );
                }
            }

            // Render terrain point light shadows.
            drawer.draw_point_shadows(
                &self.data.point_light_matrices,
                self.terrain.chunks_for_point_shadows(focus_pos),
            )
        }

        let mut first_pass = drawer.first_pass();

        self.figure_mgr.render_player(
            &mut first_pass.draw_figures(),
            state,
            player_entity,
            tick,
            camera_data,
        );

        self.terrain.render(&mut first_pass, focus_pos);

        self.figure_mgr.render(
            &mut first_pass.draw_figures(),
            state,
            player_entity,
            tick,
            camera_data,
        );

        self.lod.render(&mut first_pass);

        // Render the skybox.
        first_pass.draw_skybox(&self.skybox.model);

        // Draws translucent terrain and sprites
        self.terrain.render_translucent(
            &mut first_pass,
            focus_pos,
            cam_pos,
            scene_data.sprite_render_distance,
        );

        // Render particle effects.
        self.particle_mgr
            .render(&mut first_pass.draw_particles(), scene_data);
    }

[Imberflur]

Again, this about me being bad with lifetimes: scoped_render_pass are needed because they transfer the ownership of the encoder to the pass I suppose. Not sure I understand the magic that allows to transfer the ownership while still having the outer scope alive. Is this working because by explicitly differentiating lifetime 'b, rustc knows that the inner scope doesn't live as long and the encoder is available again afterwards?

Yep, once the returned value goes out of scope/is dropped rust knows we can access the original value that was borrowed mutably here again. The explicit lifetime actually isn't needed for this though since the lifetime ellision rules will already do this except that the RenderPassDescriptor needs to have it's lifetime parameter linked to the render pass so we need to specify that which requires manual lifetimes.

what's the usecase for ManualOwningScope? In that case one might as well do end/start manually

It could be done manually, however it does add convenience with the helper methods (e.g. not having to pass in the profiler everywhere) and prevents forgetting to end the scope (since you need to get the CommandEncoder out of it to sumbit this won't be forgotten)

Could having those two methods on GpuProfiler also solve the issue?

I think this would have the same issue as the macro does with the setup I outlined above.

[Wumpf]

thanks for illustrating all this in such a great detail. Yeah I can see now that the scope method variant won't do it here (or at least it's not obvious how that would work out)

Would you mind PRing your initial suggestion? Just put it into a separate file (scope.rs?) and we take it from there. I can then fill the gaps (creating of compute passes) and add some doc (unless ofc you do all that first ;-)). Not entirely sure about exact namings, especially if it should be prefixed like WgpuProfilerScope, but that's probably just too redundant to the module namespace.
I think I'd go as far as to remove the original macro altogether

[Imberflur]

I would be hesitant to remove the macro as I suspect it probably fits some use cases nicely, but not mine so I don't have any proof of that 🙂.

edit: maybe the best way to test is to remove it and wait for requests to add it back

[Imberflur]

Would you mind PRing your initial suggestion?

Might be a few days FYI