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

Revised Rust version not using function test_ray #15

Closed
jzakiya opened this issue Apr 22, 2021 · 3 comments
Closed

Revised Rust version not using function test_ray #15

jzakiya opened this issue Apr 22, 2021 · 3 comments

Comments

@jzakiya
Copy link
Contributor

jzakiya commented Apr 22, 2021

In going over the revised Rust version from a couple of days ago, the compiler says test_ray function not used, though the image does not appear affected because of it.

    fn test_ray(&self, ray: &Ray) -> Option<f32>
    {
        let intersect = self.intersections(ray);
        return match intersect {
            Some(result) => Some(result.dist),
            None => None
        }
    }

In the Ruby, Crystal, et al version it was used in

  def addLight(col, light, pos, norm, scene, thing, rd)
    ldis  = Vector.minus(light.pos, pos)
    livec = Vector.norm(ldis)
    neatIsect = self.testRay(Ray.new(pos, livec), scene)
   ...
 end

In Nim as

func getNaturalColor(scene: Scene, sp: SurfaceProperties,
  pos, rd, norm: Vector): Color =
  var rdNorm = rd.norm()
  for light in scene.lights:
    let
      ldis = light.pos - pos
      ldisLen = ldis.len()
      livec = ldis.norm()
      neatIsect = scene.testRay(pos, livec)
      ...
@edin
Copy link
Owner

edin commented Apr 25, 2021

It's updated now and the rest of the warnings related to the naming convention are fixed.

@jzakiya
Copy link
Contributor Author

jzakiya commented Apr 25, 2021

Great!!
I tried to fix it, but see you had to make 2 changes to make it work. Confirmed it creates correct image.

I was waiting on the fix before I submit a PR of tweaks that make it slightly faster and simpler, but will now.

@jzakiya
Copy link
Contributor Author

jzakiya commented Apr 27, 2021

Unless you feel otherwise, I'll just close this since you've resolved the issue that was raised.

@jzakiya jzakiya closed this as completed Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants