Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Displacement Map: Add morph normal support #11271

Merged
merged 7 commits into from
Jun 19, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/renderers/shaders/ShaderChunk/defaultnormal_vertex.glsl
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
#ifdef FLIP_SIDED

objectNormal = -objectNormal;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave objectNormal un-negated for future transforms (like displacement) that don't care about surface orientation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I prefer this.

vec3 transformedNormal = - normalMatrix * objectNormal;

#else

vec3 transformedNormal = normalMatrix * objectNormal;

#endif

vec3 transformedNormal = normalMatrix * objectNormal;

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifdef USE_DISPLACEMENTMAP

transformed += normal * ( texture2D( displacementMap, uv ).x * displacementScale + displacementBias );
transformed += objectNormal * ( texture2D( displacementMap, uv ).x * displacementScale + displacementBias );

#endif
12 changes: 2 additions & 10 deletions src/renderers/shaders/ShaderChunk/project_vertex.glsl
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
#ifdef USE_SKINNING
vec4 mvPosition = modelViewMatrix * vec4( transformed, 1.0 );

vec4 mvPosition = modelViewMatrix * skinned;

#else

vec4 mvPosition = modelViewMatrix * vec4( transformed, 1.0 );

#endif

gl_Position = projectionMatrix * mvPosition;
gl_Position = projectionMatrix * mvPosition;
3 changes: 2 additions & 1 deletion src/renderers/shaders/ShaderChunk/skinning_vertex.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
skinned += boneMatY * skinVertex * skinWeight.y;
skinned += boneMatZ * skinVertex * skinWeight.z;
skinned += boneMatW * skinVertex * skinWeight.w;
skinned = bindMatrixInverse * skinned;

transformed = ( bindMatrixInverse * skinned ).xyz;
Copy link
Contributor Author

@jaxry jaxry May 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continue mutating transformed rather than creating a new variable. This removes the need to deal with a special case when skinning is enabled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not mutating transformed; you are overwriting it. So there are some implicit assumptions you are making. Can you please specify what they are?

Are you able to demonstrate that the w-component you are dropping is 1?

Copy link
Contributor Author

@jaxry jaxry May 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am making the assumption that the transformations applied to skinned are affine. If that's the case, then you can drop the w-component since it will always be 1. The matrices in question should be affine, since they originate from the Object3D's matrixWorld property, and are all intended to rotate/scale/skew/translate vertices around, rather than to project vertices.

The matrices in question:
https://github.com/mrdoob/three.js/blob/r85/src/objects/Skeleton.js#L136
https://github.com/mrdoob/three.js/blob/r85/src/objects/SkinnedMesh.js#L98

Is there a reason to suspect these matrices may sometimes not be affine?

Edit:
In fact, the only named non-affine transformation I can think of is the perspective projection. Obviously setting a bone's matrixWorld to a perspective projection is entirely nonsensical.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's the case, then you can drop the w-component since it will always be 1.

Not if the weights are all zero, for example. Of course, they shouldn't be.

You are assuming the weights are normalized. They are normalized in the constructor, so I think you are OK here. I still think doing an experiment to verify the value of .w would be a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, but requiring the weights to be normalized actually follows from the original assumption that the matrices are affine. If the weights don't add up to one, then skinned would be incompatible with all affine transformations. In fact, such transformations need the w-component to be one if you intend to transform the vector correctly.

In any case, I explicitly checked the weights by logging them for a bunch of Three.js examples, and they all add up to one. I checked the matrices as well and the last row for each of them is [0, 0, 0, 1], making them affine.

I believe in the end this is an entirely reasonable assumption to make.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing the experiment and confirming this. We should be OK, then.


#endif
10 changes: 1 addition & 9 deletions src/renderers/shaders/ShaderChunk/worldpos_vertex.glsl
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
#if defined( USE_ENVMAP ) || defined( PHONG ) || defined( PHYSICAL ) || defined( LAMBERT ) || defined ( USE_SHADOWMAP )

#ifdef USE_SKINNING

vec4 worldPosition = modelMatrix * skinned;

#else

vec4 worldPosition = modelMatrix * vec4( transformed, 1.0 );

#endif
vec4 worldPosition = modelMatrix * vec4( transformed, 1.0 );

#endif
2 changes: 1 addition & 1 deletion src/renderers/shaders/ShaderLib/depth_vert.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ void main() {
#include <skinbase_vertex>

#include <begin_vertex>
#include <displacementmap_vertex>
#include <morphtarget_vertex>
#include <skinning_vertex>
#include <displacementmap_vertex>
#include <project_vertex>
#include <logdepthbuf_vertex>
#include <clipping_planes_vertex>
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shaders/ShaderLib/meshphong_vert.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ void main() {
#endif

#include <begin_vertex>
#include <displacementmap_vertex>
#include <morphtarget_vertex>
#include <skinning_vertex>
#include <displacementmap_vertex>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since displacement is done along the post-transformed normal, displacement mapping needs to take place after position vertices have been transformed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be as precise as possible, you are displacing along the "morph-skinning-adjusted" normal in object space.

Ignoring skinning for the moment, morph normals do not have to be specified -- they are optional. One can specify morph targets only. In that case, the "adjusted" normal is the original normal.

It seems to me that in that case, displacement should be first. That would be problematic, unless you can demonstrate that order does not matter...

#include <project_vertex>
#include <logdepthbuf_vertex>
#include <clipping_planes_vertex>
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shaders/ShaderLib/meshphysical_vert.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ void main() {
#endif

#include <begin_vertex>
#include <displacementmap_vertex>
#include <morphtarget_vertex>
#include <skinning_vertex>
#include <displacementmap_vertex>
#include <project_vertex>
#include <logdepthbuf_vertex>
#include <clipping_planes_vertex>
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/shaders/ShaderLib/normal_vert.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ void main() {
#endif

#include <begin_vertex>
#include <displacementmap_vertex>
#include <morphtarget_vertex>
#include <skinning_vertex>
#include <displacementmap_vertex>
#include <project_vertex>
#include <logdepthbuf_vertex>

Expand Down