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

Handle the smoothstep degenerate case of empty range #93149

Merged

Conversation

Malcolmnixon
Copy link
Contributor

@Malcolmnixon Malcolmnixon commented Jun 14, 2024

This PR fixes issue #68128 by handling the empty-range case as a binary-divisor between 0.0 and 1.0.
It also updates the documentation to describe positive and negative ranges.

Note that it depends on godotengine/godot-docs#9489 to go in first so the graph URL links work.

Bugsquad edit:

core/math/math_funcs.h Outdated Show resolved Hide resolved
@Malcolmnixon
Copy link
Contributor Author

@kleonc I copied your fix for the double implementation over to the float and squashed the commits in preparation for merge.

Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

LGTM!

But since I've proposed changing it to the current version, another opinion / approval would be good.

+Remainder for before merging:

Note that it depends on godotengine/godot-docs#9489 to go in first so the graph URL links work.

core/math/math_funcs.h Outdated Show resolved Hide resolved
@Malcolmnixon Malcolmnixon force-pushed the smoothstep-degenerate-case branch 2 times, most recently from 6e713ca to e9213c5 Compare June 22, 2024 20:01
@Malcolmnixon
Copy link
Contributor Author

I updated the PR to match the updated graph in the godotengine/godot-docs#9489 pull request. I would request people review that PR (which just adds the image) in order to get this dependent PR merged.

@Malcolmnixon
Copy link
Contributor Author

The godot-docs image was converted from png to webp to align with the guidelines. As such this PR has been updated to refer to the webp image.

@clayjohn clayjohn modified the milestones: 4.x, 4.4 Jul 24, 2024
@clayjohn
Copy link
Member

Looks good. Just needs a rebase before merging. Feel free to do that after 4.3 releases so you don't end up having to rebase again (since this needs to wait for 4.4 before merging)

It also updates the documentation to describe positive and negative ranges.

Co-Authored-By: Hugo Locurcio <hugo.locurcio@hugo.pro>
Co-Authored-By: kleonc <9283098+kleonc@users.noreply.github.com>
@Malcolmnixon
Copy link
Contributor Author

Looks good. Just needs a rebase before merging. Feel free to do that after 4.3 releases so you don't end up having to rebase again (since this needs to wait for 4.4 before merging)

Rebased

@akien-mga akien-mga merged commit 27f3dd8 into godotengine:master Aug 16, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

smoothstep returning out of range values for 0 length steps
7 participants