Getting artifacts while making the ray tracing parallel

I am getting interesting artifacts on the generated image while making it parallel with Rayon
parallel-pixel-rendering

I have the following stracture for the Tracer:

pub struct Tracer {
    image_buffer: RwLock<RgbImage>,
    image_width: u32,
    image_height: u32,
    samples_per_pixel: u32,
}

impl Tracer {
    pub fn new(width: u32,
            height: u32,
            samples: u32) -> Self {

        Tracer {
            image_buffer: RwLock::new(RgbImage::new(width, height)),
            image_width: width,
            image_height: height,
            samples_per_pixel: samples,
        }
    }

    pub fn trace(&self, cam: &Camera, world: &World, bar: &ProgressBar ,max_depth: u32) {
        
        // generate pixel coordinates from the image buffer
        let pixels = (0..self.image_width)
            .flat_map(|x| (0..self.image_height).map(move |y| (x, y)))
            .collect::<Vec<(u32, u32)>>();


        /////////////////////////////////////////////
        //Perform multi-sampling with parallelism
        pixels
            .par_iter()
            .progress_with(bar.clone())
            .for_each(|(x, y)| {
            
            let mut pixel_color = Color::new(0.0, 0.0, 0.0);

            for _ in 0..self.samples_per_pixel {
                let mut rng = rand::thread_rng();

                let random_u: f32 = rng.gen();
                let random_v: f32 = rng.gen();

                let u = ((*x as f32) + random_u) / ((self.image_width - 1) as f32);
                let v = 1.0 - (((*y as f32) + random_v) / ((self.image_height - 1) as f32));
                let ray = cam.get_ray(u, v); 

                pixel_color += self.ray_color(&ray, &world, max_depth);               

            }

            
            let image_buffer_lock_result = self.image_buffer.try_write();

            if !image_buffer_lock_result.is_err() {
                // conduct gamma correction over the pixel
                let mut buffer = image_buffer_lock_result.unwrap();
                buffer.put_pixel(*x, *y, Rgb(Util::gamma_correction(&pixel_color, self.samples_per_pixel)));
            }            
        });

    }

If possible, can you provide a full example (including Cargo.toml) that we can build?

Also, to clarify: are you saying that when you don't parallelize, you don't see the artifacts?

check out from GitHub - sajis997/path_tracer: Path Tracer in Rust , branch - implement-bounding-volume-hierarchy

hope you will be able to recreate the issue.

Your GitHub program runs correctly for me. It generates the "parallel-rendering.png" output image, which has no artifacts.

I would suggest running your program without parallelism, to see if that makes a difference. Otherwise, I guess you could log every pixel and the value that was calculated for it.

If RwLock::try_write returns an error here, your program never writes that pixel:

Try replacing try_write with write.

7 Likes

This won't solve your problem but you can write this shorter like this:

            if let Ok(mut buffer) = image_buffer_lock_result {
                // conduct gamma correction over the pixel
                buffer.put_pixel(*x, *y, Rgb(Util::gamma_correction(&pixel_color, self.samples_per_pixel)));
            }

As a note on that. Yes that's probably the reason for the black pixels. But on the other hand doing this synchronization is probably not a good idea in the first place. It's probably better to use map and collect instead of for_each and then write the buffer from a single thread. This way you don't let the threads compete with each other.

Not sure if that's true though. Probably depends a bit on how many threads you have and how long the ray tracing takes compared to do the writes into the buffer.

Or maybe you can par_iter_mut directly over the buffer.

Does the trace() function definition is similar to what I have posted before ? On which platform did you compile and executed the source ?

replacing try_write() with write() solved the issue.

Instead of iterating over just (x,y) coordinates in a shared buffer, you should use an iterator that gives you &mut Rgb for every pixel, and iterate that in parallel. This way every reference will be guaranteed to be separate, and you will not need any locking of the image buffer.

1 Like

How to declare the image_buffer then inside the struct? As of now image_buffer field is mutable while rest of the other fields are immutable

You don't need to change the struct definition, but instead take the lock once outside the iterator. You can then use RgbImage::par_enumerate_pixels_mut to make your iterator instead of manually indexing into the image.

Something like this (untested):

    pub fn trace(&self, cam: &Camera, world: &World, bar: &ProgressBar ,max_depth: u32) {
        let locked_buf = self.image_buffer.write().unwrap();
        locked_buf
            .par_enumerate_pixels_mut()
            .progress_with(bar.clone())
            .for_each(|(x, y, px_out)| {
                let mut pixel_color = Color::new(0.0, 0.0, 0.0);

                // calculate pixel color for (x, y) here ...

                *px_out = Rgb(Util::gamma_correction(&pixel_color, self.samples_per_pixel));
            }            
        });
    }
2 Likes
.par_enumerate_pixels_mut()

is not available for RgbImage

According to the docs, you'll need to enable the rayon feature for the image crate for it to be available. You do that by editing Cargo.toml.

1 Like

I assume you meant the following

image = {version = "0.24.7", features = [rayon]}

But with the above update on the Cargo.toml file , I am getting error with rust - analyzer mentioning that the feature is not available

Looks like this feature is available only from 0.24.8.

1 Like