Skip to content

Range: Only use the snapped value if it is different enough from the entered value#110176

Open
aaronfranke wants to merge 1 commit intogodotengine:masterfrom
aaronfranke:range-snap
Open

Range: Only use the snapped value if it is different enough from the entered value#110176
aaronfranke wants to merge 1 commit intogodotengine:masterfrom
aaronfranke:range-snap

Conversation

@aaronfranke
Copy link
Copy Markdown
Member

@aaronfranke aaronfranke commented Sep 1, 2025

Fixes #109838 (not a regression fix this time, this PR is actually an improvement over what Godot has ever had before)

@jan-kapoli commented (but deleted their comment?) asking why we are snapping at all. Well, the reason why is because this is the behavior of Range. However, that still raises a good point. If the value is already good, we shouldn't need to snap it. This PR checks if the value is approximately equal to the snapped value, and uses the original value. This now works correctly even for values and step sizes down to 1e-7:

Screenshot 2025-09-01 at 12 22 25 PM

Snapping will still occur if the value is different enough. For example, with a step size of 0.0000001, if you enter 0.000000101, this gets converted to 9.999999999994822e-08 instead of 0.0000001, but 0.0000001 gets preserved as the closest possible float to 0.0000001 (and therefore serialized as 1e-7).

The behavior in this PR may be slightly worse if you don't trust the user's input, allowing the value to be up to 1e-14 away from the snapped value rather than force-snapping to within 1e-15 or 1e-16 of the best value. However, I think this is a very minor problem (only slightly worse than the imprecision double already provides), and has the benefit that if you do trust the user's input and expect that the value the user entered is probably already good, this helps.

@jan-kapoli
Copy link
Copy Markdown

Sorry, I deleted my question because I felt it was not the place for general questions about Godot's behaviour. I just didn't want to spam the issue. 😅

@Repiteo Repiteo added the cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release label Sep 3, 2025
@Repiteo Repiteo modified the milestones: 4.5, 4.6 Sep 3, 2025
Comment thread scene/gui/range.cpp Outdated
@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Sep 19, 2025

How does it fix the issue? I see the same behavior when I follow the steps.

@aaronfranke aaronfranke removed the cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release label Sep 21, 2025
@akien-mga
Copy link
Copy Markdown
Member

How does it fix the issue? I see the same behavior when I follow the steps.

Poke @aaronfranke

@aaronfranke
Copy link
Copy Markdown
Member Author

aaronfranke commented Nov 27, 2025

@KoBeWi was correct, it wasn't working right in all cases, and the problem was due to the relative tolerance as he pointed out.

Since we are using fixed-precision (R128), the tolerance would go lower than where the fixed-precision float "bottoms out". Changing this to a fixed value instead of a relative one seems to work in all cases now.

MRP: Bug.zip

Video of the fix compared to 4.5.1-stable:

oh_snap.mp4

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Apr 3, 2026

Tested it with the MRP again and it still doesn't fix the original one. idk if the issue is expected to be fixable though.
The new MRP behaves the same with and without this PR, I guess except the serialization changes seen in your video.

Can't say how much useful and expected is this fix, as I'm not familiar with precision-related problems.

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.

exported float is rounded down

5 participants