fix: Fix compatibility breaks at three r157 on MToon #1304
Conversation
| #if THREE_VRM_THREE_REVISION >= 157 | ||
| uniform vec3 lightProbe[ 9 ]; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
uniform vec3 lightProbe[ 9 ] is still defined in lights_pars_begin with a macro #if defined( USE_LIGHT_PROBES ), so I think it will cause a problem when we actually want to use light probes.
There was a problem hiding this comment.
MtBlue81-san just notices that it's a change from mrdoob/three.js#26768
| @@ -711,7 +777,9 @@ void main() { | |||
|
|
|||
| vec3 irradiance = getAmbientLightIrradiance( ambientLightColor ); | |||
|
|
|||
| #if THREE_VRM_THREE_REVISION >= 133 | |||
| #if THREE_VRM_THREE_REVISION >= 157 | |||
| irradiance += getLightProbeIrradiance( lightProbe, geometryNormal ); | |||
There was a problem hiding this comment.
(premise: https://github.com/pixiv/three-vrm/pull/1304/files#r1342518857)
so, we want to use #if defined( USE_LIGHT_PROBES ) here.
Having the macro #if THREE_VRM_THREE_REVISION >= 157 looks good
| vec3 geometryPosition = - vViewPosition; | ||
| vec3 geometryViewDir = ( isOrthographic ) ? vec3( 0, 0, 1 ) : normalize( vViewPosition ); |
There was a problem hiding this comment.
I wondered why we don't have to define geometryNormal here, and I found that the existing vec3 geometryNormal = normal in <normal_fragment_begin> have been renamed to nonPerturbedNormal in r157.
For consistency, I would love if you can rename the existing geometryNormal in the same way and add the new geometryNormal here.
mrdoob/three.js@7bf83b8#diff-0ef3a127f3d8eaf75269887ebb9da53c45aa287c14fc79b687c3dcfcd5d95cd9R74
There was a problem hiding this comment.
It's mostly awesome! I really appreciate your hard work, thank you...
Let me sum up the remaining changes I want to see before merging this:
- The
lightProbe,USE_LIGHT_PROBESissue - The
geometryNormal/nonPerturbedNormalrename - The
CLEARCOAT->USE_CLEARCOATrename
Co-authored-by: 0b5vr <0b5vr@0b5vr.com>
Co-authored-by: 0b5vr <0b5vr@0b5vr.com>
re-fix of 3cf43b9
|
We've released the change @ |
also removed an impossible compat code path
also removed an impossible compat code path
Problem
Changes
See: Shaders: Remove struct GeometricContext to improve Adreno GPU support. mrdoob/three.js#26805