From 6e8f0894fda3d1359e37029b0c4e352095adaaf6 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sat, 2 Jul 2016 16:24:40 -0700 Subject: [PATCH] Image struct can now dole out buckets in a thread-safe way. This avoids the whole-image locking that was being done before, which was causing a lot of thread contention. --- src/image.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++---- src/renderer.rs | 34 +++++++--------- 2 files changed, 109 insertions(+), 28 deletions(-) diff --git a/src/image.rs b/src/image.rs index 781e64e..40430c8 100644 --- a/src/image.rs +++ b/src/image.rs @@ -4,20 +4,29 @@ use std::io; use std::io::Write; use std::path::Path; use std::fs::File; +use std::marker::PhantomData; +use std::sync::Mutex; +use std::cell::{RefCell, UnsafeCell}; +use std::mem; +use std::cmp; use color::{XYZ, xyz_to_rec709e}; -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct Image { - data: Vec, + data: UnsafeCell>, res: (usize, usize), + checked_out_blocks: Mutex>>, // (min, max) } +unsafe impl Sync for Image {} + impl Image { pub fn new(width: usize, height: usize) -> Image { Image { - data: vec![XYZ::new(0.0, 0.0, 0.0); width * height], + data: UnsafeCell::new(vec![XYZ::new(0.0, 0.0, 0.0); width * height]), res: (width, height), + checked_out_blocks: Mutex::new(RefCell::new(Vec::new())), } } @@ -29,21 +38,54 @@ impl Image { self.res.1 } - pub fn get(&self, x: usize, y: usize) -> XYZ { + pub fn get(&mut self, x: usize, y: usize) -> XYZ { assert!(x < self.res.0); assert!(y < self.res.1); - self.data[self.res.0 * y + x] + let data: &Vec = unsafe { mem::transmute(self.data.get()) }; + data[self.res.0 * y + x] } pub fn set(&mut self, x: usize, y: usize, value: XYZ) { assert!(x < self.res.0); assert!(y < self.res.1); - self.data[self.res.0 * y + x] = value; + let data: &mut Vec = unsafe { mem::transmute(self.data.get()) }; + data[self.res.0 * y + x] = value; } - pub fn write_ascii_ppm(&self, path: &Path) -> io::Result<()> { + pub fn get_bucket<'a>(&'a self, min: (u32, u32), max: (u32, u32)) -> Bucket<'a> { + let tmp = self.checked_out_blocks.lock().unwrap(); + let mut bucket_list = tmp.borrow_mut(); + + // Make sure this won't overlap with any already checked out buckets + for bucket in bucket_list.iter() { + // Calculate the intersection between the buckets + let inter_min = (cmp::max(min.0, (bucket.0).0), cmp::max(min.1, (bucket.0).1)); + let inter_max = (cmp::min(max.0, (bucket.1).0), cmp::min(max.1, (bucket.1).1)); + + // If it's not degenerate and not zero-sized, there's overlap, so + // panic. + if inter_min.0 < inter_max.0 && inter_min.1 < inter_max.1 { + panic!("Attempted to check out a bucket with pixels that are already checked out."); + } + } + + // Clip bucket to image + let max = (cmp::min(max.0, self.res.0 as u32), cmp::min(max.1, self.res.1 as u32)); + + // Push bucket onto list + bucket_list.push((min, max)); + + Bucket { + min: min, + max: max, + img: unsafe { mem::transmute(self as *const Image) }, + _phantom: PhantomData, + } + } + + pub fn write_ascii_ppm(&mut self, path: &Path) -> io::Result<()> { // Open file. let mut f = io::BufWriter::new(try!(File::create(path))); @@ -63,7 +105,7 @@ impl Image { Ok(()) } - pub fn write_binary_ppm(&self, path: &Path) -> io::Result<()> { + pub fn write_binary_ppm(&mut self, path: &Path) -> io::Result<()> { // Open file. let mut f = io::BufWriter::new(try!(File::create(path))); @@ -84,6 +126,51 @@ impl Image { } } +#[derive(Debug)] +pub struct Bucket<'a> { + min: (u32, u32), + max: (u32, u32), + img: *mut Image, + _phantom: PhantomData<&'a Image>, +} + +impl<'a> Bucket<'a> { + pub fn get(&mut self, x: u32, y: u32) -> XYZ { + assert!(x >= self.min.0 && x < self.max.0); + assert!(y >= self.min.1 && y < self.max.1); + + let img: &mut Image = unsafe { mem::transmute(self.img) }; + let data: &Vec = unsafe { mem::transmute(img.data.get()) }; + + data[img.res.0 * y as usize + x as usize] + } + + pub fn set(&mut self, x: u32, y: u32, value: XYZ) { + assert!(x >= self.min.0 && x < self.max.0); + assert!(y >= self.min.1 && y < self.max.1); + + let img: &mut Image = unsafe { mem::transmute(self.img) }; + let data: &mut Vec = unsafe { mem::transmute(img.data.get()) }; + + data[img.res.0 * y as usize + x as usize] = value; + } +} + +impl<'a> Drop for Bucket<'a> { + fn drop(&mut self) { + let img: &mut Image = unsafe { mem::transmute(self.img) }; + let tmp = img.checked_out_blocks.lock().unwrap(); + let mut bucket_list = tmp.borrow_mut(); + + // Find matching bucket and remove it + let i = bucket_list.iter().position(|bucket| { + (bucket.0).0 == self.min.0 && (bucket.0).1 == self.min.1 && + (bucket.1).0 == self.max.0 && (bucket.1).1 == self.max.1 + }); + bucket_list.swap_remove(i.unwrap()); + } +} + fn srgb_gamma(n: f32) -> f32 { if n < 0.0031308 { n * 12.92 diff --git a/src/renderer.rs b/src/renderer.rs index 8fbac55..38d282c 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -3,8 +3,7 @@ use std::path::Path; use std::cmp::min; use std::iter::Iterator; -use std::sync::{Mutex, RwLock}; -use std::cell::RefCell; +use std::sync::RwLock; use scoped_threadpool::Pool; use crossbeam::sync::MsQueue; @@ -32,13 +31,8 @@ impl Renderer { pub fn render(&self, thread_count: u32) { let mut tpool = Pool::new(thread_count); - let image = Mutex::new(RefCell::new(Image::new(self.resolution.0, self.resolution.1))); - let (img_width, img_height) = { - let i = image.lock().unwrap(); - let w = i.borrow().width(); - let h = i.borrow().height(); - (w, h) - }; + let mut image = Image::new(self.resolution.0, self.resolution.1); + let (img_width, img_height) = (image.width(), image.height()); let all_jobs_queued = RwLock::new(false); @@ -55,6 +49,7 @@ impl Renderer { // Set up job queue let job_queue = MsQueue::new(); + // Render tpool.scoped(|scope| { // Spawn worker tasks @@ -134,13 +129,15 @@ impl Renderer { } // Calculate color based on ray hits - let img = img.lock().unwrap(); - let mut img = img.borrow_mut(); - for path in paths.iter() { - let mut col = - img.get(path.pixel_co.0 as usize, path.pixel_co.1 as usize); - col += XYZ::from_spectral_sample(&path.color) / self.spp as f32; - img.set(path.pixel_co.0 as usize, path.pixel_co.1 as usize, col); + { + let min = (bucket.x, bucket.y); + let max = (bucket.x + bucket.w, bucket.y + bucket.h); + let mut img_bucket = img.get_bucket(min, max); + for path in paths.iter() { + let mut col = img_bucket.get(path.pixel_co.0, path.pixel_co.1); + col += XYZ::from_spectral_sample(&path.color) / self.spp as f32; + img_bucket.set(path.pixel_co.0, path.pixel_co.1, col); + } } } }); @@ -172,10 +169,7 @@ impl Renderer { // Write rendered image to disk - { - let img = &image.lock().unwrap(); - let _ = img.borrow().write_binary_ppm(Path::new(&self.output_file)); - } + let _ = image.write_binary_ppm(Path::new(&self.output_file)); } }