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

Added smoothstep built-in function #27231

Merged
merged 1 commit into from
Apr 8, 2019
Merged

Conversation

Chaosus
Copy link
Member

@Chaosus Chaosus commented Mar 19, 2019

Personally I misses hermite interpolation. So many algorithms are depends on it. It should be in the core, cuz its common in video game programming. Unity also have it in the builtin Math class https://docs.unity3d.com/ScriptReference/Mathf.SmoothStep.html
In Godot, currently it can be only used in shaders(smoothstep) or in Tween's. This PR exposes it to core - I named it smooth_lerp for easy search it within other lerp-functions and(I suppose) its just another type of linear interpolation.

image

I will add C# version later, (if this will be accepted)

@Chaosus Chaosus requested review from bojidar-bg, reduz, vnen and a team as code owners March 19, 2019 11:57
@Chaosus Chaosus added this to the 3.2 milestone Mar 19, 2019
@Chaosus Chaosus force-pushed the smoothstep branch 2 times, most recently from ee23fa5 to 7b15e52 Compare March 19, 2019 12:09
@bojidar-bg
Copy link
Contributor

Lerp is a contraction for linear_interpolation. Maybe it should be called smoothstep/smooth_step?

@Chaosus Chaosus changed the title Added smooth_lerp(SmoothStep) built-in function to GDScript Added smooth_step built-in function to GDScript Mar 19, 2019
@Chaosus
Copy link
Member Author

Chaosus commented Mar 19, 2019

Renamed to smooth_step

@Chaosus
Copy link
Member Author

Chaosus commented Mar 19, 2019

Added C# version too

@Chaosus Chaosus force-pushed the smoothstep branch 2 times, most recently from 5f0418c to f9641b5 Compare March 19, 2019 13:49
@Chaosus Chaosus changed the title Added smooth_step built-in function to GDScript Added smooth_step built-in function to GDScript/Mono Mar 19, 2019
@Chaosus Chaosus changed the title Added smooth_step built-in function to GDScript/Mono Added smooth_step built-in function Mar 19, 2019
@Calinou
Copy link
Member

Calinou commented Mar 19, 2019

How does lerp() combined with ease() compare with this?

Copy link
Contributor

@neikeq neikeq left a comment

Choose a reason for hiding this comment

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

C# change looks good to me but is wrongly formatted.

@Chaosus
Copy link
Member Author

Chaosus commented Mar 19, 2019

@Calinou explain more please ? I think its different functions, even if you somehow force the results to be equal its not as easy as single call

@bojidar-bg
Copy link
Contributor

@Calinou Tested with KmPlot, the closest ease + lerp combination should be:

lerp(a, b, ease(t, -1.6521))

Here are some plots of both, along with their derivatives. Notice that ease's derivative has a discontinuity at x=0.5, unlike smoothstep's derivative.

f: smoothstep g: ease (f - g) * 100
image image image

@Chaosus Chaosus force-pushed the smoothstep branch 2 times, most recently from 1f9c591 to 18491cc Compare March 21, 2019 18:21
@Chaosus
Copy link
Member Author

Chaosus commented Mar 21, 2019

Added is_equal_approx check to avoid division by zero

@Chaosus Chaosus dismissed Calinou’s stale review March 27, 2019 16:52

Its fixed !

@reduz
Copy link
Member

reduz commented Apr 4, 2019

nice addition, I wonder whether it should be "smooth_step" or "smoothstep" like in glsl..

@Chaosus Chaosus changed the title Added smooth_step built-in function Added smoothstep built-in function Apr 4, 2019
@Chaosus
Copy link
Member Author

Chaosus commented Apr 4, 2019

Changed to smoothstep (as also suggested by Calinou in IRC)

@Chaosus Chaosus force-pushed the smoothstep branch 3 times, most recently from ca5ddd2 to 87d5009 Compare April 5, 2019 05:17
@Chaosus Chaosus force-pushed the smoothstep branch 2 times, most recently from f83cf5d to 78ee9a6 Compare April 5, 2019 15:30
@@ -101,6 +101,7 @@ class VisualScriptBuiltinFunc : public VisualScriptNode {
VAR_TO_BYTES,
BYTES_TO_VAR,
COLORN,
MATH_SMOOTHSTEP,
Copy link
Member

Choose a reason for hiding this comment

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

Is it a problem if it's included further up before MATH_DECTIME, as for core/math?

Copy link
Member Author

Choose a reason for hiding this comment

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

by @vnen

I believe this breaks compatibility if put here: it should be put at the end of the list. You can see in the doc changes that all constants got new values, so if someone is using a function from below, suddenly the functions will be changed and it'll break the code.

@Chaosus Chaosus force-pushed the smoothstep branch 2 times, most recently from e3ffe18 to 078401b Compare April 7, 2019 10:52
@akien-mga akien-mga merged commit d211aff into godotengine:master Apr 8, 2019
@akien-mga
Copy link
Member

Thanks!

@Chaosus Chaosus deleted the smoothstep branch April 8, 2019 08:04
@hpvb
Copy link
Member

hpvb commented Apr 20, 2019

Cherry-picked for 3.1.1.

<argument index="2" name="weight" type="float">
</argument>
<description>
Returns a number smoothly interpolated between the [code]from[/code] and [code]to[/code], based on the [code]weight[/code]. Similar to [method lerp], but interpolates faster at the beginning and slower at the end.
Copy link
Member

Choose a reason for hiding this comment

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

@Chaosus, actually this implementation of smoothstep is similar to inverse_lerp and not to lerp so this description is misleading. Also another difference (besides interpolating faster at the beginning and slower at the end) is that smoothstep clamps result to range <0, 1>. So this description probably needs an update.
(no idea if that's proper place for this note)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants