tweak(drawable): Decouple physics and fade timing from render update#2055
tweak(drawable): Decouple physics and fade timing from render update#2055bobtista wants to merge 21 commits into
Conversation
|
Can you show a video how it looks before and after? I'd recommend replicating to Generals as the very last thing you do for any PR. It's easier for the PR creator and reviewer(s). |
|
I remember I worked on this before and it was difficult to get perfectly right and then paused this. I still have the WIP branch. I would be surprised if this change fixed it with no problems at all. For easy test, take a GLA Buggy and compare its physics with Retail at different FPS. If it is not matching, then there is work left to do. |
Oh that's right, I remember watching this, there's some buggy jank addressed is in episode 0844. I was just trying to apply changes similar to your decouple stealth fade one for remaining frame based logic, hadn't started testing yet. I'll convert to draft and assume there's more to do |
Can you push the latest to your WIP branch? |
2751da9 to
2bb1cb5
Compare
|
When I increase render FPS and update physics every render, the buggy doesn't wabble or wheelie as much. At higher render FPS we're getting effectively like a higher resolution of the damped spring math, so less overshoots, less wobble, but same total force is applied. Claude says it's called the Euler integration of a damped spring. I think it makes sense to only calculate it at logic frames, and we interpolate so it's smoother visually. Otherwise we're messing with the spring math to approximate the overshoots from before, and it's purely visual right? Anyway - I just tested, and it looks right to me. Note the other changes in this PR don't have this overshoot feedback kind of issue, so they all should work with calculations on render frames. Eg Linear fades like opacity I've implemented this approach and restored the Generals changes (can replicate once this is approved). |
2bb1cb5 to
77e6dad
Compare
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameClient/Drawable.cpp | Physics decoupling via per-site timeScale multiplication; contains the pitchDamp linear approximation bug that can flip pitchRate sign at ~15 fps or below, plus multiple redundant timeScale queries per function. |
| GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp | Uses a more robust physics-on-logic-tick + render-interpolation approach; minor style nit with compressionFactor2 naming; xfer versioning bump to 9 with correct backward-compatibility handling. |
| Generals/Code/GameEngine/Include/GameClient/Drawable.h | Type changes for m_framesAirborneCounter, m_framesAirborne (Int to Real) and m_timeElapsedFade (UnsignedInt to Real) to support fractional frame accumulation. |
| GeneralsMD/Code/GameEngine/Include/GameClient/Drawable.h | Same type changes as Generals header, plus new m_prev* fields (prevTotalPitch/Roll/Yaw/Z) added to PhysicsXformInfo to support the interpolation approach, zero-initialized in constructor. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Render Frame] --> B{WW3D Sync Frame?}
subgraph Generals["Generals - scale-at-site approach"]
B -->|Yes - logic tick| C[calcPhysicsXform with timeScale applied to every delta]
B -->|No - extra render frame| D[applyPhysicsXform uses last calculated values as-is]
C --> D
end
subgraph ZeroHour["Zero Hour - save/interpolate approach"]
B -->|Yes - logic tick| E[Save prevTotalPitch/Roll/Yaw/Z then calcPhysicsXform at fixed 30 fps rate]
B -->|No - extra render frame| F[Interpolate between prev and current state using fractionalMs / LOGIC_FRAME_MS]
E --> F
end
D --> G[Apply transform to matrix]
F --> G
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Render Frame] --> B{WW3D Sync Frame?}
subgraph Generals["Generals - scale-at-site approach"]
B -->|Yes - logic tick| C[calcPhysicsXform with timeScale applied to every delta]
B -->|No - extra render frame| D[applyPhysicsXform uses last calculated values as-is]
C --> D
end
subgraph ZeroHour["Zero Hour - save/interpolate approach"]
B -->|Yes - logic tick| E[Save prevTotalPitch/Roll/Yaw/Z then calcPhysicsXform at fixed 30 fps rate]
B -->|No - extra render frame| F[Interpolate between prev and current state using fractionalMs / LOGIC_FRAME_MS]
E --> F
end
D --> G[Apply transform to matrix]
F --> G
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp:1663-1667
**Linear pitchDamp approximation goes negative at low framerates**
`pitchDamp = 1.0f - 0.5f * timeScale` produces a negative value whenever `timeScale > 2.0f` (i.e., roughly below ~15 fps). When that happens and `m_pitchRate > 0.0f`, the multiplication flips the sign of `pitchRate` instead of damping it — the wheel chassis momentarily kicks in the wrong direction. The correct frame-rate-independent equivalent of "multiply by 0.5 every 30-fps frame" is `powf(0.5f, timeScale)`, which stays positive for all positive `timeScale` values and matches exactly at `timeScale = 1.0`. The same pattern appears again in `calcPhysicsXformWheels` around line 1908.
### Issue 2 of 3
Generals/Code/GameEngine/Source/GameClient/Drawable.cpp:1638-1758
**Redundant `getActualLogicTimeScaleOverFpsRatio()` calls within a single function**
`calcPhysicsXformTreads` queries `TheFramePacer->getActualLogicTimeScaleOverFpsRatio()` four separate times using four different local variable names (`overlapTimeScale`, `timeScale`, `hitRecoilTimeScale`, and a second `overlapTimeScale` in a different scope). All four will return the same value during a single render update. Computing it once at the top of the function and reusing it would be both more efficient and easier to follow. The same pattern repeats in `calcPhysicsXformWheels` with `bounceTimeScale`, `timeScale`, `wheelAngleTimeScale`, and `compressionTimeScale`.
### Issue 3 of 3
GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp:2472-2474
The `2` suffix on `compressionTimeScale2` and `compressionFactor2` is unnecessary — these variables are in their own local scope within `calcPhysicsXformMotorcycle`, so they don't conflict with the same-named variables in `calcPhysicsXformWheels`.
```suggestion
// TheSuperHackers @tweak Wheel compression dampening is now decoupled from the render update.
const Real compressionTimeScale = TheFramePacer->getActualLogicTimeScaleOverFpsRatio();
const Real compressionFactor = 0.5f * compressionTimeScale;
```
Reviews (1): Last reviewed commit: "Replicate to generals" | Re-trigger Greptile
| if (m_locoInfo->m_pitchRate > 0.0f) | ||
| m_locoInfo->m_pitchRate *= 0.5f; | ||
| { | ||
| const Real pitchDamp = 1.0f - (1.0f - 0.5f) * timeScale; | ||
| m_locoInfo->m_pitchRate *= pitchDamp; | ||
| } |
There was a problem hiding this comment.
Linear pitchDamp approximation goes negative at low framerates
pitchDamp = 1.0f - 0.5f * timeScale produces a negative value whenever timeScale > 2.0f (i.e., roughly below ~15 fps). When that happens and m_pitchRate > 0.0f, the multiplication flips the sign of pitchRate instead of damping it — the wheel chassis momentarily kicks in the wrong direction. The correct frame-rate-independent equivalent of "multiply by 0.5 every 30-fps frame" is powf(0.5f, timeScale), which stays positive for all positive timeScale values and matches exactly at timeScale = 1.0. The same pattern appears again in calcPhysicsXformWheels around line 1908.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/Drawable.cpp
Line: 1663-1667
Comment:
**Linear pitchDamp approximation goes negative at low framerates**
`pitchDamp = 1.0f - 0.5f * timeScale` produces a negative value whenever `timeScale > 2.0f` (i.e., roughly below ~15 fps). When that happens and `m_pitchRate > 0.0f`, the multiplication flips the sign of `pitchRate` instead of damping it — the wheel chassis momentarily kicks in the wrong direction. The correct frame-rate-independent equivalent of "multiply by 0.5 every 30-fps frame" is `powf(0.5f, timeScale)`, which stays positive for all positive `timeScale` values and matches exactly at `timeScale = 1.0`. The same pattern appears again in `calcPhysicsXformWheels` around line 1908.
How can I resolve this? If you propose a fix, please make it concise.|
@xezon are you ok with copying the zero hour changes to Generals here? Greptile is right that they used different approaches, and I don't see the benefit in keeping or trying to tweak the Generals' one vs using ZH's interpolation |
This change decouples various drawable physics calculations and visual fade effects from the render update. This way they always run at the same speed regardless of frame rate.
Physics timing fixes:
Visual timing fixes: