SDL: Minimal render rate support (force-frame-update)

[magicmyth]

Scummvm is optimised to render frames when something changes on screen b some host environments can perform poorly if the app does not reliably refresh its output regularly. For example, Steam overlay will get stuck if it is brought up when the scummvm menu is active. This also affects streaming via a Steam Link. Thus this new option ensures that Scummvm outputs a minimal amount of frames even if nothing is changing in the game renderer.

Currently this is only implemented in the SDL OpenGL renderer.

The new config option is called force-frame-update and it takes a integer value representing the desired minimum milliseconds Scummvm should wait before forcing a screen update/refresh. E.g: 50. I don't know if it is possible with Scummvm's config system but it might be good to support a value of true (as well as integers) which would simply ensure a minimum sensible value so the user does not have to think of a good millisecond value.

Note that the rendering system will not force a re-draw of a frame if the app has rendered a changed frame within the desired minimum refresh. Thus if the app is outputting 30fps and force-frame-update is set to 100ms (~10fps) then no duplicate frame will be shown (in theory).

As this is implemented in OpenGLSdlGraphicsManager::updateScreen() OpenGLGraphicsManager::_forceRedraw has had its access changed to protected so that it can be accessed by it's descendant. The reason this has been done in OpenGLSdlGraphicsManager::updateScreen() is so SDL_GetTicks() can be used to track the elapsed time. If it is useful for other platforms using OpenGL to have this feature it could be implemented within OpenGLGraphicsManager::updateScreen() provided a suitable platform independent replacement for SDL_GetTicks() is used. This would potentially be better as OpenGLGraphicsManager checks various other states when deciding if the screen should update. If anyone could give some pointers on how I should track ticks in Scummvm that would be appreciated.

The tracking of rendering is done by storing a function static variable in updateScreen() that is updated whenever _forceRedraw is found to be true. This could be changed to a property of the object if that is preferred?

[csnover]

If anyone could give some pointers on how I should track ticks in Scummvm that would be appreciated.

OSystem::getMillis is the backend-agnostic mechanism for getting time since the program was started.

These changes will conflict with #1009 (or perhaps I should say, #1009 eases the implementation by also exposing the redraw flag to all windowed graphics managers).

Unfortunately because of the architecture of OSystem and the non-requirement of engines to call updateScreen at least <refresh rate> times per second it’s impossible to guarantee frame updates, but I suppose this will at least address the common case.

I don’t think this should really be something user-configurable, instead just service at 60Hz or whatever is the optimal refresh rate for the device (i.e. make it a backend decision).

Assuming this same problem exists with SurfaceSdlGraphicsManager, I would also not like to only have this implemented for OpenGL, since the SurfaceSdlGraphicsManager is still currently the default.

Hopefully some other people with more knowledge on ports and history of the dirty-checking can weigh in here with their thoughts.

[magicmyth]

OSystem::getMillis is the backend-agnostic mechanism for getting time since the program was started.

Thanks, I'll try and test that out some time soon.

These changes will conflict with #1009 (or perhaps I should say, #1009 eases the implementation[...]
[...]it’s impossible to guarantee frame updates[...]

Good to see once that pull goes through we'll be able to more easily expand this to the other graphic managers. The implementation right now is more proof of concept that this resolves interaction with third-party overlays. In the ideal world we would, like you said, sync this to the display but as you also mentioned the engines do not call updateScreen regularly. Because of the differences between engines I thought a tweakable value so a user can find what works best on a per-engine, per-system bases was a good alternative.

One of my concerns is that having the repeating of frames more regularly than is minimally needed might interfere with actual changed rendering on screen causing visual hickups but maybe the opposite is true. It also increases the demands on the system. This is also why I think having it support both a "true" flag and and a integer value is a good idea for now as it will allow user's to test and give feedback and if running at screen refresh (or a lower division of it) proves perfectly reliable the integer value gets ignored later down the road.

Assuming this same problem exists with SurfaceSdlGraphicsManager[...]

It does but as it is not being rendered via a 3D API that allows other external components to hook into it I figured it was not of use there but I could be wrong and I see no harm having the same option available throughout.

I'm not a game engine programmer (and would greatly welcome those with knowledge on the subject and engines) so I don't know if we could actually match a vblank interval like this. For one 60hz is 16.7ms but if we are dealing with milliseconds in integers how do we represent that? Would it mater and would we set a value a bit higher than it?

To have more reliable updates would it be possible for scummvm to ensure updateScreen gets called at least once per-game loop?

Thanks for the feedback.

[rsn8887]

I don’t think this should really be something user-configurable, instead just service at 60Hz or whatever is the optimal refresh rate for the device (i.e. make it a backend decision).

It also increases the demands on the system.

Yes, I think an option to enable/disable this and/or set the minimum fps per system backend would be desirable. Older systems like the Pandora or PSP will have a hard time keeping up with a minimum update rate unless it is set very low.

[fedor4ever]

ScummVM redraw screen when needed and this very good for long battery life. As symbian porter I prefer to eliminate such functionality at compile time in Symbian port.

[sev-]

@magicmyth Apologies for not coming back, somehow I lost this PR. The idea is good and sensible.

No, it would not be possible to have both true/false and an integer in the same config key. So, you could use two keys: force_frame_update as a boolean, use ConfMan.getBool() for that, and force_frame_update_period or force_frame_update_fps, and combination of true in the first key and 0 in the second would mean "guess automatically". I think, that going with fps is more user-friendly, and you can deduce milliseconds from it in the code.

Also, the fact that it is now implemented only in OpenGL is okay, at least it is something. Once implemented, I can modify other SDL backends.

So, if you have time, could you please update this PR to the modern codebase and I merge. If I do not hear from you in several weeks or you tell me that you have no time, then I'll proceed with doing it by myself.

[sev-]

@fedor4ever what you wrote has no sense, buddy. The functionality is implemented as optional. Then, you do not have OpenGL backend on SymbianOS.

[sev-]

I will merge this change and also put this setting to GUI as a text field with numbers.

[somaen]

This variable can be declared inside the else-block to make it's scope minimal. (That also makes it easier to see when it's first assigned.

[raziel-]

just out of curiosity...was this ever merged, even partially?
Is this still something lower-end backends could benefit from?

Thank you

[bluegr]

Since this has bitrot, and the feedback given hasn't been addressed, it has been rewritten and reopened in #6092