New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement HairMaterial #38

Closed
wahn opened this Issue Jan 11, 2018 · 9 comments

Comments

Projects
None yet
1 participant
@wahn
Copy link
Owner

wahn commented Jan 11, 2018

Once we solved issue #37 to create curves, which can be used as hair, we need a matching HairMaterial:

$ rg '"hair"' ~/Graphics/Rendering/PBRT/pbrt-v3-scenes/hair/
/home/jan/Graphics/Rendering/PBRT/pbrt-v3-scenes/hair/curly-hair.pbrt
14:	Material "hair" "float eumelanin" .3

/home/jan/Graphics/Rendering/PBRT/pbrt-v3-scenes/hair/straight-hair.pbrt
14:	MakeNamedMaterial "black_hair" "string type" [ "hair" ] "float eumelanin" [ 8 ]
15:	MakeNamedMaterial "red_hair" "string type" [ "hair" ] "float eumelanin" [ 3 ]
16:	MakeNamedMaterial "brown_hair" "string type" [ "hair" ] "float eumelanin" [ 1.3 ] "float beta_m" .25 "float alpha" 2
17:	MakeNamedMaterial "blonde_hair" "string type" [ "hair" ] "float	eumelanin" [ .3 ]

/home/jan/Graphics/Rendering/PBRT/pbrt-v3-scenes/hair/sphere-hairblock.pbrt
17:  Material "hair" "rgb color" [ .2 .8 .3 ]

Here is the source code for the C++ implementation:

$ rg -tcpp "class HairMaterial :" ~/git/github/pbrt-v3/src/
/home/jan/git/github/pbrt-v3/src/materials/hair.h
57:class HairMaterial : public Material {

@wahn wahn self-assigned this Jan 11, 2018

@wahn wahn added the task label Jan 11, 2018

wahn referenced this issue Jan 19, 2018

wahn referenced this issue Jan 19, 2018

wahn referenced this issue Jan 23, 2018

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 23, 2018

Let's debug this:

$ RUST_BACKTRACE=1 ./target/release/examples/pest_test -i assets/scenes/hair/curly-hair.pbrt
...
thread '<unnamed>' panicked at 'assertion failed: !ret.has_nans()', src/core/spectrum.rs:1732:9
...
stack backtrace:                                                                                                                                                                                                                                                                          
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace                                                                                                                                                                                                                           
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49                                                                                                                                                                                                                             
   1: std::sys_common::backtrace::print                                                                                                                                                                                                                                                   
             at libstd/sys_common/backtrace.rs:68                                                                                                                                                                                                                                         
             at libstd/sys_common/backtrace.rs:57                                                                                                                                                                                                                                         
   2: std::panicking::default_hook::{{closure}}                                                                                                                                                                                                                                           
             at libstd/panicking.rs:380                                                                                                                                                                                                                                                   
   3: std::panicking::default_hook                                                                                                                                                                                                                                                        
             at libstd/panicking.rs:396                                                                                                                                                                                                                                                   
   4: std::panicking::rust_panic_with_hook                                                                                                                                                                                                                                                
             at libstd/panicking.rs:576                                                                                                                                                                                                                                                   
   5: std::panicking::begin_panic                                                                                                                                                                                                                                                         
   6: <pbrt::materials::hair::HairBSDF as pbrt::core::reflection::Bxdf>::f                                                                                                                                                                                                                
   7: pbrt::core::reflection::Bsdf::f                                                                                                                                                                                                                                                     
   8: pbrt::core::integrator::estimate_direct
   9: pbrt::core::integrator::uniform_sample_one_light
  10: <pbrt::integrators::path::PathIntegrator as pbrt::core::integrator::SamplerIntegrator>::li
  11: <F as crossbeam::FnBox>::call_box
  12: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102
  13: <F as alloc::boxed::FnBox<A>>::call_box
  14: std::sys::unix::thread::Thread::new::thread_start
             at /checkout/src/liballoc/boxed.rs:798
             at libstd/sys_common/thread.rs:24
             at libstd/sys/unix/thread.rs:90
  15: start_thread
  16: clone
@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 23, 2018

thread '<unnamed>' panicked at 'attempt to multiply with overflow', src/materials/hair.rs:660:22

wahn added a commit that referenced this issue Jan 23, 2018

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 23, 2018

The Rust version kind of renders now (at least without any panics):

curly-hair_low_res_rust

But there is a difference between C++ and Rust, which we have to attack at some point:

curly-hair_low_res_cpp

> imf_diff -d curly-hair_low_res_*.png diff.jpg
differing pixels:	 52.405% (137376 of 262144)
average difference:	  5.480%
maximum difference:	 59.144%
Summary: Many pixels differ strongly.
== "curly-hair_low_res_cpp.png" and "curly-hair_low_res_rust.png" are different

diff

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 24, 2018

Here some results with higher resolution, more pixel samples, and straight hair:

C++

straight-hair_cpp

Rust

straight-hair_rust

> imf_diff -d -f straight-hair_cpp.png straight-hair_rust.png diff.jpg 
differing pixels:	 14.389% (150877 of 1048576)
average difference:	  1.634%
maximum difference:	 37.608%
Summary: Many pixels differ strongly.
== "straight-hair_cpp.png" and "straight-hair_rust.png" are different

diff

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 24, 2018

To see what's going wrong on the Rust side I did zoom into the straight hair image rendered by the Rust version and identified which sample returns such a high value that it basically looks red:

-                                    l = integrator.li(&mut ray,
-                                                      scene,
-                                                      &mut tile_sampler, // &mut arena,
-                                                      0_i32);
+                                    if (pixel.x == 546 || pixel.x == 547 || pixel.x == 548)
+                                        && (pixel.y == 435 || pixel.y == 436 || pixel.y == 437)
+                                    {
+                                        l = integrator.li(
+                                            &mut ray,
+                                            scene,
+                                            &mut tile_sampler, // &mut arena,
+                                            0_i32,
+                                        );
+                                        if pixel.x == 547 && pixel.y == 436 && l.c[0] >= 1.0 {
+                                            println!("l({:?}) = {:#?}", tile_sampler.get_current_sample_number(), l);
+                                        }
+                                    }

So it's pixel (547, 436) and the current sample number is 251:

l(251) = RGBSpectrum {
    c: [
        954.47235,
        50.898922,
        0.04819041
    ]
}
@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Jan 24, 2018

The fireflies are even more prominent in this example:

pbrt

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Feb 1, 2018

After commit 3b1e99b the strongest differences are gone:

> imf_diff straight-hair_rust.png /mill3d/users/jan/tmp/pbrt/straight-hair_rust.png
differing pixels:	  0.296% (3099 of 1048576)
average difference:	  1.661%
maximum difference:	 37.608%
Summary: Some pixels differ strongly.
== "straight-hair_rust.png" and "/mill3d/users/jan/tmp/pbrt/straight-hair_rust.png" are different

Above is the difference between the old and the new Rust version. Below is the difference between the new Rust version and C++:

> imf_diff straight-hair_rust.png /mill3d/users/jan/tmp/pbrt/straight-hair_cpp.png
differing pixels:	 14.079% (147634 of 1048576)
average difference:	  1.622%
maximum difference:	 14.059%
Summary: Many pixels differ.
== "straight-hair_rust.png" and "/mill3d/users/jan/tmp/pbrt/straight-hair_cpp.png" are different

There are still differences though, they are just less prominent:

diff

A better comparison will be the curly hair scene ... that had more visible fireflies

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Feb 1, 2018

The fireflies from the 512x512 version of the curly hair rendering are gone:

curly-hair_debug_rust

There is a difference to the C++ version (here without false colors):

> imf_diff -d curly-hair_debug_rust.exr curly-hair_debug_cpp.exr diff.jpg
differing pixels:	 50.100% (131334 of 262144)
average difference:	  4.459%
maximum difference:	 23.612%
Summary: Many pixels differ.
== "curly-hair_debug_rust.exr" and "curly-hair_debug_cpp.exr" are different

diff

I'm still rendering a 1024x1024 version with 1024 pixel samples and will publish the result here once it's done, but for now I will close this issue ...

@wahn wahn closed this Feb 1, 2018

@wahn

This comment has been minimized.

Copy link
Owner

wahn commented Feb 2, 2018

Here is a high resolution image, 1024x1024 pixels with 1024 pixel samples, rendered by the Rust version in about 21 hours and 12 minutes (on my macOS laptop with 16 GB of memory):

curly-hair_pbrt_rust

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment