Skip to content
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

bvh_node is broken with negative-radius spheres #733

Closed
define-private-public opened this issue Sep 17, 2020 · 7 comments
Closed

bvh_node is broken with negative-radius spheres #733

define-private-public opened this issue Sep 17, 2020 · 7 comments

Comments

@define-private-public
Copy link

define-private-public commented Sep 17, 2020

I wanted to create some "Matryoshka hollow glass spheres", so I setup this scene:

    hittable_list world;

    const color ground_yellow(0.8, 0.8, 0);
    const color gold(0.8, 0.6, 0.2);
    const color blue(0.1, 0.2, 2);
    const color green(0.1, 0.9, 0.3);
    const double glass_refractive_index = 1.5;
    const auto ground_mat = make_shared<lambertian>(ground_yellow);
    const auto blue_mat   = make_shared<lambertian>(blue);
    const auto left_mat   = make_shared<dielectric>(glass_refractive_index);
    const auto center_mat = make_shared<lambertian>(green);
    const auto right_mat  = make_shared<metal>(gold, 1);
    const double tiny_y = 0.005;

    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.5,  left_mat));
    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1), -0.35, left_mat));
    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.3,  left_mat));
    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1), -0.2,  left_mat));
    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.15, left_mat));
    world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.1,  blue_mat));
    world.add(make_shared<sphere>(point3( 0,  0.15 , -1),   .5,  center_mat));
    world.add(make_shared<sphere>(point3( 1,  tiny_y, -1),  0.5,  right_mat));
    world.add(make_shared<sphere>(point3( 0, -100.5, -1),   100,  ground_mat));   // The ground (Big boi)

    const bvh_node world_bvh(world, 0, 1); 

    point3 lookfrom(0, 0.25, 3.25);
    point3 lookat(0, 0, -1);
    auto vfov = 25.0;
    auto aperture = 0.0;
    color background = color(0.70, 0.80, 1.00);

    // Camera

    const vec3 vup(0,1,0);
    const auto dist_to_focus = (lookfrom - lookat).length();
    const int image_height = static_cast<int>(image_width / aspect_ratio);

    camera cam(lookfrom, lookat, vup, vfov, aspect_ratio, aperture, dist_to_focus, 0.0, 1.0);

When rendering on world, I get this, which is correct:
three_spheres_no_bvh

But when I render with world_bvh, this is the result:
three_spheres_bvh

I don't think that's right.

@trevordblack trevordblack added this to the Backlog milestone Sep 22, 2020
@hollasch hollasch modified the milestones: Backlog, v4.0.0 Oct 12, 2020
@hollasch
Copy link
Collaborator

Ref. #532

I believe that this is due to using negative radius spheres, and that you should be using appropriate indices of refraction for your air-inside-glass bubbles.

Can you try the following instead?

  hittable_list world;

  const color ground_yellow(0.8, 0.8, 0);
  const color gold(0.8, 0.6, 0.2);
  const color blue(0.1, 0.2, 2);
  const color green(0.1, 0.9, 0.3);
  const double glass_refractive_index = 1.5;
  const auto ground_mat = make_shared<lambertian>(ground_yellow);
  const auto blue_mat   = make_shared<lambertian>(blue);
  const auto left_mat   = make_shared<dielectric>(glass_refractive_index);
+ const auto left_air   = make_shared<dielectric>(1/glass_refractive_index);
  const auto center_mat = make_shared<lambertian>(green);
  const auto right_mat  = make_shared<metal>(gold, 1);
  const double tiny_y = 0.005;

  world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.5,  left_mat));
- world.add(make_shared<sphere>(point3(-1,  tiny_y, -1), -0.35, left_mat));
+ world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.35, left_air));
  world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.3,  left_mat));
- world.add(make_shared<sphere>(point3(-1,  tiny_y, -1), -0.2,  left_mat));
+ world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.2,  left_air));
  world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.15, left_mat));
  world.add(make_shared<sphere>(point3(-1,  tiny_y, -1),  0.1,  blue_mat));
  world.add(make_shared<sphere>(point3( 0,  0.15 , -1),   .5,  center_mat));
  world.add(make_shared<sphere>(point3( 1,  tiny_y, -1),  0.5,  right_mat));
  world.add(make_shared<sphere>(point3( 0, -100.5, -1),   100,  ground_mat));   // The ground (Big boi)

  const bvh_node world_bvh(world, 0, 1); 

  point3 lookfrom(0, 0.25, 3.25);
  point3 lookat(0, 0, -1);
  auto vfov = 25.0;
  auto aperture = 0.0;
  color background = color(0.70, 0.80, 1.00);

  // Camera

  const vec3 vup(0,1,0);
  const auto dist_to_focus = (lookfrom - lookat).length();
  const int image_height = static_cast<int>(image_width / aspect_ratio);

  camera cam(lookfrom, lookat, vup, vfov, aspect_ratio, aperture, dist_to_focus, 0.0, 1.0);

@hollasch
Copy link
Collaborator

Also, I'm assuming this is for Ray Tracing: The Next Week, right?

@hollasch hollasch self-assigned this Oct 29, 2020
@hollasch hollasch changed the title bvh_node is broke in a specific case bvh_node is broken with negative-radius spheres Oct 29, 2020
@define-private-public
Copy link
Author

The "smaller negative" sphere is a trick that @petershirley mentioned as a way of producing a hollow glass sphere. I tried out you recommendation of doing the 1 / glass_refractive_index. It produced this:

No BVH:
render_inv_refract_no_bvh

With BVH:
render_inv_refract_bvh

It doesn't look right. The BVH node is from "Ray Tracing: The Next Week".

define-private-public added a commit to define-private-public/PSRayTracing that referenced this issue Oct 30, 2020
@define-private-public
Copy link
Author

I figured out the fix. When making the aabb for the sphere to use be used for the bvh_node, std::abs() should be wrapped around the radius. I'll submit a patch.

define-private-public added a commit to define-private-public/raytracing.github.io that referenced this issue Oct 30, 2020
Fix is to make sure that the AABB's is always a positive real number it
should never be negative.

Closes RayTracing#733
define-private-public added a commit to define-private-public/PSRayTracing that referenced this issue Oct 30, 2020
RayTracing/raytracing.github.io#733

Simply make sure that the AABB's are always using a positive radius.
@hollasch
Copy link
Collaborator

The entirely blue sphere is very surprising. I can't imagine how that could happen.

hollasch added a commit that referenced this issue Nov 24, 2020
The original constructor expected that the first parameter was the
minimum point, and the second was the maximum. The new constructor now
selects the minimums of all coordinates in the two parameters, and the
maximums of all the coordinates. In this way, the two parameters are
just two extreme points of the bounds, so is significantly more robust.

This change was done because the original code had bugs when handling
spheres of negative radius (used to model glass spheres with voids).

Resolves #733
@hollasch
Copy link
Collaborator

I think the above fix should get things working for you. Please let me know if this is untrue.

@define-private-public
Copy link
Author

Hi, sorry for the delays.

I've been quite preoccupied these past few weeks. I'll be taking a look at it most likely during the upcoming Holiday/weekend.

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

Successfully merging a pull request may close this issue.

3 participants