OrbitControl() Compatibility with imageLight() by fixing camera.

[perminder-17]

Resolves #6688

Changes:

Screenshots of the change:

orbitCont.mp4

Sketch for testing @davepagurek

https://editor.p5js.org/aman12345/sketches/_QiQ5omfK

PR Checklist

• npm run lint passes
• Inline documentation is included / updated
• Unit tests are included / updated

[davepagurek]

Thanks for taking this on, this is a great start! I left a few comments about some of the math and also a potential bug in some existing code that you haven't changed that is now revealed.

[davepagurek]

Maybe we can do something similar to what we do for the normal matrix above by:

• putting this block in an if (this.uniforms.uCameraMatrix) { ... } to avoid doing extra work for shaders that don't need it
• store a matrix on the renderer for this, similar to this._renderer.uNMatrix, to reuse resources

[davepagurek]

Typically for matrices used to update normals, we use the inverse transpose of the transform applied to the vertices, like we do above for the normal matrix. Does it work if we do the same here? (if both seem to work, it would be interesting logging both values to see if they differ.)

Assuming we do the inverse transpose, we maybe should rename the uniform to something like uCameraRotation to make it clear that we're not intending for it to be used to apply the camera to vertex positions; only for rotating normals.

[davepagurek]

The type of transform you do for normals and for positions are generally different (normals must always be perpendicular to the surface, while positions can squish; see the diagram in this SO answer for an idea of why we need something different for normals.) So generally you won't find yourself multiplying both positions and normals by the same matrix.

In this case, I think we only need to do it to the normal, and probably only after doing the reflection:

vec3 worldNormal = normalize(vNormal);
vec3 lightDirection = normalize( vViewPosition - worldCameraPosition );
vec3 R = reflect(lightDirection, worldNormal) * uCameraMatrix;

The idea is that, without modifying the normal or position here, we would be looking up the texture correctly if the sphere map that surrounds the scene is un-rotated. To account for its rotation, only at the end would we apply the rotation matrix to it.

[davepagurek]

I noticed something kinda interesting. When you rotate around 360 degrees, you can always see the sun in the reflection, even though presumably at some point it should be behind the sphere and not in the reflection:

Screen.Recording.2024-01-14.at.10.58.29.AM.mov

I wonder if this is a bug in mapTextureToNormal that we didn't notice before because we couldn't rotate the spheremap texture like we can now. Is that something you're interested in looking into as part of this change? The idea is to convert a normal into a spherical coordinate (an azimuthal angle and a polar angle, and a fixed radius of 1), and then map those angles to 2D texture coordinates on the input image. Maybe we're doing that slightly incorrectly?

[perminder-17]

When you rotate around 360 degrees, you can always see the sun in the reflection, even though presumably at some point it should be behind the sphere and not in the reflection

Good catch @davepagurek :')

Does it work if we do the same here? (if both seem to work, it would be interesting logging both values to see if they differ.)

I have commited the required changes you mentioned above and it's working exactly the same way as it was working earlier. Yes I noticed a sort of differences between the two.(with inverseTranspose and without it (earlier one)).

Without inverse Transpose	With inverseTranspose

I wonder if this is a bug in mapTextureToNormal that we didn't notice before because we couldn't rotate the spheremap texture like we can now. Is that something you're interested in looking into as part of this change? The idea is to convert a normal into a spherical coordinate (an azimuthal angle and a polar angle, and a fixed radius of 1), and then map those angles to 2D texture coordinates on the input image. Maybe we're doing that slightly incorrectly?

It's good that we catched this bug too. I will look into it and try to implement it.

[davepagurek]

Update: I was messing around with the same effect in Blender, and it looks like maybe that behaviour with the sun that I was mentioning is actually just how mirror balls look for real.

blender.mp4

[perminder-17]

Thanks for merging it ❤