Displacement Map: Add morph normal support #11271

jaxry · 2017-05-02T21:09:18Z

Currently displacement mapping uses the pre-transformed normal attribute to calculate the displacement vector. If morph normals are enabled, then the actual surface normal points in a different direction than the normal attribute, meaning the surface is displaced in the wrong direction.

This pull request changes the displacement mapping to use the post-transformed object normal.

I took the liberty to slightly refactor the skinning shader chunk to avoid having to introduce yet another #ifdef USE_SKINNING case. Is this change preferable?

jaxry · 2017-05-02T21:10:29Z

src/renderers/shaders/ShaderChunk/defaultnormal_vertex.glsl

@@ -1,7 +1,11 @@
 #ifdef FLIP_SIDED

-	objectNormal = -objectNormal;


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

I agree. I prefer this.

jaxry · 2017-05-02T21:11:55Z

src/renderers/shaders/ShaderLib/meshphong_vert.glsl

 	#include <morphtarget_vertex>
 	#include <skinning_vertex>
+	#include <displacementmap_vertex>


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

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...

jaxry · 2017-05-02T21:16:35Z

src/renderers/shaders/ShaderChunk/skinning_vertex.glsl

@@ -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;


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

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?

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.

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.

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.

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

WestLangley · 2017-05-03T00:48:35Z

This has been discussed previously.

Displacement mapping and morph targets? Is there a modeling tool that generates such models?

jaxry · 2017-05-03T03:58:06Z

@WestLangley
I'm confused by what you're asking. As far as I understand it, displacement mapping and morph targets are entirely unrelated things and are compatible with one another.

Displacement mapping is typically used to add detail to an object, and it accomplishes the same thing as normal mapping or bump mapping, just by a different means. In regards to the previous pull request you linked, I agree with @bhouston that displacement maps usually share the same UV set with normal/bump maps (thus sharing the offset/repeat), since as he says, they're all varying degrees of sub-face surface detail.

As for a use case for displacement mapping and morph normals, here's a dumb example: Suppose you're modeling a highly detailed sheet of bubble wrap. You could represent the sheet as a smoothly flowing surface and add the details of the individual bubbles via a displacement map. Then you could animate the sheet to different positions by morphing.

bhouston · 2017-05-03T13:45:45Z

The main issues with displacement mapping in WebGL is that WebGL doesn't support tessellation shaders, thus you can only deform the original vertex positions, thus any details in the displacement map that are between vertices are not represented. It greatly limits the usefulness of displacement maps in Three.JS.

Details: https://www.khronos.org/opengl/wiki/Tessellation

WestLangley · 2017-05-03T17:01:25Z

@jaxry wrote

Currently displacement mapping uses the pre-transformed normal attribute to calculate the displacement vector. If morph normals are enabled, then the actual surface normal points in a different direction than the normal attribute, meaning the surface is displaced in the wrong direction.

You appear to be making the assumption that morphs would precede displacement. Maybe so. That is the order that makes the most sense to me, too.

I have not seen a use case in which morph targets and displacement maps (or skeletal animation and displacement maps) were used concurrently. This is why I asked my question above.

Can you identify a modeling tool that does this?

If you cannot, my assumption would remain that they are not used together, so the order of include files is irrelevant.

Regardless, the order we apply operations must be consistent with that of the modeling software.

bhouston · 2017-05-03T17:17:18Z

I have not seen a use case in which morph targets and displacement maps (or skeletal animation and displacement maps) were used concurrently. This is why I asked my question above.

Displacement maps are part of high-end game characters in CryEngine and I believe UE4. Here is an old NVIDIA demo combining tessellation, skinning and displacement maps: https://www.youtube.com/watch?v=1c_PVtMIz-A But again it doens't work well in WebGL because we are restricted to displacing non-tessellated geometry.

jaxry · 2017-05-04T05:46:36Z

@bhouston wrote

But again it doens't work well in WebGL because we are restricted to displacing non-tessellated geometry.

I agree, however that doesn't make displacement mapping totally worthless. If you're working with a very fine mesh, then displacement mapping is a viable way to add extra detail to an object. That happens to be my use case for example.

@WestLangley
I'm writing an implicit surface plotter that uses a mesh detailed with PBR textures including albedo, normals, and displacement. I'm also making use of morphing to animate between surfaces, so to suggest displacement mapping and morphing don't go together sounds absurd to me.
A clip of displacement maps and morphing in action.

Can you identify a modeling tool that does this?

Blender. You can apply a displacement modifier to a surface and morph it via shape keys concurrently.

As for when displacement mapping should be applied, here's what I know:

Displacement mapping via a heightfield is most often done along a surface's normal. This is how every source I've studied defines it. Morphing doesn't change that. A intermediate surface created via morphing is still a surface, and displacement should still take place along the surface normal, exactly how it's done with normal maps.
Because morphing only translates position vectors, displacement mapping can be applied before or after morphing for the same exact results.
If bones simply rotate or translate position vectors (which is usually the case), then displacement mapping can also be applied either before or after for the same exact results. The exception is if the bones scale vertices. If positions are scaled, then we have to decide whether displacing should take place before or after the scaling.

I don't have much experience with any modeling programs in particular. Would anyone be willing to check how a modeling tool handles the scale of displacement maps when the bones are scaled differently relative to one another?

jaxry · 2017-05-04T22:59:28Z

I redesigned the code to make as few assumptions as possible. Displacement once again takes place before morphing and skinning. If morph normals are provided, displace in the direction of the morph normals. Otherwise displace in the direction of the pre-morphed normal. Does the code look okay?

From my tests, I can verify that displacement looks correct with both skinning and morphing, and the displacement direction always appears to be perpendicular to the surface.

Should I also revert the changes made to skinning?

WestLangley · 2017-05-04T23:47:00Z

I would recommend the following for now. See how you feel about it. I know this is somewhat subjective.

defaultnormal_vertex.glsl

vec3 transformedNormal = normalMatrix * objectNormal;

#ifdef FLIP_SIDED

	transformedNormal = - transformedNormal;

#endif

displacementmap_vertex.glsl

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

morphnormal_vertex.glsl - do not change
project_vertex.glsl - your change is OK
skinning_vertex.glsl - your change is OK
worldpos_vertex.glsl - your change is OK
depth_vert.glsl - do not change
meshphong_vert.glsl - do not change
meshphysical_vert.glsl - do not change
normal_vert.glsl - do not change

jaxry · 2017-05-05T00:24:20Z

Unfortunately, objectNormal is a skin transformed normal so it won't work for displacement mapping if we do it before skinning. That's why I was forced to create an intermediate morphNormal vector. I agree the added complexity doesn't look nice, but is there another way to do it?

WestLangley · 2017-05-05T00:42:23Z

So are we concluding that objectNormal is OK to use for displacement if displacement is applied last? The exception would be if morph normals are not specified?

Very nice demo, BTW. :)

jaxry · 2017-05-05T01:01:18Z

So are we concluding that objectNormal is OK to use for displacement if displacement is applied last?

Yes.

The exception would be if morph normals are not specified?

No. Displacement applied last will still work as intended if morph normals are not specified. objectNormal will simply be the normal attribute transformed by bones in that case.

But it's important to note that using objectNormal last has a slightly different effect than using morphNormal first. Specifically, if displacement is done before skinning, then any scaling of positions done by the bones will also scale the displacement. That's how three.js currently does it and I don't think we should change that without a good reason.

WestLangley · 2017-05-05T01:36:03Z

Well then, let's see how it looks with displacement last. Sorry for the back-and-forth.

jaxry · 2017-05-05T08:32:46Z

I already implemented displacement last with the original commit. Are you asking me to revert the most recent changes back to what we had before?

Either order works so it's up to you at this point. I'm testing the code with a mesh that combines displacement, skinning, and morphing. Aside from the scaling caveat I mentioned, the results are identical.

By the way, thanks a lot for being so patient and working through this with me. Who knew this small change would have been such a big can of worms.

WestLangley · 2017-05-05T18:19:16Z

I already implemented displacement last with the original commit.

I know. Sorry...

Unfortunately, objectNormal is a skin transformed normal so it won't work for displacement mapping if we do it before skinning.

That means that displacement is best if performed last. Do you agree? (I am hoping so, because it is the only order that made sense to me from the beginning. I just was not sure if that order was correct if all three methods are used simultaneously -- or if they ever would be used simultaneously.)

Also, if displacement is last, then proper normals must be used for displacement. So if a user uses both morphs and displacements, then a failure to specify correct morph normals is a user error.

Thanks a lot for being so patient and working through this with me.

ditto. :)

bhouston · 2017-05-05T19:33:37Z

I am confused, I always thought that displacement is sort of the very last thing in the vertex deformation pipeline -- after skinning, after morphing.

bhouston · 2017-05-05T19:35:30Z

The order should be based on the degree of transformation - largest first to smallest. Skinning is the largest scale transformation, then morphing (which if used with skinning should be minor, like facial expressions) and then displacement.
I believe that is the correct order and how everyone else does it.

WestLangley · 2017-05-05T19:45:02Z

@bhouston

(1) You are correct. Displacements last is best. The confusion here was we were trying to get the order right for all possible combinations. The combination that is problematic is if displacements and morph targets are specified, but morph normals are not. I think the consensus is to put displacements last and consider the problematic case a user error. Sorry for all the confusion here. I am glad this PR was filed, however.

(2) Right now, we have morphing before skinning. We could switch those in a separate PR.

jaxry · 2017-05-06T02:16:31Z

How does this look?

Currently depth_vert.glsl does not compile. No objectNormal is ever defined in that shader. How might we resolve this? We can't displace along a post-skin post-morph normal if it's never been computed.

WestLangley · 2017-05-06T03:15:23Z

Just edit depth_vert.glsl like so:

#include <common>
#include <uv_pars_vertex>
#include <displacementmap_pars_vertex>
#include <morphtarget_pars_vertex>
#include <skinning_pars_vertex>
#include <logdepthbuf_pars_vertex>
#include <clipping_planes_pars_vertex>

void main() {

	#include <uv_vertex>

	#include <skinbase_vertex>

	#include <beginnormal_vertex>
	#include <morphnormal_vertex>
	#include <skinbase_vertex>
	#include <skinnormal_vertex>
	#include <defaultnormal_vertex>

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

}

jaxry · 2017-05-06T03:44:29Z

src/renderers/shaders/ShaderLib/depth_vert.glsl

@@ -12,10 +12,18 @@ void main() {

 	#include <skinbase_vertex>

+	#ifdef USE_DISPLACEMENTMAP


Normals are only used for displacement. Prevent unnecessary computation of normals if displacement is not used. Is this okay or is this decision needlessly complex?

This is good.

If you want to, you can squash the commits into one clean one. It is not required, though.

Nice job. Thanks.

WestLangley · 2017-05-07T23:32:47Z

The order should be based on the degree of transformation - largest first to smallest. Skinning is the largest scale transformation, then morphing (which if used with skinning should be minor, like facial expressions) and then displacement.

I thought so too, but after studying our implementation more carefully, I see that our morphing calculation must precede our skinning calculation. Here are some course slides that develop this idea further. Our current implementation in that regard seems correct to me.

@mrdoob To summarize, this PR makes a few minor code improvements, and one significant change -- applying displacements after morphing and skinning, rather than before.

This change allows the displacement to be in the direction of the morph/skinned normal, which is the correct thing to do. Shadows for morph/skinned models utilizing displacement maps should also be correct.

In my opinion, this change is a good one and can be merged.

Many thanks @jaxry and @bhouston.

mrdoob · 2017-06-19T19:33:37Z

Oops, seems like I forgot about this PR. Glad that it's still mergeable 😅

mrdoob · 2017-06-19T19:33:45Z

Thanks guys!

... as in #11271.

mrdoob · 2017-06-20T01:30:02Z

I suspect NodeMaterial, NormalNode, OutlineEffect and WebGLDeferredRenderer are affected by this PR.

/cc @takahirox @sunag

takahirox · 2017-06-20T04:30:00Z

OK, let me check later.
BTW, in general I don't think we should merge any PRs which can break some modules right before releasing a new revision (unless they're super important).
How about reverting once, and then merging again after releasing r86?

mrdoob · 2017-06-20T04:59:48Z

BTW, in general I don't think we should merge any PRs which can break some modules right before releasing a new revision (unless they're super important).

I agree. I should have merged this one after releasing. But I was still working on WebVR efficiency (which I think I've solved) so there was still more time.

Sorry about that.

mrdoob · 2017-06-20T06:08:42Z

@takahirox

I think vertexShaderChunk2 can now be simplified to this:

var vertexShaderChunk2 = [

	"gl_Position = calculateOutline( gl_Position, transformedNormal, vec4( transformed, 1.0 ) );",

	"#include <fog_vertex>"

].join( "\n" );

mrdoob · 2017-06-20T19:21:21Z

Ignore my previous comment. I think this is the right fix:

var vertexShaderChunk2 = [

	"#if ! defined( LAMBERT ) && ! defined( PHONG ) && ! defined( TOON ) && ! defined( PHYSICAL )",

	"	#ifndef USE_ENVMAP",
	"		vec3 transformedNormal = normalize( normal );",

	"		#ifdef FLIP_SIDED",
	"			transformedNormal = -transformedNormal;",
	"		#endif",

	"	#endif",

	"#endif",

	"#ifdef DECLARE_TRANSFORMED",
	"	vec3 transformed = vec3( position );",
	"#endif",

	"gl_Position = calculateOutline( gl_Position, transformedNormal, vec4( transformed, 1.0 ) );",

	"#include <fog_vertex>"

].join( "\n" );

I think OutlineEffect would benefit a lot of #11475.

…l changes. See #11271.

…rtex.glsl changes. See #11271.

takahirox · 2017-06-21T14:15:44Z

I'll check the detail later (maybe tomorrow) but OutlineEffect seems like still broken.
(Sorry I've been busy lately!)

mrdoob · 2017-06-21T14:44:42Z

I'll check the detail later (maybe tomorrow) but OutlineEffect seems like still broken.

How so? All the examples seem to look correct?

takahirox · 2017-06-21T14:58:11Z

In webgl_loader_mmd example.

Rotate and see from back.

mrdoob · 2017-06-21T15:11:20Z

Oh! I'll take a look.

takahirox · 2017-06-22T02:28:47Z

Perhaps, this'll fix...

var vertexShaderChunk2 = [

	"#if ! defined( LAMBERT ) && ! defined( PHONG ) && ! defined( TOON ) && ! defined( PHYSICAL )",
	"	#ifndef USE_ENVMAP",
	"		vec3 objectNormal = normalize( normal );",
	"	#endif",
	"#endif",

	"#ifdef DECLARE_TRANSFORMED",
	"	vec3 transformed = vec3( position );",
	"#endif",

	"#ifdef FLIP_SIDED",
	"	objectNormal = -objectNormal;",
	"#endif",

	"gl_Position = calculateOutline( gl_Position, objectNormal, vec4( transformed, 1.0 ) );",

	"#include <fog_vertex>"

].join( "\n" );

I'll make a PR later.

I think OutlineEffect would benefit a lot of #11475.

Yup, agreed. I'll try after fixing the issue.

Fix OutlineEffect broken by #11271

Displacement Map: add morph normal support

4768ca0

jaxry commented May 2, 2017

View reviewed changes

add missing newline

78b236b

Displacement Map: displace before morphing/skinning

bdd12d1

defaultnormal_vertex: terser code

68d46fc

jaxry force-pushed the morph-target-displacement-map branch from 572bd54 to 68d46fc Compare May 6, 2017 02:05

Displacement Map: displace after morphing/skinning

13b6a37

displacementmap_vertex: Add spacing

60729cd

MeshDepthMaterial: compute normals for displacement

21d6826

jaxry commented May 6, 2017

View reviewed changes

mrdoob merged commit bd807bd into mrdoob:dev Jun 19, 2017

WestLangley added a commit that referenced this pull request Jun 19, 2017

Move displacement map chunk

381dadc

... as in #11271.

WestLangley mentioned this pull request Jun 19, 2017

GPGPU Water Example: Move displacement map chunk #11563

Merged

mrdoob added a commit that referenced this pull request Jun 20, 2017

OutlineEffect: Fixed breakage due to normals defaultnormal_vertex.gls…

3c4b4b8

…l changes. See #11271.

mrdoob added a commit that referenced this pull request Jun 20, 2017

WebGLDeferredRenderer: Fixed breakage due to normals defaultnormal_ve…

b461683

…rtex.glsl changes. See #11271.

takahirox added a commit to takahirox/three.js that referenced this pull request Jun 22, 2017

Fix OutlineEffect broken by mrdoob#11271

11e45b8

takahirox mentioned this pull request Jun 22, 2017

Fix OutlineEffect broken by #11271 #11578

Merged

mrdoob added a commit that referenced this pull request Jun 22, 2017

Merge pull request #11578 from takahirox/FixOutlineEffect

e6c0d10

Fix OutlineEffect broken by #11271

mrdoob pushed a commit that referenced this pull request Jun 22, 2017

Fix OutlineEffect broken by #11271

b35244d

		@@ -1,7 +1,11 @@
		#ifdef FLIP_SIDED

		objectNormal = -objectNormal;

		@@ -12,10 +12,18 @@ void main() {

		#include <skinbase_vertex>

		#ifdef USE_DISPLACEMENTMAP

Displacement Map: Add morph normal support #11271

Displacement Map: Add morph normal support #11271

Conversation

jaxry commented May 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaxry May 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaxry May 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WestLangley commented May 3, 2017 • edited Loading

jaxry commented May 3, 2017 • edited Loading

bhouston commented May 3, 2017

WestLangley commented May 3, 2017

bhouston commented May 3, 2017

jaxry commented May 4, 2017 • edited Loading

jaxry commented May 4, 2017 • edited Loading

WestLangley commented May 4, 2017

jaxry commented May 5, 2017

WestLangley commented May 5, 2017

jaxry commented May 5, 2017

WestLangley commented May 5, 2017

jaxry commented May 5, 2017 • edited Loading

WestLangley commented May 5, 2017

bhouston commented May 5, 2017

bhouston commented May 5, 2017

WestLangley commented May 5, 2017 • edited Loading

jaxry commented May 6, 2017 • edited Loading

WestLangley commented May 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WestLangley commented May 7, 2017

mrdoob commented Jun 19, 2017

mrdoob commented Jun 19, 2017

mrdoob commented Jun 20, 2017

takahirox commented Jun 20, 2017 • edited Loading

mrdoob commented Jun 20, 2017 • edited Loading

mrdoob commented Jun 20, 2017

mrdoob commented Jun 20, 2017 • edited Loading

takahirox commented Jun 21, 2017

mrdoob commented Jun 21, 2017

takahirox commented Jun 21, 2017

mrdoob commented Jun 21, 2017

takahirox commented Jun 22, 2017

jaxry commented May 2, 2017 •

edited

Loading

jaxry May 2, 2017 •

edited

Loading

jaxry May 3, 2017 •

edited

Loading

WestLangley commented May 3, 2017 •

edited

Loading

jaxry commented May 3, 2017 •

edited

Loading

jaxry commented May 4, 2017 •

edited

Loading

jaxry commented May 4, 2017 •

edited

Loading

jaxry commented May 5, 2017 •

edited

Loading

WestLangley commented May 5, 2017 •

edited

Loading

jaxry commented May 6, 2017 •

edited

Loading

WestLangley commented May 6, 2017 •

edited

Loading

takahirox commented Jun 20, 2017 •

edited

Loading

mrdoob commented Jun 20, 2017 •

edited

Loading

mrdoob commented Jun 20, 2017 •

edited

Loading