ElisaLegnani / PhotorealisticRendering

A simple raytracer for generating photorealistic images written in C++
https://elisalegnani.github.io/PhotorealisticRendering/
GNU General Public License v3.0
3 stars 0 forks source link

Remove redundancy in ONB constructor #15

Closed ziotom78 closed 3 years ago

ziotom78 commented 3 years ago

In the branch pathtracing, there are two implementations of ONB::ONB (lines 222–257 in geometry.h), which are essentially the same but accept either a Vec or a Normal.

It would be better to create a constructor that takes the three parameters x, y, and z:

class ONB {
    // …
    ONB(float x, float y, float x) {
        // Put here your implementation of the OBN algorithm
    }
    // …
};

and then call it from the other two constructors:

class ONB {
    // …
    ONB(Vec normal) : ONB{normal.x, normal.y, normal.z} {}
    ONB(Normal normal) : ONB{normal.x, normal.y, normal.z} {}
};

In this way you have just one implementation of the algorithm and it is easier to catch bugs.

adelezaini commented 3 years ago

Thank you for the enhancement suggestion. We fixed the issue as recommended. Please do not hesitate to report any other bugs or feature requests! We would be pleased...