Page MenuHome

Shading: Add Interpolation modes to Map Range node
ClosedPublic

Authored by Charlie Jolly (charlie) on Sep 17 2019, 6:34 PM.
Tags
None
Tokens
"Love" token, awarded by EAW."Like" token, awarded by filibis."Love" token, awarded by astrand130."Love" token, awarded by mistajuliax."Love" token, awarded by alterdings."Love" token, awarded by sonicdee1981."Love" token, awarded by Bariboy."Love" token, awarded by avpiedra."Love" token, awarded by michaelknubben."Like" token, awarded by Fracture128."Love" token, awarded by Lumpengnom."Love" token, awarded by moony."Love" token, awarded by Ristovski."Love" token, awarded by intracube."Like" token, awarded by knightknight."Love" token, awarded by thiagodesul."Love" token, awarded by viadvena."100" token, awarded by valera."Like" token, awarded by JayE01."Love" token, awarded by astroblitz."Love" token, awarded by 100drips."Burninate" token, awarded by CAEL."Mountain of Wealth" token, awarded by TheAngerSpecialist."Love" token, awarded by looch."Love" token, awarded by brilliant_ape."Love" token, awarded by linolafett."Like" token, awarded by rjg."Party Time" token, awarded by mazigh."Love" token, awarded by klutz."Love" token, awarded by misiewiczradoslaw."Love" token, awarded by dbystedt."Burninate" token, awarded by zanqdo."Love" token, awarded by R2792."Love" token, awarded by wuzzle."Love" token, awarded by dpdp."Love" token, awarded by MizManFryingP."Like" token, awarded by nacioss."Love" token, awarded by abdoubouam."Love" token, awarded by phalaris."Love" token, awarded by Linkun."Love" token, awarded by jskksj."Love" token, awarded by Thane5."Love" token, awarded by quollism."Love" token, awarded by shakesoda."Love" token, awarded by Draise."Love" token, awarded by Zino."Love" token, awarded by julperado."Love" token, awarded by andruxa696."Love" token, awarded by ChrisGraz."Love" token, awarded by LapisSea."Love" token, awarded by carlosmu."Love" token, awarded by Schamph."Love" token, awarded by fin.eskimo."Love" token, awarded by ogierm."Orange Medal" token, awarded by Alrob."Love" token, awarded by Taros."Love" token, awarded by mantissa."Love" token, awarded by MetinSeven."Love" token, awarded by Kramon."Love" token, awarded by CGKrisu."Love" token, awarded by zeffii."Love" token, awarded by BlenderBrit."Love" token, awarded by Samirosman."Love" token, awarded by davidmcsween."Love" token, awarded by simonthommes."Love" token, awarded by tilapiatsu."Love" token, awarded by monio.

Details

Summary

This patch adds Interpolation modes to Map Range node.

Modes: Linear interpolation (default), stepped linear, smoothstep and smootherstep.

This also includes an additional option for the Clamp node to switch between Min Max (default) and Range mode.

This was needed to allow clamping when To Max is less than To Min.

Notes:
The Map Range node effectively carries out two operations, first an interpolation is carried out on the From values and then mapped to the To values. The Result value is then optionally clamped to the output range. Clamp is not required for Smoothstep or Smootherstep as these automatically clamp. In Stepped Linear mode, the Steps value controls how many steps are used in the interpolation. This is based on the [0-1] range. A value of 1 will give two values, imagine this is like steps on a staircase. Inverse Linear and Linear are closely related and flipping the values of the two ranges effectively swaps between them.

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Add node label support.

Jacques Lucke (JacquesLucke) requested changes to this revision.Nov 28 2019, 1:23 PM

In find the naming a bit confusing. Linear, Smooth and Stepped are fine.

Why is "step" in the names "Smoothstep" and "Smootherstep"? Seems to be completely unrelated to the "stepped" mode.
Also the semantic of the Factor/Start/End inputs is very different between the two groups of modes (linear, smooth, stepped vs. smoothstep, smootherstep, inverse linear). That could be quite confusing when people just want to try different interpolation modes.

When using Smoothstep, Smootherstep or Inverse Linear, the node behaves like a map range node followed by an interpolate node.

I'd suggest that these last three modes are removed from the interpolate node. Then another Smoother mode could be added if necessary. Then this patch could be merged.
Afterwards, we can think about adding these interpolation modes to the Map Range node. This is what we did in Animation Nodes.

These two node setups are identical:

The map range node in AN allows you to use an interpolation when mapping from the inputs to outputs:

