Unexpected pixels changing in opengl texture before being drawn to screen

I am writing a framework to be able to draw pixels to the screen. However now that I am trying to update the screen the first 4 pixels are showing random colors.

I did not have this problem when I was just sending a pointer to the image data to my window module however now that i am passing a drawing function as a closure this started happening. I do not think this is a problem within my drawing function as I wrote to those pixels and they were still modified.

My main drawing function:

// Define drawing function
fn rand_pixel(image_width: usize, image_height: usize) -> *const std::ffi::c_void
{
    // Define colors
    let black_pixel = image::Color { red: 0, green: 0, blue: 0 };
    let white_pixel = image::Color { red: 255, green: 255, blue: 255 };

    // Create blank image
    let mut pixels: image::Image = image::Image::new(image_width, image_height, black_pixel);

    // Get two random numbers
    use rand::Rng;
    let mut rng = rand::thread_rng();
    let rand_x = rng.gen::<usize>() % image_width;
    let rand_y = rng.gen::<usize>() % image_height;

    // Change pixels
    pixels.set_pixel(rand_x, rand_y, white_pixel);

    // return pointer
    pixels.get_ptr()
}

My window drawing routine:

// Execute draw function
let image_ptr = draw_function();

unsafe 
{
    // Create texture
    let mut tex_id: gl::types::GLuint = 0;
    gl::GenTextures(1, &mut tex_id);
    gl::BindTexture(gl::TEXTURE_2D, tex_id);

    // Move pixels to texture
    gl::TexImage2D(gl::TEXTURE_2D, 0, gl::RGBA as i32, 
        image_width, image_height, 0, gl::RGBA, gl::UNSIGNED_BYTE, image_ptr);

    gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_MIN_FILTER, gl::NEAREST as i32);
    gl::TexParameteri(gl::TEXTURE_2D, gl::TEXTURE_MAG_FILTER, gl::NEAREST as i32);

    // Create read framebuffer
    let mut read_fbo_id: gl::types::GLuint = 0;
    gl::GenFramebuffers(1, &mut read_fbo_id);
    gl::BindFramebuffer(gl::READ_FRAMEBUFFER, read_fbo_id);

    // Bind texture to read framebuffer
    gl::FramebufferTexture2D(gl::READ_FRAMEBUFFER, gl::COLOR_ATTACHMENT0, gl::TEXTURE_2D, tex_id, 0);

    // Transfer from read framebuffer to draw framebuffer
    gl::BlitFramebuffer(0, 0, image_width, image_height, 
        0, 0, window_width as i32, window_height as i32, 
        gl::COLOR_BUFFER_BIT, gl::NEAREST);

    // Clear out read framebuffer
    gl::BindFramebuffer(gl::READ_FRAMEBUFFER, 0);
    gl::DeleteFramebuffers(1, &mut read_fbo_id);
};

// Update screen
let _ = windowed_context.swap_buffers();
std::thread::sleep(std::time::Duration::from_millis(15));

The full project can be found: https://github.com/Pandabear314/sam_graphics

I am expecting a black screen with a single white pixel drawing randomly on the screen every frame. However the bottom left corner includes 4 random pixels. The first and third pixel change while the second and fourth do not change.

An image showing the result: http://prntscr.com/nwl2gk

Solution

The function rand_pixel creates an image::Image in a local variable, and then returns a pointer to it. The problem is that the image is dropped at the end of the function, so this pointer is left dangling.

Dereferencing this pointer is Undefined Behaviour, which is apparently manifesting itself in your application as some pixels being drawn the wrong colour. After the Image is dropped, the memory it was using to store pixels can be re-used for anything else. It seems that most of it has not yet been re-used but some data has been written to the first few bytes.

In safe code, Rust protects you from this kind of error by checking that the lifetimes of references do not exceed the scope of the referenced variables. But, when you dip into unsafe code and use raw pointers, you have to ensure memory safety yourself.

A possible fix is to make the drawing function take a mutable reference to an Image instead of owning it itself:

fn rand_pixel(pixels: &mut image::Image) -> *const std::ffi::c_void {
    // Define colors
    let black_pixel = image::Color { red: 0, green: 0, blue: 0 };
    let white_pixel = image::Color { red: 255, green: 255, blue: 255 };

    // Overwrite with a blank image
    *pixels = image::Image::new(pixels.width, pixels.height, black_pixel);

    // Get two random numbers
    use rand::Rng;
    let mut rng = rand::thread_rng();
    let rand_x = rng.gen() % pixels.width;
    let rand_y = rng.gen() % pixels.height;

    // Change pixels
    pixels.set_pixel(rand_x, rand_y, white_pixel);

    // return pointer (still valid because the image lives longer than this function)
    pixels.get_ptr()
}

In your main function you own the image and pass it via a mutable reference:

// this variable lives longer than the call to window::create_window
let mut pixels = image::Image::new(image_width, image_height, image::Color { red: 0, green: 0, blue: 0} );

// Define draw function
let draw_fun = || rand_pixel(&mut pixels);

// Create new window
window::create_window(1600.0, 900.0, image_width as i32, image_height as i32, draw_fun);

Note, you'll also have to change the signature of create_window to accept a FnMut instead of a Fn.