fix: Lerp smoothing frame rate dependence and freeze at maximum interpolation time - #4130
fix: Lerp smoothing frame rate dependence and freeze at maximum interpolation time#4130NoelStephensUnity wants to merge 4 commits into
Conversation
Sorry, something went wrong.
…polation time The lerp smoothing pass used by the Lerp and SmoothDampening interpolation types applied a fixed factor of 1.0 minus the maximum interpolation time once per frame, with no delta time. The wall clock smoothing rate therefore scaled with the frame rate, so the same setting smoothed by different amounts on different hardware. The factor is now raised to the number of 60fps reference frames elapsed, which makes the rate a function of elapsed time. Results at 60fps are unchanged for every legal setting. Separately, a maximum interpolation time of 1.0 (the upper bound of the inspector range) produced a factor of exactly 0, so the interpolated value never advanced and the transform stopped moving entirely on all three axes. The retained portion is now clamped just below 1.0. This is an independent defect, as raising 1.0 to any power is still 1.0. The LegacyLerp path was already frame rate correct and is unchanged. The documentation on the lerp smoothing fields described the LegacyLerp formula for all interpolation types and has been corrected.
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop-2.0.0 #4130 +/- ##
=================================================
+ Coverage 73.86% 73.96% +0.09%
=================================================
Files 172 172
Lines 28099 28103 +4
=================================================
+ Hits 20756 20786 +30
+ Misses 7343 7317 -26
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes
|
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Harness Review
The time-based smoothing update is applied to both non-legacy interpolation paths, but the upper-bound handling changes valid high-end settings and the revised public documentation overstates the legacy mode's behaviour.
Reviewed commit 3c1dc9d
🤖 Helpful? 👍/👎
Sorry, something went wrong.
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| private float GetLerpSmoothTime(float deltaTime) | ||
| { | ||
| var retained = Mathf.Clamp(MaximumInterpolationTime, 0.0f, k_MaximumLerpSmoothRetention); |
There was a problem hiding this comment.
This caps every legal value in (0.99, 1.0], not just the freezing endpoint. For example, a Lerp/SmoothDampening transform configured with MaximumInterpolationTime = 0.995f now uses the same t as 0.99f (at 60 fps, 0.01 rather than 0.005), so it becomes twice as responsive and no longer honors the documented “higher is smoother” setting. Clamp to 1.0f first and special-case only the exact 1.0f endpoint (or otherwise preserve values below it).
🤖 Helpful? 👍/👎
Sorry, something went wrong.
| /// When enabled, the <see cref="BufferedLinearInterpolator{T}"/> will apply a final lerping pass where the "t" parameter is calculated by dividing the frame time divided by the <see cref="PositionMaxInterpolationTime"/>. | ||
| /// When enabled, the <see cref="BufferedLinearInterpolator{T}"/> will apply a final lerping pass towards | ||
| /// the interpolated result at a rate determined by <see cref="PositionMaxInterpolationTime"/>.<br /> | ||
| /// This is frame rate independent for all <see cref="InterpolationTypes"/>, but the same value will not |
There was a problem hiding this comment.
LegacyLerp is not frame-rate independent: its smoothing path still applies deltaTime / MaximumInterpolationTime each update. With a fixed target and MaximumInterpolationTime = 0.9, one second leaves (1 - 1/27)^30 of the delta at 30 fps versus (1 - 1/216)^240 at 240 fps. Restrict this statement to Lerp and SmoothDampening (the same incorrect statement is repeated for rotation and scale), unless legacy smoothing is also changed.
🤖 Helpful? 👍/👎
Sorry, something went wrong.
Purpose of this PR
Fixes two pre-existing defects in the lerp smoothing pass used by the Lerp and SmoothDampening interpolation types.
1. Smoothing was frame rate dependent. The factor was 1.0f - MaximumInterpolationTime applied once per frame with no delta time, so the wall clock smoothing rate scaled with the frame rate. It is now raised to the number of 60fps reference frames elapsed, making the rate a function of elapsed time.
2. A maximum interpolation time of 1.0 froze the transform. That is the upper bound of the inspector [Range], and it produced a factor of exactly 0, so the value never advanced. Affects all three axes and both interpolation types. The retained portion is now clamped just below 1.0.
Three things worth knowing that aren't obvious from the diff:
For scale: settle time at MaxInterpolationTime = 0.9 was 2.13s @30fps vs 0.275s @240FPS; it is now a flat ~1.02s at 30/60/120/240. At the 0.1 default the effect was invisible, which is why this went unnoticed.
The doc comments on the lerp smoothing fields described the LegacyLerp formula for all interpolation types and have been corrected.
Jira ticket
TODO: add ticket
Changelog
com.unity.netcode.gameobjects
Documentation
The remarks on PositionLerpSmoothing, RotationLerpSmoothing, ScaleLerpSmoothing, MaximumInterpolationTime, and the InterpolationTypes.Lerp / SmoothDampening enum entries all described the LegacyLerp formula regardless of the selected type. They now state that smoothing is frame rate independent and note that the same value does not produce the same result under LegacyLerp as under the other types.
Testing & QA (How your changes can be verified during release Playtest)
Both regression tests were written first and confirmed red against unfixed code, failing on the intended assertions rather than on timeouts:
After the fix, all 13 tests in InterpolatorTests pass, including the 10 pre-existing ones.
For release playtest, the highest value check is an object using Lerp or SmoothDampening with a maximum interpolation time above ~0.5, observed at a non-60fps frame rate — that is where the old and new behaviour differ. At the 0.1 default there should be no perceptible change at any frame rate.
Functional Testing
Manual testing :
Automated tests:
Does the change require QA team to:
Up-port
#4132 is the up-port.
Required. Will be up-ported to develop-3.x.x once this PR lands.
Backports
Not needed.