andros21 / rustracer Goto Github PK

View Code? Open in Web Editor NEW

3.0 2.0 1.0 28.22 MB

rustracer - a multi-threaded raytracer in pure rust

Home Page: https://andros21.github.io/rustracer/docs

License: GNU General Public License v3.0

Rust 97.67% Makefile 0.80% Shell 1.54%

rust raytracing cargo coverage cosign rayon clap cue yaml slsa

rustracer's People

Contributors

Stargazers

Watchers

Forkers

paolo-azzini

rustracer's Issues

wrong default values for `--init-state` and `--init-seq` cli parameters

rustracer/src/cli.rs

Lines 40 to 47 in 9b028ac

 /// Default init seed for random generator. 

 /// 

 /// When no arguments are provided to `--init-state` flag 

 const INIT_STATE: &str = "45"; 

 /// Default identifier for random generator sequence. 

 /// 

 /// When no arguments are provided to `--init-seq` flag 

 const INIT_SEQ: &str = "45";

deafult values should be 42 and 52, see

rustracer/src/random.rs

Lines 12 to 20 in 9b028ac

 impl Default for Pcg { 

 /// Provides a default constructor for [`Pcg`](struct@Pcg), 

 /// the default values for the seed are: 

 /// - `init_state==42`; 

 /// - `init_seq==54`. 

 fn default() -> Self { 

 Self::new(42, 54) 

 } 

 }

propose to change default value for `--factor`

images rendered always seems too dark without manually adjust --factor 1.0
how about change its default value in src/cli?

avoid repetition of `match` inside a nested for loop

rustracer/src/hdrimage.rs

Lines 83 to 95 in a851061

 for y in (0..height).rev() { 

 for x in 0..width { 

 match endianness { 

 ByteOrder::LittleEndian => buf_reader 

 .read_f32_into::<byteorder::LittleEndian>(&mut buffer) 

 .map_err(HdrImageErr::PfmFileReadFailure)?, 

 ByteOrder::BigEndian => buf_reader 

 .read_f32_into::<byteorder::BigEndian>(&mut buffer) 

 .map_err(HdrImageErr::PfmFileReadFailure)?, 

 } 

 hdr_img.set_pixel(x, y, (buffer[0], buffer[1], buffer[2]).into())?; 

 } 

 }

In this double nested for cycle, it's maybe better to reverse the logic ... not repeat match width*height times, but match before loop and create a nested for each branch of the match. Something like this:

match endianess {
condition1 => {  double for loop }
condition2 => { double for loop }
}

images flipped upside-down

inside demo feature branch is now visible a problem when a scene is rendered ...
shapes placed along y-axis are flipped upside-down, 👀 at that:

for how scene is built in demo subcommand insidemain.rs

rustracer/src/main.rs

Lines 104 to 119 in cb35aef

 let mut world = World::default(); 

 for x in [-0.5, 0.5].into_iter() { 

 for y in [-0.5, 0.5].into_iter() { 

 for z in [-0.5, 0.5].into_iter() { 

 world.add(Box::new(Sphere::new( 

 translation(Vector::from((x, y, z))) * scaling(Vector::from((0.1, 0.1, 0.1))), 

 ))); 

 } 

 } 

 } 

 world.add(Box::new(Sphere::new( 

 translation(Vector::from((0.0, 0.0, -0.5))) * scaling(Vector::from((0.1, 0.1, 0.1))), 

 ))); 

 world.add(Box::new(Sphere::new( 

 translation(Vector::from((0.0, 0.5, 0.0))) * scaling(Vector::from((0.1, 0.1, 0.1))), 

 )));

the big circle in the upside middle part of the image should stay at the bottom!

Probably there is problem inside already merged imagetracer module

PR is coming

wrong transformation composition in scene parser

description of the problem

command

$ rustracer render -f 0.5 examples/demo.yml demo.png

result

command

$ rustracer render --angle-deg 90 -f 0.5 examples/demo.yml demo.png

result

This is wrong! It's an unexpected behaviour, because z-axis rotation is applied before the translation (see examples/demo.yml)

rustracer/examples/demo.yml

Lines 103 to 105 in 71a9531

 - name: camera_tr 

 compose: 

 - translation: [-2, 0, 0.5]

instead the logic should be reversed, before translation and then rotation added via cli, the problem is transformation product order:

rustracer/src/scene.rs

Lines 1190 to 1195 in 71a9531

 let transformation = var.transformations.get(&transformation_id).copied().ok_or( 

 SceneErr::UndefinedIdentifier { 

 loc, 

 msg: format!("{:?} transformation not defined", transformation_id), 

 }, 

 )? * rotation_z(f32::to_radians(cli.angle_deg));

and it's not the only occurrences of that type cause also when composing multiple transformation, the product must be reversed:

rustracer/src/scene.rs

Line 968 in 71a9531

 transformation = transformation * self.parse_transformation(transformations, var)?; 

rustracer/src/scene.rs

Lines 992 to 993 in 71a9531

 transformation = 

 transformation * self.parse_transformation(transformations, var)?;

so that the expected output of precedent command will be:

expected

arguments exchanged in `PerspectiveCamera::new`

in the method new of PerspectiveCamera the argument aspect_ratio and distance
are exchanged respect how are used inside test

rustracer/src/camera.rs

Lines 103 to 114 in 66115ce

 pub fn new( 

 aspect_ratio: f32, 

 distance: f32, 

 transformation: Transformation, 

 ) -> PerspectiveCamera { 

 PerspectiveCamera { 

 aspect_ratio, 

 distance, 

 transformation, 

 } 

 } 

 }

this kind of errors are not caught in unit tests cause objects are created directly (using the power of super::*) ...
this teach us to always use new method also inside unit test or at least alternate with the direct creation
(unrelated this workflow probably will improve also unit test coverage)

PR is coming

todo list

todo 📘

other improvements're welcome!

	/// Default init seed for random generator.
	///
	/// When no arguments are provided to `--init-state` flag
	const INIT_STATE: &str = "45";
	/// Default identifier for random generator sequence.
	///
	/// When no arguments are provided to `--init-seq` flag
	const INIT_SEQ: &str = "45";

	impl Default for Pcg {
	/// Provides a default constructor for [`Pcg`](struct@Pcg),
	/// the default values for the seed are:
	/// - `init_state==42`;
	/// - `init_seq==54`.
	fn default() -> Self {
	Self::new(42, 54)
	}
	}

	for y in (0..height).rev() {
	for x in 0..width {
	match endianness {
	ByteOrder::LittleEndian => buf_reader
	.read_f32_into::<byteorder::LittleEndian>(&mut buffer)
	.map_err(HdrImageErr::PfmFileReadFailure)?,
	ByteOrder::BigEndian => buf_reader
	.read_f32_into::<byteorder::BigEndian>(&mut buffer)
	.map_err(HdrImageErr::PfmFileReadFailure)?,
	}
	hdr_img.set_pixel(x, y, (buffer[0], buffer[1], buffer[2]).into())?;
	}
	}

	let mut world = World::default();
	for x in [-0.5, 0.5].into_iter() {
	for y in [-0.5, 0.5].into_iter() {
	for z in [-0.5, 0.5].into_iter() {
	world.add(Box::new(Sphere::new(
	translation(Vector::from((x, y, z))) * scaling(Vector::from((0.1, 0.1, 0.1))),
	)));
	}
	}
	}
	world.add(Box::new(Sphere::new(
	translation(Vector::from((0.0, 0.0, -0.5))) * scaling(Vector::from((0.1, 0.1, 0.1))),
	)));
	world.add(Box::new(Sphere::new(
	translation(Vector::from((0.0, 0.5, 0.0))) * scaling(Vector::from((0.1, 0.1, 0.1))),
	)));

	let transformation = var.transformations.get(&transformation_id).copied().ok_or(
	SceneErr::UndefinedIdentifier {
	loc,
	msg: format!("{:?} transformation not defined", transformation_id),
	},
	)? * rotation_z(f32::to_radians(cli.angle_deg));

	transformation =
	transformation * self.parse_transformation(transformations, var)?;

	pub fn new(
	aspect_ratio: f32,
	distance: f32,
	transformation: Transformation,
	) -> PerspectiveCamera {
	PerspectiveCamera {
	aspect_ratio,
	distance,
	transformation,
	}
	}
	}

andros21 / rustracer Goto Github PK

rustracer's People

Contributors

Stargazers

Watchers

Forkers

rustracer's Issues

description of the problem

command

result

command

result

expected

todo 📘

Recommend Projects

Recommend Topics

Recommend Org