Skip to content

[GEN][ZH] Fix viewport scroll speed so it is independent of FPS in LookAtTranslator::translateGameMessage() #1026

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Generals/Code/GameEngine/Include/GameClient/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ class Display : public SubsystemInterface
virtual void setCinematicTextFrames( Int frames ) { m_cinematicTextFrames = frames; }

virtual Real getAverageFPS( void ) = 0; ///< returns the average FPS.
virtual Real getCurrentFPS( void ) = 0; ///< returns the current FPS.
virtual Int getLastFrameDrawCalls( void ) = 0; ///< returns the number of draw calls issued in the previous frame

protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,13 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
// scroll the view
if (m_isScrolling)
{

// TheSuperHackers @bugfix Mauller 07/06/2025 Adjust the viewport scrolling so it is independent of GameClient FPS
// The scaling is based on the current logic rate, this provides a consistent scroll speed at all GameClient FPS
// This also fixes scrolling within replays when fast forwarding due to the uncapped FPS
// When the FPS is in excess of the expected frame rate, the ratio will reduce the offset of the cameras movement
Real logicToFpsRatio = TheGlobalData->m_framesPerSecondLimit / TheDisplay->getCurrentFPS();

switch (m_scrollType)
{
case SCROLL_RMB:
Expand All @@ -416,34 +423,35 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
m_anchor.y = m_currentPos.y - maxY;
}

offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * (m_currentPos.x - m_anchor.x);
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * (m_currentPos.y - m_anchor.y);
// TheSuperHackers @fix Mauller 16/06/2025 Fix RMB scrolling to make it scale properly with scroll factor and to make it move consistently in all directions
Coord2D vec;
vec.x = offset.x;
vec.y = offset.y;
vec.x = (m_currentPos.x - m_anchor.x);
vec.y = (m_currentPos.y - m_anchor.y);
// TheSuperHackers @info we need to get the length of the vector before the vector is normalised
// The length is then used to properly scale the speed using the mouse position from the anchor point
float vecLength = vec.length();
vec.normalize();
// Add in the window scroll amount as the minimum.
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * vec.x * sqr(TheGlobalData->m_keyboardScrollFactor);
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * vec.y * sqr(TheGlobalData->m_keyboardScrollFactor);
offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.x * TheGlobalData->m_keyboardScrollFactor;
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.y * TheGlobalData->m_keyboardScrollFactor;
Copy link

Choose a reason for hiding this comment

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

This looks as if it does not produce the same result as before. You can verify correctness by temporarily running the original code beside the refactored code, then assert value equality (with a small epsilon for rounding error tolerance when so necessary).

Simplifying the implementation is certainly desirable.

Copy link
Author

@Mauller Mauller Jun 17, 2025

Choose a reason for hiding this comment

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

After doing a little more testing last night the original code doesn't actually do what it was intended to.

The significant factor that makes up the majority of the movement is the following block from the original code.

offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * (m_currentPos.x - m_anchor.x);
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * (m_currentPos.y - m_anchor.y);

The second block that gets added to the offset provides a miniscule amount to the overall movement. It's why when you change the scrolling speed it hardly affected right mouse scrolling.

If you just set the RMB scrolling to run from the second block you can see that it hardly moves the viewport, even with the scroll speed set to maximum. Making that added factor insignificant in the end.
(you also have to make sure the FPS scaling is handled when checking)

The refactored code allows the scrolling to work properly and get scaled by the scroll speed options.
When compared with the first line in the original code, it has essentially become the following

offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * vec.x * vecLength;
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * vec.y * vecLength;

That then provides normalised scrolling in all directions. Then you include the fps scaling to mitigate the FPS issues, Then multiply by TheGlobalData->m_keyboardScrollFactor which adds the ability to alter the scroll speed through the options menu.

Copy link
Author

Choose a reason for hiding this comment

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

The speed of the scrolling can be a little faster than expected at the moment.
But i think to work around that we just need to rescale how the keyboard scroll factor affects the RMB scrolling.

}
break;
case SCROLL_KEY:
{
if (scrollDir[DIR_UP])
{
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_DOWN])
{
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_LEFT])
{
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_RIGHT])
{
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
}
break;
Expand All @@ -453,19 +461,19 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
UnsignedInt width = TheDisplay->getWidth();
if (m_currentPos.y < edgeScrollSize)
{
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.y >= height-edgeScrollSize)
{
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.x < edgeScrollSize)
{
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.x >= width-edgeScrollSize)
{
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ class W3DDisplay : public Display
static W3DAssetManager *m_assetManager; ///< W3D asset manager

void drawFPSStats( void ); ///< draw the fps on the screen
virtual Real getAverageFPS( void ); ///< return the average FPS.
virtual Real getAverageFPS( void ); ///< return the average FPS.
virtual Real getCurrentFPS( void ); ///< return the current FPS.
virtual Int getLastFrameDrawCalls( void ); ///< returns the number of draw calls issued in the previous frame

protected:
Expand All @@ -166,6 +167,7 @@ class W3DDisplay : public Display
IRegion2D m_clipRegion; ///< the clipping region for images
Bool m_isClippedEnabled; ///<used by 2D drawing operations to define clip re
Real m_averageFPS; ///<average fps over the last 30 frames.
Real m_currentFPS; ///<current fps value.
#if defined(RTS_DEBUG) || defined(RTS_INTERNAL)
Int64 m_timerAtCumuFPSStart;
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,8 @@ void W3DDisplay::updateAverageFPS(void)
if (historyOffset >= FPS_HISTORY_SIZE)
historyOffset = 0;

double currentFPS = 1.0/elapsedSeconds;
fpsHistory[historyOffset++] = currentFPS;
m_currentFPS = 1.0/elapsedSeconds;
fpsHistory[historyOffset++] = m_currentFPS;
numSamples++;
if (numSamples > FPS_HISTORY_SIZE)
numSamples = FPS_HISTORY_SIZE;
Expand Down Expand Up @@ -1559,6 +1559,11 @@ Real W3DDisplay::getAverageFPS()
return m_averageFPS;
}

