Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multisampled textures + resolve?\ #198

Closed
EriKWDev opened this issue Oct 31, 2024 · 7 comments · Fixed by #213
Closed

Support for multisampled textures + resolve?\ #198

EriKWDev opened this issue Oct 31, 2024 · 7 comments · Fixed by #213

Comments

@EriKWDev
Copy link
Contributor

EriKWDev commented Oct 31, 2024

I see there are some indications in the code that sample_count was almost added to the texture descriptor but don't see it implemented yet.

What would be the challenges in supporting multisampled textures and resolve targets for render passes? I would be happy to help though I have mostly experience with wgpu:s user-facing code

(side note: Such a fresh breath of air to read this code! It's all so simple.. blade-graphics looks very promising and I would like to give blade a try for our rendering engine at my company that's currently using wgpu (for this game https://idno.se/swap/). Seeing your presentation and descirbing the simple binding model in blade really motivated me after having to deal with so much pipeline description and setup of bind groups for our multiple shader permutations... )

@kvark
Copy link
Owner

kvark commented Nov 7, 2024

I haven't investigated this again since WebGPU times, but I think the support for MSAA will largely look the same as in wgpu. The sample count needs to be used and respected by the texture creation. Then we'll need extra data in RenderPipelineDesc to specify the resolve target. This is the only way with zero translation overhead to both Vulkan and Metal.

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Nov 7, 2024

I made some experiments in my fork and also found that there was a ClearOp::ResolveTo(TextureView) in the code which I successfully used together with some more info in the texturedesc for sample count and some more msaa state in the renderpipelinedesc to get msaa working for the particles demo. I did have some issues with it though on my intel integrted graphics laptop as well as an intel+nvidia machine, though my Ryzen+AMD gpu seemed to work fine. None of the targets had validation errors.

I will investigate more and get back, though on vacation atm

@kvark
Copy link
Owner

kvark commented Nov 10, 2024

Yes, I think this is a fine way to expose it in the API. Technically, one could try to both store the result and resolve it in Vulkan, but that isn't very useful. If we provide this through ClearOp, it becomes very neatly out of the way for most usages.

@kvark
Copy link
Owner

kvark commented Nov 17, 2024

I'm finishing working on #203 and thinking about making a release. Would you want to prototype a change to squeeze into blade-graphics-0.6 about multisampling? @EriKWDev

@EriKWDev
Copy link
Contributor Author

#203 looks like a nice addition!

I am still on vacation and will be back in December so will not be able to contribute much work until then sadly.

So if a release is in order then go ahead!

(On a side-note I also started investigating https://docs.vulkan.org/features/latest/features/proposals/VK_KHR_dynamic_rendering_local_read.html on the airplane as well as it might become relevant for our company in the future too, though support in drivers seemed not great at the moment)

@EriKWDev
Copy link
Contributor Author

EriKWDev commented Dec 1, 2024

Getting back at this for my next flight home but I am struggling with the opengl es implementation of the finishop resolve.

I created a new Command BlitFramebuffers with two TextureViews. Upon finishing the rendering if the finishop is a ResolveTo, I push that command to the commands. Finally, in the implementation I want to bind DRAW_FRAMEBUFFER and READ_FRAMEBUFFER, and blit.

Here is my current attempt at the opengl ES. Perhaps this should be broken up into smaller commands? :

// ... new Command in enum Command
pub enum Command {
    // ...
    BlitFramebuffer {
        from: TextureView,
        to: TextureView,
    },
    // ..
}


// ... inside gles/command.rs's fn render

for (i, rt) in targets.colors.iter().enumerate() {
    if let crate::FinishOp::ResolveTo(to) = rt.finish_op {
        self.commands
            .push(super::Command::BlitFramebuffer { from: rt.view, to });
    }
}



// ... inside gles/command.rs's Command's unsafe fn execute's match 

Self::BlitFramebuffer { from, to } => {
    let target_from = match from.inner {
        super::TextureInner::Renderbuffer { raw } => raw,
        _ => panic!("Unsupported resolve between non-renderbuffers"),
    };
    let target_to = match to.inner {
        super::TextureInner::Renderbuffer { raw } => raw,
        _ => panic!("Unsupported resolve between non-renderbuffers"),
    };

    let framebuf_from = gl.create_framebuffer().unwrap();
    let framebuf_to = gl.create_framebuffer().unwrap();

    gl.bind_framebuffer(glow::READ_FRAMEBUFFER, Some(framebuf_from));
    gl.framebuffer_renderbuffer(
        glow::READ_FRAMEBUFFER,
        glow::COLOR_ATTACHMENT0, // NOTE: Assuming color attachment
        glow::RENDERBUFFER,
        Some(target_from),
    );

    gl.bind_framebuffer(glow::DRAW_FRAMEBUFFER, Some(framebuf_to));
    gl.framebuffer_renderbuffer(
        glow::DRAW_FRAMEBUFFER,
        glow::COLOR_ATTACHMENT0, // NOTE: Assuming color attachment
        glow::RENDERBUFFER,
        Some(target_to),
    );

    gl.blit_framebuffer(
        0,
        0,
        from.target_size[0] as _,
        from.target_size[1] as _,
        0,
        0,
        to.target_size[0] as _,
        to.target_size[1] as _,
        glow::COLOR_BUFFER_BIT, // NOTE: Assuming color
        glow::NEAREST,
    );

    gl.bind_framebuffer(glow::READ_FRAMEBUFFER, None);
    gl.bind_framebuffer(glow::DRAW_FRAMEBUFFER, None);

    gl.delete_framebuffer(framebuf_from);
    gl.delete_framebuffer(framebuf_to);
}

Vulkan and metal was easier, though I need to get to a computer at work in order to test that the metal implementation actually works

@kvark
Copy link
Owner

kvark commented Dec 3, 2024

This is pretty much what wgpu-hal is also doing - https://github.com/gfx-rs/wgpu/blob/02b28e28f0f0904a943de44b647bf18a66ac4fc4/wgpu-hal/src/gles/queue.rs#L1107-L1133
It does reuse both the FBOs though instead of recreating them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants