Cycles: change material displacement to vector, add displacement node.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Jan 21 2018, 12:00 AM.

Details

Summary

As discussed in D1734#38749, this will make vector displacement fit in,
and is also more convenient / user friendly than using a vector math node
to tweak displacement scale.

It's backwards compatible by inserting this node automatically in existing
files, and moving the hardcoded 0.1 scaling into the distance socket.

There's few issue still:

  • Perhaps the name of the node should be changed to something like "Normal Displacement" or "Scalar Displacement"?
  • Changing displacement socket types did not work correct, and was fixed in rB79563d2a33ef. However releases before 2.79a will hit an assert when rendering files saved with this change. It will not crash or cause any issues because we don't use the value of this socket, but it's still weak. Is this acceptable?
  • The original reason for the 0.1 scale was that bump and displacement looked very different with the same values. That is still case, and it would be good if we could figure out why and address the real problem, ideally as part of this change.

Diff Detail

Repository
rB Blender

Fix bump/displacement scale difference. Probably changing the node name is not so helpful, and I don't think we can do much better regarding forward compatibility, so no further changes planned from me here.

Looks mostly good, would be great to finally have this done.

It might be good for the node to be named "Normal Displacement" or "Scalar Displacement", as eventually we may add a "Vector Displacement" node. Can we also rename the sockets? "Distance" -> "Scale" at least? (I find the words "distance" and "height" to be confusing in this context)

That and the inline comments are the only issues I can see with this. Below are some thoughts of things to work out at a later time:

Theres a few object / world transforms that are weird (from before this patch). All shader values are in world space except for displacement amount. This sort of makes sense for baked vector displacement maps, but when I talked to artists good reasons were given to have it done in world space. Can we make that change later?

For a vector displacement map node it would be good to have layering, by tracking differentials thru the graph to get displaced normal etc, to add another displacement on top of. I think the node in this patch will work fine in such a system without any breaking changes.

intern/cycles/render/graph.cpp
865 ↗(On Diff #9863)

Doing this before the copy below means the original graph is modified, which breaks displacement type "both". Either this has to happen after the copy, or some check will need to be done.

Also not sure how bump will work for "both", but I think thats probably fine to leave as a todo for now (as long as we don't break scalar displacement.)

Thanks for the review.

It might be good for the node to be named "Normal Displacement" or "Scalar Displacement", as eventually we may add a "Vector Displacement" node. Can we also rename the sockets? "Distance" -> "Scale" at least? (I find the words "distance" and "height" to be confusing in this context)

The problem is that I don't really like either name. "Normal Displacement" sounds a bit too much like normal mapping, and "Scalar" is a technical term that we don't use anywhere else in Blender. I think users will understand if there are "Displacement" and "Vector Displacement" nodes in the list, though maybe there is another good name I can't think of.

The socket names are consistent with the Bump node. I don't mind naming it Scale but then maybe both should be changed.

Theres a few object / world transforms that are weird (from before this patch). All shader values are in world space except for displacement amount. This sort of makes sense for baked vector displacement maps, but when I talked to artists good reasons were given to have it done in world space. Can we make that change later?

We can add an object space / world space enum to the Displacement node. World space does not work with instancing though, so I'm not sure it would be a good default, but I can see how it can be useful.

For a vector displacement map node it would be good to have layering, by tracking differentials thru the graph to get displaced normal etc, to add another displacement on top of. I think the node in this patch will work fine in such a system without any breaking changes.

Indeed.

The problem is that I don't really like either name. "Normal Displacement" sounds a bit too much like normal mapping, and "Scalar" is a technical term that we don't use anywhere else in Blender. I think users will understand if there are "Displacement" and "Vector Displacement" nodes in the list, though maybe there is another good name I can't think of.

I can't think of anything better, so I guess its fine to leave as "Displacement".

The socket names are consistent with the Bump node. I don't mind naming it Scale but then maybe both should be changed.

It always confused me on the bump node as well. I'll leave it up to you.

We can add an object space / world space enum to the Displacement node. World space does not work with instancing though, so I'm not sure it would be a good default, but I can see how it can be useful.

Ah, good point about instancing, I had forgotten about that case.

I don't think I have anything else to add, so if the "both" case is fixed then this looks fine to me.

This revision is now accepted and ready to land.Tue, Jan 23, 10:24 AM
  • Rename Distance to Scale, we can change the Bump node later.
  • Fix Both displacement type.
This revision was automatically updated to reflect the committed changes.
This revision is now accepted and ready to land.Tue, Jan 23, 11:34 AM

This shouldn't have been closed..

This revision was automatically updated to reflect the committed changes.