Real W3DDisplay::getCurrentFPS()
{
return m_currentFPS;
}

Int W3DDisplay::getLastFrameDrawCalls()
{
return Debug_Statistics::Get_Draw_Calls();
Expand Down
1 change: 1 addition & 0 deletions Generals/Code/Tools/GUIEdit/Include/GUIEditDisplay.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class GUIEditDisplay : public Display
#endif

virtual Real getAverageFPS(void) { return 0; }
virtual Real getCurrentFPS(void) { return 0; }
virtual Int getLastFrameDrawCalls( void ) { return 0; }

protected:
Expand Down
1 change: 1 addition & 0 deletions GeneralsMD/Code/GameEngine/Include/GameClient/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ class Display : public SubsystemInterface
virtual void setCinematicTextFrames( Int frames ) { m_cinematicTextFrames = frames; }

virtual Real getAverageFPS( void ) = 0; ///< returns the average FPS.
virtual Real getCurrentFPS( void ) = 0; ///< returns the current FPS.
virtual Int getLastFrameDrawCalls( void ) = 0; ///< returns the number of draw calls issued in the previous frame

protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,13 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
// scroll the view
if (m_isScrolling)
{

// TheSuperHackers @bugfix Mauller 07/06/2025 Adjust the viewport scrolling so it is independent of GameClient FPS
// The scaling is based on the current logic rate, this provides a consistent scroll speed at all GameClient FPS
// This also fixes scrolling within replays when fast forwarding due to the uncapped FPS
// When the FPS is in excess of the expected frame rate, the ratio will reduce the offset of the cameras movement
Real logicToFpsRatio = TheGlobalData->m_framesPerSecondLimit / TheDisplay->getCurrentFPS();

switch (m_scrollType)
{
case SCROLL_RMB:
Expand All @@ -415,34 +422,35 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
m_anchor.y = m_currentPos.y - maxY;
}

offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * (m_currentPos.x - m_anchor.x);
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * (m_currentPos.y - m_anchor.y);
// TheSuperHackers @fix Mauller 16/06/2025 Fix RMB scrolling to make it scale properly with scroll factor and to make it move consistently in all directions
Coord2D vec;
vec.x = offset.x;
vec.y = offset.y;
vec.x = (m_currentPos.x - m_anchor.x);
vec.y = (m_currentPos.y - m_anchor.y);
// TheSuperHackers @info we need to get the length of the vector before the vector is normalised
// The length is then used to properly scale the speed using the mouse position from the anchor point
float vecLength = vec.length();
vec.normalize();
// Add in the window scroll amount as the minimum.
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * vec.x * sqr(TheGlobalData->m_keyboardScrollFactor);
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * vec.y * sqr(TheGlobalData->m_keyboardScrollFactor);
offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.x * TheGlobalData->m_keyboardScrollFactor;
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.y * TheGlobalData->m_keyboardScrollFactor;
}
break;
case SCROLL_KEY:
{
if (scrollDir[DIR_UP])
{
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_DOWN])
{
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_LEFT])
{
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (scrollDir[DIR_RIGHT])
{
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
}
break;
Expand All @@ -452,19 +460,19 @@ GameMessageDisposition LookAtTranslator::translateGameMessage(const GameMessage
UnsignedInt width = TheDisplay->getWidth();
if (m_currentPos.y < edgeScrollSize)
{
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y -= TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.y >= height-edgeScrollSize)
{
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.x < edgeScrollSize)
{
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x -= TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
if (m_currentPos.x >= width-edgeScrollSize)
{
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * SCROLL_AMT * TheGlobalData->m_keyboardScrollFactor;
}
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ class W3DDisplay : public Display
static W3DAssetManager *m_assetManager; ///< W3D asset manager

void drawFPSStats( void ); ///< draw the fps on the screen
virtual Real getAverageFPS( void ); ///< return the average FPS.
virtual Real getAverageFPS( void ); ///< return the average FPS.
virtual Real getCurrentFPS( void ); ///< return the current FPS.
virtual Int getLastFrameDrawCalls( void ); ///< returns the number of draw calls issued in the previous frame

protected:
Expand All @@ -167,6 +168,7 @@ class W3DDisplay : public Display
IRegion2D m_clipRegion; ///< the clipping region for images
Bool m_isClippedEnabled; ///<used by 2D drawing operations to define clip re
Real m_averageFPS; ///<average fps over the last 30 frames.
Real m_currentFPS; ///<current fps value.
#if defined(RTS_DEBUG) || defined(RTS_INTERNAL)
Int64 m_timerAtCumuFPSStart;
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -899,8 +899,8 @@ void W3DDisplay::updateAverageFPS(void)
if (historyOffset >= FPS_HISTORY_SIZE)
historyOffset = 0;

double currentFPS = 1.0/elapsedSeconds;
fpsHistory[historyOffset++] = currentFPS;
m_currentFPS = 1.0/elapsedSeconds;
fpsHistory[historyOffset++] = m_currentFPS;
numSamples++;
if (numSamples > FPS_HISTORY_SIZE)
numSamples = FPS_HISTORY_SIZE;
Expand Down Expand Up @@ -1647,6 +1647,11 @@ Real W3DDisplay::getAverageFPS()
return m_averageFPS;
}

Real W3DDisplay::getCurrentFPS()
{
return m_currentFPS;
}

Int W3DDisplay::getLastFrameDrawCalls()
{
return Debug_Statistics::Get_Draw_Calls();
Expand Down
1 change: 1 addition & 0 deletions GeneralsMD/Code/Tools/GUIEdit/Include/GUIEditDisplay.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class GUIEditDisplay : public Display
#endif

virtual Real getAverageFPS(void) { return 0; }
virtual Real getCurrentFPS(void) { return 0; }
virtual Int getLastFrameDrawCalls( void ) { return 0; }

protected:
Expand Down
Loading