Copy/Move an field that doesn't implement Copy

Question

Capitain Ferris has a map (seven_seas.png) of an area where he has hidden multiple treasures (at the coordinates (5,7) and at (7,9)). He wants to create a separate treasuremap for each of the treasures. The original map should not be altered.

He decides to use rust and the image crate for this.

extern crate image;
use crate::image::GenericImage;


struct Desk {
    map_of_seven_seas: image::DynamicImage,
    colored_ink: image::Rgba<u8>,
}

impl Desk {
    fn create_treasure_map(mut self, x: u32, y:u32) -> image::DynamicImage {
        self.map_of_seven_seas.put_pixel(x, y, self.colored_ink);
        self.map_of_seven_seas
    }

}

fn main() {
    let desk = Desk {
        map_of_seven_seas: image::open("seven_seas.png").unwrap(),
        colored_ink: image::Rgba([255, 0, 0, 255]), // red             
    };

    println!("Marking my treasures!");
    // works fine
    let treasure_map_0 = desk.create_treasure_map(5, 7);
    treasure_map_0.save("treasure_map_0.png").unwrap();

    // breaks
    let treasure_map_1 = desk.create_treasure_map(7, 9);
    treasure_map_1.save("treasure_map_1.png").unwrap();
}

Dark clouds obscure the sky as Captain Ferris realizes there is a flaw in his plan:

error[E0382]: use of moved value: `desk`
  --> src/main.rs:30:26
   |
19 |     let desk = Desk {
   |         ---- move occurs because `desk` has type `Desk`, which does not implement the `Copy` trait
...
26 |     let treasure_map_0 = desk.create_treasure_map(5, 7);
   |                          ---- value moved here
...
30 |     let treasure_map_1 = desk.create_treasure_map(7, 9);
   |                          ^^^^ value used here after move

error: aborting due to previous error

Capitain Ferris is in trouble now. It seems like he can only use his desks once. He evaluates what Mate Cargo hints at and tries to derive the Copy trait for the Desk (adding #[derive(Clone, Copy)] to the struct). But once again he fails:

error[E0204]: the trait `Copy` may not be implemented for this type
 --> src/main.rs:4:17
  |
4 | #[derive(Clone, Copy)]
  |                 ^^^^
5 | struct Desk {
6 |     map_of_seven_seas: image::DynamicImage,
  |     -------------------------------------- this field does not implement `Copy`
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

image::DynamicImage can't be copied and he can't derive Copy for it because it is part of a crate.

Does Captain Ferris have to create a new Desk every time he wants to create a treasure map (including loading the map_of_the_seven_seas from the file system)? Or is there a better way to reuse the Desk?

This is a learning excercise for me, so a nice solution with explanation would be better than a clever hack.

Answer 1

Does Captain Ferris have to create a new Desk every time he wants to create a treasure map (including loading the map_of_the_seven_seas from the file system)? Or is there a better way to reuse the Desk?

There is a better way indeed. The issue here is

fn create_treasure_map(mut self, x: u32, y:u32) -> image::DynamicImage

the self parameter (whether mut or not) means calling this method consumes the desk. While deriving Clone would be possible (Copy definitely is not), it would indeed copy the entire desk every time which does seem rather unnecessary.

This is avoidable though, by having the method borrow self instead:

fn create_treasure_map(&mut self, x: u32, y:u32) -> &image::DynamicImage {
    self.map_of_seven_seas.put_pixel(x, y, self.colored_ink);
    &self.map_of_seven_seas
}

However note that this updates the map_of_seven_seas in-place, meaning treasure_map_1 will also contain the treasure noted in treasure_map_0.

I would assume this is undesirable as it would be a tad too easy for the holder of the second map, and would rather defeat the usual point of treasure maps.

The fix, then, is to not update the desk: only borrow it immutably (we don't need to update it or its "reference" map of the seven seas) and copy just the map before adding the teasure location:

fn create_treasure_map(&self, x: u32, y:u32) -> image::DynamicImage {
    let mut treasure_map = self.map_of_seven_seas.clone();
    treasure_map.put_pixel(x, y, self.colored_ink);
    treasure_map
}

Answer 2

There is a lot of solutions here:

• clone not copy (simple & easy to read, allow to have both map in the same time, straightforward, I would advice this unless you have bench problem) #[derive(Clone)]
• borrow instead of consuming but this require side effect, doesn't allow to have both map at the same time
• read again as your file (clone is way better), allow to have both map at the same time

A good solution would be to use a guard pattern:

use image::{GenericImage, GenericImageView};

struct Desk {
    map_of_seven_seas: image::DynamicImage,
    colored_ink: image::Rgba<u8>,
}

struct PixelTmpChange<'a> {
    image: &'a mut image::DynamicImage,
    pixel: image::Rgba<u8>,
    x: u32,
    y: u32,
}

impl PixelTmpChange<'_> {
    fn new(
        image: &mut image::DynamicImage,
        x: u32,
        y: u32,
        tmp_pixel: image::Rgba<u8>,
    ) -> PixelTmpChange {
        let pixel = image.get_pixel(x, y);
        image.put_pixel(x, y, tmp_pixel);

        PixelTmpChange { image, pixel, x, y }
    }

    fn get_image(&mut self) -> &mut image::DynamicImage {
        self.image
    }
}

impl Drop for PixelTmpChange<'_> {
    fn drop(&mut self) {
        self.image.put_pixel(self.x, self.y, self.pixel);
    }
}

impl Desk {
    fn create_treasure_map(&mut self, x: u32, y: u32, name: &str) -> image::error::ImageResult<()> {
        let mut guard = PixelTmpChange::new(&mut self.map_of_seven_seas, x, y, self.colored_ink);

        guard.get_image().save(name)
    }
}

fn main() {
    let mut desk = Desk {
        map_of_seven_seas: image::open("seven_seas.png").unwrap(),
        colored_ink: image::Rgba([255, 0, 0, 255]), // red
    };

    println!("Marking my treasures!");
    desk.create_treasure_map(5, 7, "treasure_map_0.png")
        .unwrap();
    desk.create_treasure_map(7, 9, "treasure_map_1.png")
        .unwrap();
}

The idea is to let the compiler do the job of put the pixel in its original state. This is reusable and clean. We must use a lifetime and borrow the image as mut reference, so it's a little bit difficult to write, and we must ask the guard to get access to the image. But overall, I think it's a ok solution if you don't want to clone.

If you want you could make create_treasure_map() return the guard instead, to allow more options but less private behavior.