Render thread gets stuck in end_frame() when attempting to close window

[hannobraun]

Hi everyone,

I'm currently evaluating gfx-rs for my project, but I'm having a problem: When I'm attempting to close the window, the program won't quit.

I've managed to narrow the problem down. Here's the modified triangle example. This works as expected. I'm going to show the change that causes it to exhibit the problem below.

#![feature(phase)]

extern crate device;
#[phase(plugin)] extern crate gfx_macros;
extern crate getopts;
extern crate gfx;
extern crate glfw;
extern crate glfw_platform;
extern crate native;
extern crate render;
extern crate sync;

use glfw_platform::BuilderExtension;

#[vertex_format]
struct Vertex {
    pos: [f32, ..2],
    color: [f32, ..3],
}

static VERTEX_SRC: gfx::ShaderSource = shaders! {
GLSL_150: b"
    #version 150 core
    in vec2 pos;
    in vec3 color;
    out vec4 v_Color;
    void main() {
        v_Color = vec4(color, 1.0);
        gl_Position = vec4(pos, 0.0, 1.0);
    }
"
};

static FRAGMENT_SRC: gfx::ShaderSource = shaders! {
GLSL_150: b"
    #version 150 core
    in vec4 v_Color;
    out vec4 o_Color;
    void main() {
        o_Color = v_Color;
    }
"
};

struct GfxRsContext {
    glfw  : glfw::Glfw,
    window: glfw::Window,
    events: sync::comm::Receiver<(f64,glfw::WindowEvent)>,
}

impl GfxRsContext {
    fn init() -> (GfxRsContext, device::Device<render::resource::handle::Handle,device::gl::GlBackEnd,glfw_platform::Platform<glfw::RenderContext>>) {
        let glfw = glfw::init(glfw::FAIL_ON_ERRORS).unwrap();

        let (mut window, events) = glfw_platform::WindowBuilder::new(&glfw)
            .title("[GLFW] Triangle example #gfx-rs!")
            .try_modern_context_hints()
            .create()
            .expect("Failed to create GLFW window.");

        glfw.set_error_callback(glfw::FAIL_ON_ERRORS);
        window.set_key_polling(true); // so we can quit when Esc is pressed
        let (w, h) = window.get_size();

        let device = gfx::build()
            .with_glfw_window(&mut window)
            .with_queue_size(1)
            .spawn(proc(r) render_loop(r, w as u16, h as u16))
            .unwrap();

        let context = GfxRsContext {
            glfw  : glfw,
            window: window,
            events: events,
        };

        return (context, device);
    }

    fn input(&self) -> bool {
        self.glfw.poll_events();
        if self.window.should_close() {
            return false;
        }
        // quit when Esc is pressed.
        for (_, event) in glfw::flush_messages(&self.events) {
            match event {
                glfw::KeyEvent(glfw::KeyEscape, _, glfw::Press, _) => return false,
                _ => {},
            }
        }

        return true;
    }

    fn render(&self, device: &mut device::Device<render::resource::handle::Handle,device::gl::GlBackEnd,glfw_platform::Platform<glfw::RenderContext>>) {
        device.update();
    }
}

// We need to run on the main thread for GLFW, so ensure we are using the `native` runtime. This is
// technically not needed, since this is the default, but it's not guaranteed.
#[start]
fn start(argc: int, argv: *const *const u8) -> int {
     native::start(argc, argv, main)
}

fn main() {
    let (context, mut device) = GfxRsContext::init();

    let mut keep_running = true;
    while keep_running {
        keep_running = context.input();
        context.render(&mut device);
    }
}

fn render_loop(mut renderer: gfx::Renderer, width: u16, height: u16) {
    let frame = gfx::Frame::new(width, height);
    let state = gfx::DrawState::new();

    let vertex_data = vec![
        Vertex { pos: [ -0.5, -0.5 ], color: [1.0, 0.0, 0.0] },
        Vertex { pos: [ 0.5, -0.5 ], color: [0.0, 1.0, 0.0]  },
        Vertex { pos: [ 0.0, 0.5 ], color: [0.0, 0.0, 1.0]  }
    ];

    let mesh = renderer.create_mesh(vertex_data);
    let program = renderer.create_program(VERTEX_SRC.clone(), FRAGMENT_SRC.clone());

    let clear = gfx::ClearData {
        color: Some(gfx::Color([0.3, 0.3, 0.3, 1.0])),
        depth: None,
        stencil: None,
    };

    while !renderer.should_finish() {
        renderer.clear(clear, frame);
        renderer.draw(&mesh, mesh.get_slice(), &frame, &program, &state)
            .unwrap();
        renderer.end_frame();
        for err in renderer.errors() {
            println!("Renderer error: {}", err);
        }
    }
}

Please note that device is not part of my context struct. As I said above, this works as expected. If I click the close button or press escape, the window closes and the program terminates.

It won't work after I make this change:

diff --git a/src/main.rs b/src/main.rs
index 5779ffa..355d54b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -47,10 +47,11 @@ struct GfxRsContext {
    glfw  : glfw::Glfw,
    window: glfw::Window,
    events: sync::comm::Receiver<(f64,glfw::WindowEvent)>,
+   device: device::Device<render::resource::handle::Handle,device::gl::GlBackEnd,glfw_platform::Platform<glfw::RenderContext>>,
 }

 impl GfxRsContext {
-   fn init() -> (GfxRsContext, device::Device<render::resource::handle::Handle,device::gl::GlBackEnd,glfw_platform::Platform<glfw::RenderContext>>) {
+   fn init() -> GfxRsContext {
        let glfw = glfw::init(glfw::FAIL_ON_ERRORS).unwrap();

        let (mut window, events) = glfw_platform::WindowBuilder::new(&glfw)
@@ -73,9 +74,10 @@ impl GfxRsContext {
            glfw  : glfw,
            window: window,
            events: events,
+           device: device,
        };

-       return (context, device);
+       return context;
    }

    fn input(&self) -> bool {
@@ -94,8 +96,8 @@ impl GfxRsContext {
        return true;
    }

-   fn render(&self, device: &mut device::Device<render::resource::handle::Handle,device::gl::GlBackEnd,glfw_platform::Platform<glfw::RenderContext>>) {
-       device.update();
+   fn render(&mut self) {
+       self.device.update();
    }
 }

@@ -107,12 +109,12 @@ fn start(argc: int, argv: *const *const u8) -> int {
 }

 fn main() {
-   let (context, mut device) = GfxRsContext::init();
+   let mut context = GfxRsContext::init();

    let mut keep_running = true;
    while keep_running {
        keep_running = context.input();
-       context.render(&mut device);
+       context.render();
    }
 }

This simply adds device to the struct instead of keeping it in a separate variable. With this change, the program will no longer react if I press escape or click the close button.

When I click the close button or press escape, the following things do happen:

• As expected, the main thread leaves the while-loop in main.
• The render thread seems to get stuck in the renderer.end_frame(); call.

I have uploaded a repository that contains the example above: https://github.com/hannobraun/gfx-rs-evaluation The last commit is broken, the second-to-last is the version that works.

[kvark]

That's a nice bug report. Thanks @hannobraun!

[hannobraun]

You're welcome, @kvark, and thank you for your work on gfx-rs!

[kvark]

FYI, I'll not be able to look into your issue any time soon, since I'm deep in the metamorphosis, which BTW will render your issue irrelevant ;). Hope someone else can take care of it (@cmr? @bjz?).

[hannobraun]

Do you have a rough guess when the metamorphosis changes will land? I'm about to do some significant work on my project's render code, so if I'm going replace my shitty OpenGL code with gfx-rs, now's the time :)

If your timeline is a few weeks or more, I might try to fix this issue myself. If you expect to be done sooner, I might just do some other work first before tackling my render code.

[kvark]

@hannobraun Very roughly, metamorphosis is going to land this weekend. Plus-minus a week ;) Note, however, that the draw call syntax will not change, so if you port your app now (what is it, btw?), you'll only need minor fixes to the way device/renderer pair is created and manged, leaving the rendering itself untouched.

[kvark]

The device declaration is really broken. We need a better type alias to hide the render::Token (thus, leaving no need to have extern crate render for you). On the other hand, the metamorphosis will make that irrelevant. You are welcome to our gitter to discuss the experience you got so far.

[hannobraun]

Very roughly, metamorphosis is going to land this weekend. Plus-minus a week ;)

Great, sounds like it's worth the wait then :)

Note, however, that the draw call syntax will not change, so if you port your app now [...], you'll only need minor fixes to the way device/renderer pair is created and manged, leaving the rendering itself untouched.

Porting now is not really an option, this issue is a showstopper. I have to work around it or debug and fix it, both of which doesn't seem so appealing, if metamorphosis isn't that far out anyway.

(what is it, btw?)

A multiplayer online game that's way too ambitious :) Website: http://vndf.de/

The device declaration is really broken. We need a better type alias to hid the render::Token (thus, leaving no need to have extern crate render for you). On the other hand, the metamorphosis will make that irrelevant.

Those have been my main complaints so far, but they are minor complaints.

You are welcome to our gitter to discuss the experience you got so far.

I'm on my way out right now, but I might show up later this week or next week. I haven't done a lot anyway so far. Set up the window, hit this bug, and that's where we are right now :) Once the metamorphosis lands, I'll attempt to port my render code. I guess I'll have more to say then.

By the way, I hang out in #rust-gamedev sometimes, usually around the afternoon (CEST).

[emberian]

should_finish is never being set when the Device is dropped. Investigating why...

[emberian]

Devices destructor is never getting called O_o

[emberian]

Ok, this is a Rust bug.

@hannobraun: you can work around this by implementing Drop for GfxRsContext.

[hannobraun]

@cmr: Thanks, got it. Will try tomorrow. Out of curiosity, do you have a link to the issue?

[emberian]

No, still trying to make a minimal testcase.

On Wed, Aug 13, 2014 at 6:04 PM, Hanno Braun notifications@github.com wrote:

@cmr https://github.com/cmr: Thanks, got it. Will try tomorrow. Out of curiosity, do you have a link to the issue?

— Reply to this email directly or view it on GitHub https://github.com/gfx-rs/gfx-rs/issues/204#issuecomment-52118243.

http://octayn.net/

[emberian]

@kballard discovered that, at least here, implementing Drop causes the fields of the struct to be droppped bottom-to-top instead of top-to-bottom. My guess is that there is badness with dropping the Glfw/Window and then trying to do a swap_buffers.

[kvark]

@cmr I'm glad you took care of this one, I'd be totally useless here ;)

[hannobraun]

you can work around this by implementing Drop for GfxRsContext.

I can confirm that this works, both for the reduced example (https://github.com/hannobraun/gfx-rs-evaluation/commit/50aee7375d772d8b642f23ac9bfaf92dd41948fb) and my full app.

Thanks, @cmr, my evaluation of gfx-rs will resume shortly :)

[lilyball]

It won't work for long. rustc is getting fixed so implementing Drop doesn't affect the order of field destruction. Instead you should simply reverse the order of field declaration on the type in question.

[kvark]

Closing as irrelevant as of #209