Add triangle wave node

[Cinifreak]

This node creates values from 0.0 to "Scale" based on input value.
Example: using ping-pong node with scale=0.18 over "V" component.

[jstone-lucasfilm]

@Cinifreak This sounds like a good proposal for a new node, though I wonder if trianglewave might be a clearer name than pingpong, extending naturally to squarewave and sawtoothwave in the future.

I don't have a strong opinion on this, but here are two references that I found for the trianglewave alternative:

https://dev.epicgames.com/community/snippets/o4A/unreal-engine-triangle-wave-material-function
https://docs.unity3d.com/Packages/com.unity.shadergraph@12.1/manual/Triangle-Wave-Node.html

[dbsmythe]

Should this node be in the "procedural2d" category, rather than "math", as it seems to mainly be an image generator as opposed to doing a math-like operation on an input value?

[Cinifreak]

@dbsmythe Triangle wave is defined as a math node in other softwares. that's why i put it there. It's a mathematical operation after all.

[Cinifreak]

@jstone-lucasfilm Should i inverse the amplitude value?
it gets more frequency with a smaller value, I think it should be inverted? or let it be as it is?

[jstone-lucasfilm]

@Cinifreak Ah, I see what you mean now. The amplitude input affects both the amplitude and the frequency of the wave at the same time.

My sense is that we should make these two independent inputs that the artist can control, though this requires some modifications to the graph implementation. In the future, this would allow an artist to swap between different wave nodes (e.g. triangle, sawtooth, square), keeping the same amplitude and frequency inputs, and expect consistent results across all of them.

Does that sound like a reasonable approach to you as well?

[jstone-lucasfilm]

@Cinifreak Let us know how the suggestions above sound to you, and we're happy to assist with the generalization of this node to support both amplitude and frequency.

[jstone-lucasfilm]

This latest version looks excellent, thanks @Cinifreak!

Before merging this change, I wanted to check with you and @crydalch about the best name for "frequency" node inputs in MaterialX.

In the Unified Noise nodes that @crydalch recently contributed, he used the name freq for frequency inputs, with the UI names stating the capitalized word Frequency:

MaterialX/libraries/stdlib/stdlib_defs.mtlx

Line 1001 in cdd53d2

<input name="freq" type="vector2" uiname="Frequency" uifolder="Common" value="1, 1" doc="Adjusts the noise frequency, with higher values producing smaller noise shapes. Default is (1,1)." />

Should we follow this same convention for the new trianglewave node as well?

[Cinifreak]

Yes it would be better to follow @crydalch naming to unify it across all nodes. Just confirm that and i will change it for this PR and consider this for any upcoming nodes PR in the future!

[jstone-lucasfilm]

Sounds good, thanks @Cinifreak, and let's move forward with the convention for frequency inputs that @crydalch has set.

[jstone-lucasfilm]

@meshula @Cinifreak Maybe we're overthinking this, and the best answer is the simplest? Looking at other wave nodes in material graph editors, it looks like the most popular interface is to provide no controls at all, and to rely upon the artist to add nodes when they need additional scale or bias features.

As one example, here is the presentation of wave nodes in NodeToy, where all wave nodes have the same minimal interface as a sine wave:

[jstone-lucasfilm]

Following that approach, what if we were simply to remove both the height and period controls, making this a pure triangle wave generator?

I like @Cinifreak's choice to generate triangle waves with a period of one and output values in the [0-1] range, which seems consistent with triangle wave nodes in applications in the industry.

[Cinifreak]

From Blender, here is the detention of the node that behave I made it in the first iterations like this. just input and increment value to move to it. I think it shouldn't be complex or having something more than that!

[jstone-lucasfilm]

@Cinifreak Could we take this even further and remove the scale input as well? That would align with the trianglewave nodes in Unity and NodeToy, for example, and it would remove any potential confusion as to what scale refers to (e.g. height-only, height and frequency, etc).

[Cinifreak]

But how will the user control the increment value ?
let's say I have ramp [0, 1] and i want to use this node to increment from 0.2, So, i get 5 waves for that range. How will the user do that with this node with just the input value?

[jstone-lucasfilm]

@Cinifreak In Unity and NodeToy, for example, the artist just adds a multiply node after the trianglewave, and this scales the result in the same fashion that the scale input would have done. I think I can see why these applications made this decision, as it aligns trianglewave with other wave nodes like sine, and it makes sure that the artist only pays for the functionality they use in their graph, rather than having an extra multiply inside of trianglewave that they may not have needed.

[Cinifreak]

I think we have misunderstanding here. The scale here is not a multiplier. scale is not a good naming I know.
but here is the equation this node simulates: Triangle Wave(Value) = Scale - abs( (abs(Value) % (2*Scale)) - Scale)

[jstone-lucasfilm]

@Cinifreak In your original proposal, the scale input affected both the height and the period of the resulting waveform. To create exactly that same effect with a pure trianglewave node, I believe you would just scale both the input and the output of the node by constant values (where the input scale is one over the output scale). Scaling the input affects the period, and scaling the output affects the height.

Does that approach seem reasonable to you? The advantage of splitting this out into separate artist-created nodes is that we don't create extra behavior inside the node that artists may never use, but that they'll still pay for every time they place a trianglewave in their graph. Additionally, it seems to align with most triangle wave nodes that I see in DCC applications.

[jstone-lucasfilm]

@Cinifreak I've made the change described above, and let me know how this looks to you. This seems like the leanest possible implementation of trianglewave (a graph of just five nodes), which makes it a good candidate as a core node in MaterialX.

[Cinifreak]

Thank you for that effort. I will give it a test and let you know ASAP.

[meshula]

@jstone-lucasfilm I like the notion of treating "any" wave as one would treat a sine function, i.e. the input to a triangle wave spans one cycle over [0 .. 1] (as opposed to over 0 to 2*pi), and the output is ranged between 0..1. Given this simple formulation, it would make sense to me to add sawtooth (with a "rising" flag), and square (with a "duty cycle" parameter). Then you'd have the usual set of sin, saw, tri, and square. I can't decide if it's confusing or not that a sin completes a cycle over a circle, and the proposed new generators cycle over zero to one. I think it's probably not confusing, and 0...1 is definitely less confusing than wondering if sine takes radians or degrees.

(sawtooth rises, or falls, as noted earlier)
(duty cycle, 0.5 is the usual square wave, 0.1 is 1 10% of the time, 0.9 is 1 90% of the time)

[jstone-lucasfilm]

Completely agree, @meshula, and I think this proposal makes a great template for additional wave nodes in the future. In particular, squarewave seems like a valuable core node, and its presence would make certain existing nodes simpler to implement (e.g. checkerboard), not to mention future nodes.

For sawtoothwave, this might end up being identical to our existing modulo node, which by default returns the fractional component of the input scalar. But I'm not against having sawtoothwave as an alias for this node if it's clearer from an artistic perspective.

[dbsmythe]

Rather than add a sawtoothwave function that's basically the same as modulo, I'd vote for leaving it out and then instead creating a unifiedwave (or something- naming is hard) that would be a higher-level wrapper for all the xxwave nodes plus "sine" (similar in idea to the unifiednoise2d/3d nodes). And the unifiedwave node could include things like period and height and other niceties. (I do agree that adding a squaretoothwave node with a dutycycle input would be good)

[Cinifreak]

I totally agree with @dbsmythe.
About the node right now, @jstone-lucasfilm it's working now from zero to one and repeats on integer boundaries as you described it. But it didn't mean that because now it's behaving like sine. My contribution was a node that sets a value from zero to an increment value and repeats to the largest value in the input.
As i said before in a previous example, If I have a ramp [0, 1] and want to get a wave every 0.2 I will connect this ramp to node's input and set the incremental value (I named it in the first iteration "scale") to 0.2 and the output should be waves starts from zero to 0.2 and back down to Zero untill it finishs the ramp range. (That's why I called it ping-pong node before)

I think what we have now is another behaviour that could be helpful if we have all the waves set in one node not just this triangle wave as a single node.

[jstone-lucasfilm]

@Cinifreak I'm not sure I fully understand what you're suggesting in the post above.

I like @dbsmythe's proposal to omit sawtoothwave since modulo already exists, but wouldn't trianglewave still be a useful node to support? Further in the future, we can extend this to a unifiedwave that combines all of the possible wave types, but I would imagine we'd still want individual wave generators to be available, as they are in environments such as Unity and NodeToy.

Let me know if the current pull request looks good to you, or if there are additional changes that should be made before we merge!

[Cinifreak]

I tested the final edit. I still seeing that we have to let user control how many periods he needs at least. I pushed another update with just two inputs in and period, If you want to change the parameter name feel free to do that. Also, now every period will remain consistent from zero to one and if users need to change the height they can multiply it as @jstone-lucasfilm said before. We don't have to set a multiplier inside the node. Give it a second look and let me know what do you think about that?

[jstone-lucasfilm]

@Cinifreak Couldn't the artist just multiply the input by 1 / period to get this same effect?

Although I'm open to adding useful controls, I'm noticing that the trianglewave nodes in Unity and NodeToy don't provide a period input (or any other input beyond in), so perhaps it's not something that artists found they needed in a core node like trianglewave?

As @dbsmythe notes above, we can always design very high-level wave nodes later on, providing a very rich set of controls and options, but that doesn't necessarily mean that our underlying core nodes need to provide them as well.

[Cinifreak]

I tested to do 1/period to get the same effect as this screenshot but it didn't work. I could miss something.
But, we can go with your edit after all, I agree with @dbsmythe and your suggestion we can set the core nodes first then in the future we can build another complex controls over it. I was just recommending period from an artist perspective because I'm already using it in Herringbone tilling.

[jstone-lucasfilm]

@Cinifreak If you want to get the effect above, then you should just multiply the input by five, effectively making the period 1/5. I'll go back to the earlier implementation for now, and I'll see if I can post a screenshot using the technique described here. Thanks for all of your work on this node, and I think it should be very useful!

[Cinifreak]

Oh, Okay I understand the technique now. Thank you for you patience. Let's Merge the earlier implementation. Thanks again!

[jstone-lucasfilm]

This looks ready to go, thanks @Cinifreak!