[Usage Question] Trouble implementing a custom cursor

[Manghi]

Hello fellow Rustaceans,

I am trying to add custom cursor support to the ggez gaming library but I've been stuck on a problem for the past few days. I am able to run the example cursor program in rust-sdl2 just fine with the cursor updated as cursor.png flawlessly. When implementing basically the same code in ggez, the cursor remains unchanged. I am using the same image.

I have a function in the init step (before the event loop runs) which sets up the cursor. My point is this runs once I suppose.

Important Excerpts:

extern crate sdl2;
use sdl2::mouse::Cursor;
use sdl2::surface::Surface;

/// Sets the cursor as the supplied image
pub fn set_cursor_from_surface(image: &Path) {
    let surface = match Surface::from_file(image) {
        Ok(s) => s,
        Err(err) => panic!("Failed to load cursor image: {}", err)
    };
    let cursor = match Cursor::from_surface(surface, 0, 0) {
        Ok(c) => c,
        Err(err) => panic!("Failed to create cursor from image: {}", err)
    };

    cursor.set();
}

I've also tried setting the cursor through SystemCursor but I see no change.

I've tried stepping through with a debugger and examining the context variables surface and cursor between the "good" rust-sdl2 case, and the "bad" ggez case, but I do not notice any difference; I've examined the raw format segment of the returned surface and they match among the two cases. Furthermore the code runs without panic so I know its finding the image and creating a surface out of it. I also backported this to ggez 0.2.2 which was purely SDL2 (before they switched to gfx as a drawing backend) with no luck.

I'm perplexed and not sure where else to turn. Any help would be appreciated.

Thanks, Manghi

[Cobrand]

The problem is probably because your Surface doesn't live long enough. Lifetimes should normally take care of that, but there are none for Cursor, so I think this is a use-after-free...

If you put this exact same code in the main (outside the function, it will probably work).

There are obviously changes to be done on the rust-sdl2 side, and any PR is welcome. By the way, a comment in the source code made reference of this:

https://docs.rs/sdl2/0.31.0/src/sdl2/mouse/mod.rs.html#56

[Manghi]

Well you were definitely right about that. Once I put it in main the cursor was properly updated. Outside of that it doesn't update. Thank you for your insight.

So even though Surface was being passed as a reference in the cursor creation step, it still was dropped at the end of the function. Sounds like you're saying Surface needs an explicit lifetime bound to it to fix this issue. Wouldn't Cursor also Drop at the end of the function too? I would love to fix this myself but to be honest I'm not really sure where to begin.

[Cobrand]

The most obvious fix that I see here would be that Cursor should hold a reference to a Surface (and not a SurfaceRef). That would mean that the Cursor struct would hold a &'a Surface<'b> with 'a: 'b, and that we would have a Cursor<'a> overall.

Don't take that totally for granted though, there are design questions related to that: should a Cursor hold a reference to a Surface, or should the cursor "own" the Surface? Can we try to do both?

I think the Surface system is very messy as it is and I would love to re-think how everything should be implemented (I'm not sure we even need a SurfaceRef anymore, this was probably created in the early rust days...), but this is outside this issue's scope I think.

[Manghi]

Outside of your last statement, I personally would think that Cursor should own the surface. I don't see the benefit, or reason, of supporting both to be honest, unless I am in great need of enlightenment.

Just had a thought: If Cursor owned a &Surface, wouldn't the surface not live long enough and we would suffer from this very same issue? By this I'm reaffirming that the Cursor should own a Surface and not just a reference.