Gfx: assertion failed: self.dimensions.map(|d| d == dim).unwrap_or(true) when rendering to off-screen targets

[norru]

I have tracked this assertion down to a possible bug we discussed on the https://gitter.im/gfx-rs/gfx, I only need this as a place to track changes.

`thread 'main' panicked at 'assertion failed: self.dimensions.map(|d| d == dim).unwrap_or(true)', /u01/home/nico/Projects/3rdParty/gfx/src/core/src/pso.rs:297:8

• set_dimensions is initially None
• then in add_color it gets set to Some(1024, 1024, 0, 1)
• then in add_depth_stencil is set to Some(1024, 1024, 1, 1)

It appears that the (width, height, depth) of the on-screen framebuffer is (w, h, 1) while it's set to (w, h, 0...) for off-screen targets. As the depth buffer is created with the initial window, it is consistent with the former but mismatched when used with another target,

This inconsistency appears to be introduced in texture.rs:

impl Kind {
    /// Get texture dimensions, with 0 values where not applicable.
    pub fn get_dimensions(&self) -> Dimensions {
        let s0 = AaMode::Single;
        match *self {
            Kind::D1(w) => (w, 0, 0, s0),
            Kind::D1Array(w, a) => (w, 0, a as Size, s0),
            Kind::D2(w, h, s) => (w, h, 0, s),
            Kind::D2Array(w, h, a, s) => (w, h, a as Size, s),
            Kind::D3(w, h, d) => (w, h, d, s0),
            Kind::Cube(w) => (w, w, 6, s0),
            Kind::CubeArray(w, a) => (w, w, 6 * (a as Size), s0)
        }
    }
   ...

Replacing Kind::D2(w, h, s) => (w, h, 0, s) with Kind::D2(w, h, s) => (w, h, 1, s) seems to fix the assert with no other ill effects. I'll put together a pull request once I've done with testing.

[kvark]

As the depth buffer is created with the initial window, it is consistent with the former but mismatched when used with another target,

Are you testing this on GL?

There is no need to be consistent between mian framebuffer's depth and user's off-screen target because these are never allowed to be used together anyway.

[norru]

Yes, using GL - looks like it's using the off-screen target against the screen's depth buffer and it's happily letting me do it. However, I don't even need depth testing for my application (at the moment though), I'd rather get rid of the depth+stencil altogether. Back then when I wrote it, I could not find a way to skip the depth buffer creation. I am rendering onto an off-screen MSAA and then resolving to the screen as the last step of the chain.

[kvark]

Hmm, then I don't see why we'd fix this issue. I mean, the error message could be better, but it was at least great to have an error there in the first place (and now we don't, instead relying on GL backend to fail gracefully).

Mixing main FBO's surfaces with user ones is not possible on GL backend, and we should complain loudly about it. This is unfortunate, it's where the abstraction level of pre-ll fails a little bit, but we need to live with it.

Therefore, I believe this issue should be closed as invalid.

[norru]

Fair enough. How would I go on and fix properly though? How can I render on an off-screen FBO without a Depth buffer at all? Or, alternatively, how do I create an off-screen depth buffer matching the on-screen one? Is anybody else doing it?

Also, I think others were having a similar problem, with a different use case (blitting from an off-screen surface to the on-screen FB).

[kvark]

In gfx-rs, we just need to add a message to this assert explaining the things.
In your app, just don't mix user surfaces with main framebuffer surfaces?

[norru]

Uhm, easier said than done. First time I tried last year I could not find a way to do it, so I just left it there. If I could solve this by just not making it happen, I'd be much happier as I could use the "stock" gfx rather than my hacked fork.

[kvark]

@norru it's not like I can just tell you "do X". Look at the examples, describe to us why you wanted to use the user off-screen target with the main framebuffer, then maybe we can be more specific.

[norru]

Back then when I originally did it I think it was the only way to implement MSAA + HDR. If there is a better way nowadays and there is an example that does a similar thing, I'd be super happy to have a look.

Edit: i think there is something of the sorts in render_target example. I'll try that.

[kvark]

it was the only way to implement MSAA + HDR

So you were mixing an MSAA-enabled user color target with non-msaa main FBO's depth? Sorry, this can't ever be the only way to do anything, it's plainly a wrong thing to do.

You need to have MSAA color and depth views, render to them, then bind the color for reading and render into the main FBO's color while doing tone-mapping and resolving.

[kvark]

(closing as agreed earlier)

[norru]

I was already doing that, pretty much ignoring the existing depth buffer. I'll try that with a new depth buffer instead. Cheers.

[norru]

Hello, creating a MSAA depth buffer matching the off-screen render target seems to fix the problem with the existing 0.16.1 gfx on cargo.io. I've pushed the changes to rust-oids.