ShaderChunks: Fix orthographic view direction #17767
Conversation
|
Can you please add this to the two examples: geometry = new THREE.IcosahedronBufferGeometry( 128, 0 ); |
|
@WestLangley Looks good? |
As @sciecode and I discussed offline, this change has weird side effects in environment reflections. I am not sure I would support this PR. |
Yes. In reality neither the dev model nor the PR model are truly correct or incorrect. Both are approximating some aspect of orthographic projection correctly, but not entirely. This ambiguous way of handling orthographic projections is noticeable across multiple engines: Unity and BabylonJS are using perspective view direction model for light and reflections. It is a subjective choice. Personally, I don't like having the fresnel component completely detached from the mesh as we have on dev; but I, equally, dislike the flat reflections on this PR. So, I'm afraid we gonna need a judgement call. PS: Cinema4D is the only example I could find using a mixed approach. Orthographic view direction for lighting and perspective view direction for reflections. I like this approach, but it isn't very popular or considered standard. |
|
@WestLangley As discussed offline, the reflections in orthographic projection looks good to me after this PR. Are you happy with the implementation in this PR? |
| vec3 cameraToFrag; | ||
|
|
||
| if ( isOrthographic ) { | ||
|
|
||
| cameraToFrag = normalize( vec3( -viewMatrix[0][2], -viewMatrix[1][2], -viewMatrix[2][2] ) ); | ||
|
|
||
| } else { | ||
|
|
||
| cameraToFrag = normalize( vWorldPosition - cameraPosition ); | ||
|
|
||
| } |
There was a problem hiding this comment.
Not tested, but may work.
vec3 cameraToFrag = ( isOrthographic ) ? normalize( - viewMatrix[ 2 ].xyz ) : normalize( vWorldPosition - cameraPosition );There was a problem hiding this comment.
No, doesn't work. matrix[i] is a column selector. Unfortunately, I don't think there is a row selector on GLSL.
There was a problem hiding this comment.
Oh, right. You are selecting the row, not the column.
Only the white space needs to be corrected: - viewMatrix[ 0 ][ 2 ]
When @sciecode has fully-tested it, we can give it a try. |
I'll just make sure I haven't missed any cases where I need to input |
|
PR is ready to merge. Should I make a follow up PR adjusting example shaders to work correctly with orthographic projection as well? |
That would be great yeah! |
|
Does any conclusions of this discussion transfer to distance-based fog? The proposal in #17592 assumes (like @supereggbert's and several older ones) that this is independent of camera type, i.e. that the distance is considered from eye to fragment. An alternative for orthographic camera could be to take distance along pixel normals, i.e. depth-based fog... Also mentioned in #17592 (comment) I think that to answer the question, one needs to consider the use cases. If map top views are the main application of ortho in scenes with fog, then it makes sense that the fog density is independent of screen position (depth-based, not distance-based). But I barely cannot think of an example where I wouldn't disable fog for map views... |
I can't think of a reason why anyone would want to use @mrdoob @WestLangley I believe every stylistic changes were covered, any chance we could get this merged before r110? edit: I didn't explicitly set this PR to close #17662, but it's fair to say we can close when we merge. |
|
Looks good to me! |
|
Thanks! |
|
@EliasHasle Noes, I haven't studied that case yet. Probably worth its own issue? |
|
I guess it is natural to discuss it in #17592 . |
Fixes #17379.
Based on #17662.
Adds
isOrthographicuniform only for light/view dependant built-in materials.Correctly handles view direction for orthographic projection, specular lighting and environment maps.