bugfix(drawable): Decouple visual render from projectile's logic position - #2598
Conversation
|
| Filename | Overview |
|---|---|
| GeneralsMD/Code/GameEngine/Source/GameClient/Drawable.cpp | Adds logic-frame transform sampling and a decoupled visual matrix used during drawable rendering. |
| GeneralsMD/Code/GameEngine/Include/GameClient/Drawable.h | Declares the drawable interpolation state and sub-frame transform helper. |
| Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp | Adds frame-sampled interpolation for turret rotation and pitch. |
| Core/GameEngineDevice/Include/W3DDevice/GameClient/Module/W3DModelDraw.h | Stores previous and current turret samples for visual interpolation. |
| Generals/Code/GameEngine/Include/Common/GameEngine.h | Exposes the current logic-time accumulator to rendering code. |
| GeneralsMD/Code/GameEngine/Include/Common/GameEngine.h | Exposes the current logic-time accumulator to rendering code. |
Sequence Diagram
sequenceDiagram
participant Logic as GameLogic
participant Engine as GameEngine
participant Drawable as Drawable
participant Model as W3DModelDraw
participant Renderer as W3D Renderer
Logic->>Drawable: Update object transform at fixed tick
Engine->>Drawable: Provide remaining logic-time accumulator
Drawable->>Drawable: Sample previous/current transforms
Drawable->>Drawable: Compute sub-frame visual transform
Model->>Model: Sample and interpolate turret angles
Drawable->>Renderer: Render interpolated world transform
Model->>Renderer: Render interpolated turret bones
Reviews (8): Last reviewed commit: "fix generals" | Re-trigger Greptile
| { | ||
|
|
||
| } | ||
| void Drawable::setLogicVelocity(const Coord3D* velocity) |
There was a problem hiding this comment.
I think this is going into the right direction and this ideally becomes applicable to ALL drawable, not just projectiles. I am not a fan of this hand holding of requiring external users to set velocities. It makes caller code more complicated.
I suggest to try implement this more generic so that any movements are interpolated between real logic frames. To do that, try to have a recycled history of 3 to 5 transforms and then extrapolate them. This could work alright even with accelleration and braking. Maybe make the history length configurable per drawable, because for objects that have no accelleration, 2 points would be enough for velocity.
…osition, rotation, and turrets
be62f3b to
0c3caf7
Compare
|
|
||
| Bool m_receivesDynamicLights; | ||
|
|
||
| private: |
| m_currLogicMtx = currentMtx; | ||
|
|
||
| Vector3 deltaPos = m_currLogicMtx.Get_Translation() - m_prevLogicMtx.Get_Translation(); | ||
| if (deltaPos.Length2() > 40000.0f) // >200 units threshold for teleportation |
There was a problem hiding this comment.
why 40.000?
Probably should be a well documented const variable.
There was a problem hiding this comment.
sqr(200.f)
In what event does teleportation occur? Is there a script action for it?
The cleaner way would be to remove the magic teleport distance and invalidate the interpolation on real teleportation events.
xezon
left a comment
There was a problem hiding this comment.
This is going into the right direction judging by the code. A few things can be improved.
| m_useExtrapolation = FALSE; | ||
| m_visualExtrapolationMtx.Make_Identity(); | ||
| m_prevLogicMtx.Make_Identity(); | ||
| m_currLogicMtx.Make_Identity(); |
There was a problem hiding this comment.
Are 2 sample points sufficient for acceleration and braking units? I would expect we need at least 3 samples to extrapolate that more accurately, because the delta of transform and rotation between N-2, N-1 and N will give a prediction scale for N to N+1 (, which may or may not be beneficial).
| m_lastSampledLogicFrame = currentLogicFrame; | ||
| } | ||
|
|
||
| Real alpha = TheGameEngine->getLogicTimeAccumulator() * TheFramePacer->getActualLogicTimeScaleFps(); |
There was a problem hiding this comment.
My understanding is that this is a value between 0 and 1 to tell how far the current sub frame has advanced before it reaches the next logic frame.
It would be nice to abstract this concept in the FramePacer because it can be useful in many places. For that the GameEngine needs to give it information every render update.
|
|
||
| m_visualExtrapolationMtx = m_prevLogicMtx; | ||
|
|
||
| if (fabs(angleDelta) > 0.001f && fabs(angleDelta) < 3.0f) |
| ai->getTurretRotAndPitch((WhichTurretType)tslot, &turretAngle, &turretPitch); | ||
| } | ||
| Real angleDelta = stdAngleDiff(m_currTurretAngle[tslot], m_prevTurretAngle[tslot]); | ||
| Real interpTurretAngle = m_prevTurretAngle[tslot] + angleDelta * clampAlpha; |
| const AIUpdateInterface* ai = obj ? obj->getAIUpdateInterface() : nullptr; | ||
| UnsignedInt currentLogicFrame = TheGameLogic ? TheGameLogic->getFrame() : 0; | ||
|
|
||
| if (currentLogicFrame != m_lastTurretSampledFrame) |
There was a problem hiding this comment.
Can move this portion into a new function to tidy up and give it a name.
|
|
||
| UnsignedInt currentLogicFrame = TheGameLogic->getFrame(); | ||
|
|
||
| if (currentLogicFrame != m_lastSampledLogicFrame) |
There was a problem hiding this comment.
Can move this portion into a new function to tidy up and give it a name.
|
|
||
| if (fabs(angleDelta) > 0.001f && fabs(angleDelta) < 3.0f) | ||
| { | ||
| m_visualExtrapolationMtx.In_Place_Pre_Rotate_Z(angleDelta * clampAlpha); |
There was a problem hiding this comment.
If I am reading this right this only predicts rotation on Z axis. I think this should Slerp with Quaternion to rotate all axis. Using Quaternion, because that should yield better interpolation than the Matrix does.
Caball009
left a comment
There was a problem hiding this comment.
There are quite a number of variables in the new code that can be marked as const. I think it wouldn't hurt to check for const correctness and change where possible.
| for (i = 0; i < MAX_TURRETS; ++i) | ||
| { | ||
| m_prevTurretAngle[i] = 0.0f; | ||
| m_currTurretAngle[i] = 0.0f; | ||
| m_prevTurretPitch[i] = 0.0f; | ||
| m_currTurretPitch[i] = 0.0f; | ||
| } |
There was a problem hiding this comment.
Could do this instead:
std::fill(m_prevTurretAngle, m_prevTurretAngle + ARRAY_SIZE(m_prevTurretAngle), 0.0f);
std::fill(m_currTurretAngle, m_currTurretAngle + ARRAY_SIZE(m_currTurretAngle), 0.0f);
std::fill(m_prevTurretPitch, m_prevTurretPitch + ARRAY_SIZE(m_prevTurretPitch), 0.0f);
std::fill(m_currTurretPitch, m_currTurretPitch + ARRAY_SIZE(m_currTurretPitch), 0.0f);
This PR fixes the visual "stutter" or "jitter" seen on drawables when playing at high framerates. While the game's internal logic updates at a fixed frequency, rendering often happens much faster, causing drawables to appear to "step" across the screen rather than move smoothly.
Implemented a system that fills in the gaps between logic steps, allowing drawables to match the user's framerate.
Created a unified way for the game's rendering system to communicate with various drawables types.
Video is of 30 logic and 60 visual, notice the projectile moving with every step:
https://github.com/user-attachments/assets/fcdefce9-723d-4d3b-aeda-d60d1fcce5de
Todo:
replicate to generals