But removing the rotation_z transformation the test will pass.
I suppose that a previous errata updated the transformation matrix m but not the resulting normal vector n.
This is my normal with the rotation_z:
x: -0.4149850363522584, y: 0.8620719608670172, z: -0.29089405956569586
I don’t think that you’re right about this. Mine passes all tests with the z-axis rotation in. It also passes without, but that actually makes sense. First, we would rotate the original point into world space. We would then find the vector and then rotate it back to world space, causing it to point in the same direction as before. Without the rotation, we just don’t do and then undo the rotation. The rotation in this test does pretty much nothing because we already have the intersection point. It would be very different (in later chapters) when we cast a ray from world space in a direction to intersect the sphere. Then, it will make quite a bit of difference.
Please correct me if my logic is off, and I have not done the math by hand to verify all of this, but it makes logical sense to me. And my tests are passing without making any changes like you recommend.
My reasoning is as follows: the scaling transformation makes the sphere an ellipsoid, i.e. a sphere “flattened” on the y axis. Unlike a sphere, if you rotate an ellipsoid the normal change its orientation. So to me seems strange that with or without a rotation the normal stays the same.
For how the test is formulated, the normal lay on the y-z plane but my intuition says that it cannot be possible, the normal must have an x coordinate not equal to zero.
My thought is reinforced if you read the old errata page of the book:
Search for “normal_at” and you’ll find an errata on page 84 when a reader had a problem on the same test: with or without the rotation the test passed because the reader forgot to add the transpose operation.
The test was changed to add a scale and a rotation just to be sure to catch this error. In my opinion, this means that now the test must pass only with a rotation and not without it.
Just to be sure, I’ve added many more tests on my suite, with combinations of scaling, translations, and rotations.
What do you think about it? Please, let me know your thoughts!
I think that I see the problem. You are applying the transformations in the wrong order. You are not rotating an ellipsoid in that test at all. Transformations are applied to a point in the opposite direction of what they appear, so in this test, the sphere is rotated and then scaled.
The blue ellipsoid is generated using the the order in the test (Scaling * Rotation) while the pinkish one is generated with the opposite order (Rotation * Scaling). This demonstrates that you are rotating a sphere in the test and then squishing it to an ellipsoid, not the other way around.
I agree with you that the other way would definitely rotate the normal, but I don’t believe it would in this case. Perhaps maybe Jamis intended for the second case when he made this correction, but inadvertently used this one which has no effect with or without the rotation.
Yes, you are right.
In my code I choose a fluent API, in witch the order of the matrix multiplications is reversed behind the scene.
So instead of using scaling(1, 0.5, 1).rotation_z(pi/5) (that equals to m = rotation_z * scaling) I should have written the opposite: rotation_z(pi/5).scaling(1, 0.5, 1).
I ran into this too. I’m also using a fluent interface and transcribed the test wrong. Thank you both for helping me understand what’s going wrong.
It’s a bit odd that the rotation is even part of the transformation since it’s clearly a no-op when applied to the unit sphere centered on the object origin. After fixing my test transcription bug, I came up with an alternative test where each part of the transform has a meaningful impact:
Scenario: Given the normal on a transformed sphere
Given s <- sphere()
And m <- scaling(1, 0.5, 1) * shearing(1, 0, 0, 0, 0, 0)
And set_transform(s, m)
When n <- normal_at(s, point(0, sqrt(2)/2, sqrt(2)/2))
Then n = vector(-0.24077171, 0.96308684, -0.120385855)
But beware, I have not verified the computation beyond using my implementation to get the normal vector. Also, note if you’re using a fluent interface for your transforms, the identity should be followed by shearing.