6
\$\begingroup\$

For the past week I have been studying and improving a ray tracer that I ported from JavaScript. The bare finished port was only exactly what was written in JavaScript and rendered at around 20fps at 100x100 resolution with a scene of just one plane and two spheres, over the week I have implemented very simple frustum culling (although it doesn't help as nothing is occluded currently), I reduced the number of new objects created per ray by making the Color and Vector class more instance-based and multi-threaded the whole render job which has improved it to around 85fps.

While improving the code, debugging and benchmarking it i found that simply five math functions can impact the whole render in the end by 20ms! So as I do not understand the ins-and-outs of all the mathematical functions, I would appreciate any improvements that could be made highly.

private static Color traceRay(Scene scene, Raycast job, Ray ray, int depth) {
    if (depth >= maxRays) return job.sumColor;
    RaycastReport intersections = intersectScene(scene, ray);
    if (intersections == null) // Stop tracing if no hit
        return Color.voidColor;
    if (depth == 0) job.objdistance += intersections.distance;
    job.hits = depth;
    Color hitcolor = shade(job, intersections, scene, depth);
    job.sumColor.add(hitcolor);
    return hitcolor;
}


private static float testRay(Scene scene, Ray ray) {
    RaycastReport intersection = intersectScene(scene, ray);
    if (intersection == null)
        return -1;
    return intersection.distance;
}

private static RaycastReport intersectScene(Scene scene, Ray ray) {
    float closest = Float.POSITIVE_INFINITY;
    RaycastReport closestobject = null;
    for (RenderObject object : scene.objects) {
        RaycastReport intersection = object.intersect(ray);
        if (intersection != null && intersection.distance < closest) {
            closestobject = intersection;
            closest = intersection.distance;
        }
    }
    return closestobject;
}

private static Color shade(Raycast job, RaycastReport report, Scene scene, int depth) {
    Vector direction = report.ray.direction;
    Vector pos = Vector.add(Vector.multiply(report.distance, direction), report.ray.position);
    job.hitPositions[depth] = pos;
    Vector normal = report.object.getNormal(pos);
    direction.subtract(Vector.multiply(2, Vector.multiply(Vector.dot(normal, direction), normal)));
    Color naturalcolor = light(scene, report.object, pos, normal, direction);
    Color reflectedcolor = getReflectionColor(job, scene, report.object, pos, normal, direction, depth);
    return Color.add(naturalcolor, reflectedcolor);
}


private static Color light(Scene scene, RenderObject hitobject, Vector hitpos, Vector normal, Vector reflectdirection) { // Rename
    Color col = Color.defaultcolor;
    for (Light light : scene.lights) {
        Vector
        ldis = Vector.subtract(light.position, hitpos),
        livec = Vector.normal(ldis);
        float hitlight = testRay(scene, new Ray(hitpos, livec));
        boolean isinshadow = (hitlight == -1) ? false : (hitlight <= Vector.distance(ldis));
        if (isinshadow) continue;
        float illum = Vector.dot(livec, normal);
        Color lcolor = (illum > 0) ? Color.scale(illum, light.color) : Color.defaultcolor;
        float specular = Vector.dot(livec, Vector.normal(reflectdirection));
        Color scolor = (specular > 0) ? Color.scale((float)Math.pow(specular, hitobject.material.roughness), light.color) : Color.defaultcolor;
        lcolor.multiply(hitobject.material.diffuseColor(hitpos));
        scolor.multiply(hitobject.material.specularColor(hitpos));
        lcolor.add(scolor);
        col = Color.add(col, lcolor);
    }
    return col;
}

private static Color getReflectionColor(Raycast job, Scene scene, RenderObject object, Vector pos, Vector normal, Vector reflectdirection, int depth) {
    float reflectivity = object.material.reflectivity(pos);
    if (reflectivity == 0) return Color.black;
    return Color.scale(reflectivity, traceRay(scene, job, new Ray(pos, reflectdirection), depth+1));
}

Edit:

public abstract class RenderObject {
    public Material material;

    public RenderObject(Material mat) {
        material = mat;
    }

    public abstract Vector getNormal(Vector pos);
    public abstract RaycastReport intersect(Ray ray);
}

Raycast class contains useful general information about a rays journey as it was reflected such as distance, number of reflections and positions hit.
RaycastReport class is used to send data about the last raycast around more cleanly.
depth is used to limit the recursion that reflective objects use.

\$\endgroup\$
  • \$\begingroup\$ Try to follow some style guides like Google Java style guide, it will make your code more readable and will guide you with some best practices. \$\endgroup\$ – vivek Mar 28 '14 at 9:46
10
\$\begingroup\$

Let your code breathe
Your code is very hard to read - lines are long and crowded together, containing multiple method nesting (direction.subtract(Vector.multiply(2, Vector.multiply(Vector.dot(normal,...).

Reading code is like reading a book. If you don't give the reader some breathing room (a period, coma, paragraph) - your reader will get lost, and might abandon the book altogether.

You should consider adding a few blank lines to make the structure of your methods more apparent, for example:

private static Color traceRay(Scene scene, Raycast job, Ray ray, int depth) {
    if (depth >= maxRays) return job.sumColor;

    RaycastReport intersections = intersectScene(scene, ray);
    if (intersections == null) // Stop tracing if no hit
        return Color.voidColor;

    if (depth == 0) job.objdistance += intersections.distance;

    job.hits = depth;
    Color hitcolor = shade(job, intersections, scene, depth);
    job.sumColor.add(hitcolor);
    return hitcolor;
}

You should also consider extracting some of the hard computational lines to helper methods, and name them so that the reader will better understand your code:

private static Vector travel(Vector startPosition, Vector direction, float distance) {
    Vector travelPath = Vector.multiply(distance, direction)
    return Vector.add(travelPath, startPosition);
}

Variable Naming
Also hindering readability is your choice to use an alllower variable naming convention. naturalColor is much more readable than naturalcolor, all the more so isInShadow vs. isinshadow.

You also tend to use lazy shortcuts like ldis, livec, lcolor, illum, etc. which might seem meaningful to you, but to an outside reader the names might be less helpful.

Something must be missing...

for (Object object : scene.objects) {
    RaycastReport intersection = object.intersect(ray);

How does your object intersect with ray? As far as I know, there is no such method on Object... It is quite hard to calculate complexity of code, if we can't know how this intersection happens, especially since it happens within a polynomial iteration (lights*objects) in testRay, and within a recursion in traceRay.

Caching?
If the complexity of intersect is a constant, your complexity would be \$O(maxRays \cdot objects \cdot lights)\$ (I'm counting your Vector operations' complexity as constant), this might not be ideal.

I've got lost in all the recursion and iteration loop you have there, but might there be a way to cache some of the calculations? Would it help you to "remember" distances between your scene (which does not change during your calculations), and your rays, and reuse them as you go along? This might lower your effective complexity, and make your code run a lot faster.

\$\endgroup\$
  • \$\begingroup\$ If I where to cut up the lines into many variables, surely that would only make it more messy, I wouldn't know what to name them either, as I said, I don't understand the mathematics much, I was hoping someone here would. I have added my Object class (which I've now named more appropriately to RenderObject), meanwhile, I will fix the "lazy" names and space things out. Have you got any suggestions on how I could go about caching even though the camera moves? \$\endgroup\$ – Lee Allan Mar 28 '14 at 15:08
  • \$\begingroup\$ I suggested that you add blank lines, so your code won't look like a big kludge of code. Making a few iterations of method extractions (like the one in my example), will make you understand the code better. Unfortunately, I'm not actively familiar with vector optics, and I can't tell you how you could effectively cache these results... Perhaps debug some results, and try to see if you get similar/identical calculations over and over, and cache those results... \$\endgroup\$ – Uri Agassi Mar 28 '14 at 15:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.