Makeup Assignment #1 #6
Conversation
There was a problem hiding this comment.
- CPrimPlane::intersect returns -1 instead of false - yellow line on horizon (-2)
- CPrimTriangle::intersect gives incorrect results (-2)
- CCameraPerspective - good idea, but the results are not correct, extra 30 mins of work would made your implementation fine. I have debugged your code and gave the feedback in GittHub.
| float den = m_normal.dot(ray.dir); | ||
| if (den == 0) | ||
| { | ||
| return -1; |
There was a problem hiding this comment.
it is a Boolean function, it should be false. This error gives the yellow line on horizon in your renders.
| return false; | ||
| } | ||
| ray.t = t; | ||
| return true; |
There was a problem hiding this comment.
triangles are rendered only in the right part of the screen. I think somewhere inside this algorithm an error. Perhaps in the line float t = p[0] / ray.dir[0];I recommend to check the sign and take into consideration also other dimensions.
| // --- PUT YOUR CODE HERE --- | ||
|
|
||
| class CPrimDisc : public IPrim { |
There was a problem hiding this comment.
I would recommend to derive the disc from plane primitive here, however the implementation is fine
| { | ||
| public: | ||
| CCameraEnvironmental(Size resolution, const Vec3f& pos, float angle) | ||
| : ICamera(resolution) |
| auto height = getResolution().height; | ||
| auto width = getResolution().width; | ||
|
|
||
| double theta = 2 * M_PI * dx / height; |
There was a problem hiding this comment.
Here dx should be decided by width and not height.
The same error for dy.
| double x1 = sin(phi) * cos(theta); | ||
| double y1 = sin(phi) * sin(theta); | ||
| double z = cos(phi); | ||
| Vec3f spherePoint = Vec3f(x1, y1, z); |
There was a problem hiding this comment.
this point is on the unit sphere, which is located in the origin. So you can use it as direction vector. Next, you forgot the coordinate system transformation, in the WCS y axis looks up, in the sphere you generate, z axis looks up
| auto width = getResolution().width; | ||
|
|
||
| double theta = 2 * M_PI * dx / height; | ||
| double phi = acos(1 - 2 * dy / width); |
There was a problem hiding this comment.
Here would be enough to have phi changing from 0 to Pi. arccosine is not needed.
| } | ||
| virtual ~CCameraEnvironmental(void) = default; | ||
|
|
||
| virtual void InitRay(Ray& ray, int x, int y) override |
There was a problem hiding this comment.
I like your implementation, it general it looks correct. After a closer look, there are some error. You need to spend a bit more time for debugging.
| // input values | ||
| Vec3f m_pos; ///< Camera origin (center of projection) | ||
| }; | ||
|
|
There was a problem hiding this comment.
I suggest the following implementation based on yours:
float dx = static_cast<float>(x) / getResolution().width;
float dy = static_cast<float>(y) / getResolution().height;
float theta = 2 * Pif * dx;
float phi = Pif * dy;
float x1 = sinf(phi) * cosf(theta);
float z1 = sinf(phi) * sinf(theta);
float y1 = cosf(phi);
ray.org = m_pos;
ray.dir = Vec3f(x1, y1, z1);
ray.t = std::numeric_limits<float>::infinity();
Could not finalize the last task and get the correct renders for the Environmental camera.