intern/cycles/kernel/shaders/node_interpolate.osl
19 ↗(On Diff #18998)

There is already a safe_divide in node_math.osl and node_vector_math.osl. Looks like these functions should be factored out into a common header.

33 ↗(On Diff #18998)

Code style, use only one white space after =. Same below.

34 ↗(On Diff #18998)

Please configure your editor so that it removes trailing whitespaces automatically.

40 ↗(On Diff #18998)

Code style, no white space next after ( and before ).

50 ↗(On Diff #18998)

Code style, white space after ,.

intern/cycles/kernel/svm/svm_interpolate.h
21 ↗(On Diff #18998)

why "edge"?

source/blender/editors/gpencil/gpencil_primitive.c
1410 ↗(On Diff #18998)

Should probably not be part of this patch.

This revision now requires changes to proceed.Nov 28 2019, 1:23 PM
Charlie Jolly (charlie) marked 7 inline comments as done.

Address comments and rename 'smooth' to 'linear smooth' and 'stepped' to 'linear stepped'.

intern/cycles/kernel/shaders/node_interpolate.osl
19 ↗(On Diff #18998)

I checked usage in other OSL files and it's in node_math.osl too. There might be other functions so suggest refactor be done in a separate patch.

intern/cycles/kernel/svm/svm_interpolate.h
21 ↗(On Diff #18998)

Most implementations of smoothstep function refer to the variables as edge0 and edge1. These are the two values that the interpolation uses to set the step edge.

source/blender/editors/gpencil/gpencil_primitive.c
1410 ↗(On Diff #18998)

Ooops!

I've renamed Stepped to Linear Stepped and Smooth to Linear Smooth. This avoids confusion with the Smoothstep function.

As a point of reference, Unity groups these functions together in their documentation which is from where I based this node on.

https://docs.unity3d.com/Packages/com.unity.shadergraph@6.9/manual/Smoothstep-Node.html

@Jacques Lucke (JacquesLucke) , as a shading artist I find you suggestions quite weird and worrisome.

I agree that packing all those modes in single node isn't elegant but solution you proposed practically removes second most useful functionality of this patch - smoothstep.

Why is "step" in the names "Smoothstep" (...)?

It produce step between two values and adds smooth interpolation between them. But more importantly, this is proper name of extremely common function used in computer graphics. While smoothstep is complex function and do many things at once (remapping, clamping and Hermite interpolation) it's so popular that it's built-in function in all shading languages and pretty much all nodal shading tools. Shader artist outside of blender-land use smoothstep very often, in all kinds of shaders and for various purposes.
I did an experiment. I opened few pages of latest shaders on shadertoy.com and counted how often people used smoothstep. Out of 60 shaders, 32 used this functions! There were +71 smoothstep function calls total.

Here is the problem.
You suggest to obscure this extremely common function behind some not-obvious node setup.
We should implement smoothstep as a single node. This is what most of shader artists want and expect.


When using Smoothstep (...) the node behaves like a map range node followed by an interpolate node.

I can guarantee you that once we provide proper smoothstep node it will be used more often than Map Range. Just like anywhere outside blender.
Mind that "smooth" mode is @Charlie Jolly (charlie)'s invention, something not found in shading languages, rarely present in other tools. Nice to have but this isn't replacement for actual smoothstep, it's something extra built with it. We should focus on basics first.

Smootherstep was invented by Ken Perlin to combat small discontinuities on tangent points in his Perlin Noise. AFAIK other nodal systems don't have this function but I think we should include it for artifact-free displacement and bumpmaps.

Also the semantic of the Factor/Start/End inputs is very different between the (...) linear (...) inverse linear.

I don't understand this part. Those are opposite functions. Inverse linear interpolation is defined through its relationship with linear interpolation, exactly the same like Division and Multiplication.

That could be quite confusing when people just want to try different interpolation modes.

Seems like you think of those similarly to MixRGB blending modes. Try to think of it like Math modes.

My suggestion.

We split all those modes into two separate nodes.

  • Interpolation node with Linear, Inverse Linear, Smooth and Stepped modes.
  • Smoothstep node with Smoothstep and Smootherstep modes.

i would listen really to what monio says.. he really knows what he is talking about.

I agree...
Not sure about "smootherstep" and if it can have a more accurate name but "smoothstep" is pretty much commonly accepted as a standardized name for this particular operation. I don't think it should be broken up into separate nodes however. Seeing as though we already cram all the vector and float math operations into a single node already.

I can see that smoothstep is very useful and yes, maybe more useful than the others. I did not mean to discard the idea of having a single node that does this operation. The two-node-setup I showed was just to show how the operation works internally.

My main concern is that parameters for smoothstep have a very different meaning than the parameters for e.g. smooth. For smoothstep the two last inputs indicate the input range, while for smooth they indicate the output range. I think that can be confusing when one just wants to try different interpolation modes. One could also argue that the last three modes should stay in the Interpolate Node and the others should go somewhere else etc. I don't know.

We split all those modes into two separate nodes.

I agree. I'd just put inverse linear in the second node as well because it does the same kind of operation, just with a different interpolation.

Another solution would be to just put all of these modes into an enum in the map range node. This node has a very clearly defined input and output range. That makes switching between the different modes easier. The only issue might be the stepped mode, because it needs the additional input.

I think splitting the node into two as suggested by @Bartosz Moniewski (monio) is best and makes it less ambiguous. This way, the commonly used smoothstep has it's own node. I think map range node is well defined already and it's already used for the compositor too. I'll also remove the smooth mode as this can be recreated using a smoothstep node when they are split.

@Jacques Lucke (JacquesLucke) since they are related can I create one patch with the two nodes or do I need to create two separate patches?

Thanks to everyone for feedback.

Charlie Jolly (charlie) updated this revision to Diff 20036.EditedDec 3 2019, 12:57 PM

Split into two nodes, Interpolate and Smoothstep.

Interpolate: Linear, Stepped and Inverse Linear modes. With clamp option for all modes.

Smoothstep: Smoothstep, Smootherstep and Linearstep modes. All output 0.0 to 1.0. No clamp option required.

Removed ambigous Smooth mode from Interpolate node. This can be recreated by using plugging a smoothstep into the interpolate factor.

Fix unused variable warning.

Remove node labels for Interpolate node. Clearer to just show Interpolate.

Charlie Jolly (charlie) retitled this revision from Shading: Interpolation node to Shading Nodes: Add Interpolation and Smoothstep nodes.Dec 3 2019, 2:06 PM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)
Brecht Van Lommel (brecht) requested changes to this revision.Dec 4 2019, 1:23 AM

I have to agree with @Jacques Lucke (JacquesLucke), I think all this functionality can be part of the map range node.

Shading nodes are not a low level programming API, and having nodes that can do a bunch of commonly useful things together helps to keep node setups manageable. Map range gives control over the input and output range, which is often useful. These are the guidelines we've followed so far in deciding how to create nodes, and I'm not in favor of changing direction here.

If discoverability is a concern, then really I think that should be addressed in another way. The same argument can be made to split tons of node like Geometry, Math, etc into individual nodes. What should be done instead is make it easy to search nodes also by their enum values, and perhaps add submenus in the node add menu to quickly add a node with a particular mode.

This revision now requires changes to proceed.Dec 4 2019, 1:23 AM

I have to agree with @Jacques Lucke (JacquesLucke), I think all this functionality can be part of the map range node.

@Brecht Van Lommel (brecht) @Jacques Lucke (JacquesLucke) After taking a looking at the Map Range code, I think combining these functions into the Map Range node is not such a good fit. The inputs To Min and To Max are misleading and do not work for these interpolation functions. The inputs From Min and From Max are not required for these functions. This would mean additional sockets, ambiguous variable names and introducing more complexity into the map range code to account for the different functions.

Shading nodes are not a low level programming API, and having nodes that can do a bunch of commonly useful things together helps to keep node setups manageable.

If there is a technical or code reason to combine these nodes into the Map Range node then I would find it more compelling, but I think @Bartosz Moniewski (monio) summed it up very well from a user perspective. Both of the proposed Interpolate and Smoothstep nodes have multiple modes which are bunching common functions together. Originally they were a single Interpolate node but on testing it was much better having them as separate nodes. This is also beneficial to users who use python to create shaders for addons etc.

If discoverability is a concern, then really I think that should be addressed in another way. The same argument can be made to split tons of node like Geometry, Math, etc into individual nodes. What should be done instead is make it easy to search nodes also by their enum values, and perhaps add submenus in the node add menu to quickly add a node with a particular mode.

Definitely agree on the discoverability and ability to search by enum values.

The inputs To Min and To Max are misleading and do not work for these interpolation functions.

They do work for any interpolation function, it's just a matter of mapping the output of the interpolation from 0..1 to that range. If they are left to the default 0..1, they would do nothing.

The inputs From Min and From Max are not required for these functions.

Similar to To Min and Max, this is just mapping some range to 0..1. Functions like smoothstep have it builtin as edge0 and edge1. But the same formula works for all types of interpolation and is convenient to bring values into a useful range without the need to add additional nodes.

This would mean additional sockets, ambiguous variable names and introducing more complexity into the map range code to account for the different functions.

Some sockets will depend on the mode, but we have mechanisms for that. The to/from min/max can always be available.

If there is a technical or code reason to combine these nodes into the Map Range node then I would find it more compelling, but I think @Bartosz Moniewski (monio) summed it up very well from a user perspective.

The only compelling argument I see is discoverability, which I don't think should be addressed this way. Other than that the functionality is closely related. For example Arnold has smoothstep as part of their Range node, it's not that unusual.

Both of the proposed Interpolate and Smoothstep nodes have multiple modes which are bunching common functions together. Originally they were a single Interpolate node but on testing it was much better having them as separate nodes. This is also beneficial to users who use python to create shaders for addons etc.

To me it absolutely makes sense as a user to switch between linear, stepped, smoothstep, and other interpolation functions. Putting smoothstep in a separate category because it happens to be a common function in shading languages is not a good reason, that is not what we are optimizing for.

I've rewritten the patch so that it incorporates the different interpolations into the map range node. This works by applying the interpolation option to the from values and then mapping to the to values.

Mostly this is working and providing the same options with the exception of Inverse Linear which can be created by flipping the values of the to and from ranges.

I have the following developer query which needs resolving before uploading the patch.

The clamp option currently doesn't take into account ranges where the toMax value is lower than the toMin value and breaks interpolation in these cases.

Top) Map Range node
Bottom) previous Interpolation node.

I can work around this by bringing the clamping code back into the node rather than linking the toMin and toMax values into a clamp node.

Is this fix ok to proceed with or is there an alternative suggestion on how this should be fixed?

Charlie Jolly (charlie) updated this revision to Diff 20103.EditedDec 5 2019, 6:38 PM

Refactor and add Interpolation modes to Map Range node

Linear interpolation, stepped linear, smoothstep and smootherstep.

This also includes an additional option for the Clamp node to switch between Min Max (default) and Range mode.
This was needed to allow clamping when To Max is less than To Min.

Notes:
The Map Range node effectively carries out two operations, first an interpolation is carried out on the From values and then mapped to the To values. The Result value is then optionally clamped to the output range. Clamp is not required for Smoothstep or Smootherstep as these automatically clamp. In Stepped Linear mode, the Steps value controls how many steps are used in the interpolation. This is based on the [0-1] range. A value of 1 will give two values, imagine this is like steps on a staircase. Inverse Linear and Linear are closely related and flipping the values of the two ranges effectively swaps between them.

Charlie Jolly (charlie) retitled this revision from Shading Nodes: Add Interpolation and Smoothstep nodes to Shading: Add Interpolation modes to Map Range node.Dec 5 2019, 7:06 PM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)
Brecht Van Lommel (brecht) requested changes to this revision.Dec 5 2019, 8:43 PM

Works great, just minor comments.

intern/cycles/kernel/shaders/node_map_range.osl
43

Shouldn't these two lines be swapped? To me it seems more consistent with other interpolation modes that way, to first bring the value into the range and then do the stepping.

That's also what I understood from your description as the intended behavior.

source/blender/editors/space_node/drawnode.c
253

drawnode.c:268:13: warning: unused function 'node_buts_map_range'

source/blender/gpu/shaders/material/gpu_shader_material_map_range.glsl
1–13

{ on new line.

This revision now requires changes to proceed.Dec 5 2019, 8:43 PM
Charlie Jolly (charlie) marked 3 inline comments as done.
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)

Address comments including a change to the step behaviour to relate to input range rather than [0-1].

intern/cycles/kernel/shaders/node_map_range.osl
43

Agree, this is probably better. Steps now relate to the from range rather than [0-1]. Changed.

source/blender/editors/space_node/drawnode.c
253

Oops.

I feel like the code could still be simplified.

As far as I can see, all modes have the same pattern:

1. Map input range from [from_min, to_min] to [0, 1].
2. Optionally clamp the computed value to [0, 1].
3. Apply the interpolation.
4. Map the range from [0, 1] to [to_min, to_max].

Especially step 1 and 4 are currently duplicated multiple times and are mixed into the interpolation formulas.

@Jacques Lucke (JacquesLucke)

The original code for map range was as follows:

to_min + (value - fromMin) / (fromMax - fromMin) * (to_max - to_min);

This is a combination of an inverse lerp and lerp.

The new code splits this so as follows:

factor = (value - fromMin) / (fromMax - fromMin); #inverse_lerp
result = to_min + factor * (to_max - to_min); #lerp

For the steps mode, factor is then quantized.

For smoothstep, the factor is calculated by a smoothstep interpolation instead of an inverse_lerp.

The clamp happens at the end.

1. Calculate factor based on interpolation mode
2. Perform lerp using previously calculated factor
3. Clamp output as an option

While it is possible to unify the code more, performance would be slightly worse for smoothstep. So I think it's ok as is.

This revision is now accepted and ready to land.Dec 7 2019, 10:17 AM