Page MenuHomeWildfire Games

Pause renderer option for unfocused window
Needs ReviewPublic

Authored by ffffffff on Jan 11 2018, 11:33 PM.

Details

Reviewers
bb
elexis
vladislavbelov
Trac Tickets
#4975
Summary

Can help if wanna keep cpu usage down (can drop heavily depending on fps throttle from 100% down to 20 for me) when unfocus window but stay in game as observer/player and gameplay continues.

Also when in replay or general game and have high speed like 2x or 20x it can go many more turns per second, when renderer is paused due to unfocused window. So try this option if u like to pause rendering in general options.

Bugfix for game, apply background pause option directly when changed in options.

Test Plan

test

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

ffffffff created this revision.Jan 11 2018, 11:33 PM
ffffffff edited the summary of this revision. (Show Details)Jan 11 2018, 11:38 PM
ffffffff updated this revision to Diff 5239.Jan 12 2018, 1:01 AM

bugfix reload globals in core code on setChanges call in configdb object

ffffffff edited the summary of this revision. (Show Details)Jan 12 2018, 1:02 AM
ffffffff updated this revision to Diff 5240.Jan 12 2018, 1:40 AM
ffffffff edited the summary of this revision. (Show Details)
ffffffff updated this revision to Diff 5242.Jan 12 2018, 2:29 AM

default resetted to pause game on focus loss.

ffffffff updated this revision to Diff 5246.Jan 12 2018, 5:12 PM

name js interface function UpdatePauseOnFocusLoss.

ffffffff updated this revision to Diff 5247.Jan 12 2018, 5:22 PM

update option description.

ffffffff retitled this revision from Pause renderer option for unfocus window to Pause renderer option for unfocused window.Jan 15 2018, 5:12 AM
ffffffff edited the summary of this revision. (Show Details)
ffffffff edited the summary of this revision. (Show Details)
ffffffff added inline comments.Jan 17 2018, 12:09 AM
binaries/data/mods/public/gui/options/options.json
28–29

(gameplay continues, but cpu usage is shortend heavily, also good for hosting without play)

ffffffff updated the Trac tickets for this revision.Jan 18 2018, 7:12 AM

We definitely need the feature.

binaries/data/mods/public/gui/options/options.json
28–29

gameplay = simulation.
gameplay continuing is the reason why these are two different settings.
Theres the possibility to disable rendering if not focused and theres the possiblity to pause the simulation.

So the one should be in the graphics settings and the other in the gameplay / generic settings.

elexis requested changes to this revision.Jan 20 2018, 7:07 PM
This revision now requires changes to proceed.Jan 20 2018, 7:07 PM
ffffffff added inline comments.Jan 22 2018, 3:45 AM
binaries/data/mods/public/gui/options/options.json
28–29

graphics settings seem overloaded

Yet we may not mix graphics and simulation options. So I would click on request changes if I already hadn't.

We should doubt if it should be in Main.cpp and it's interface, rather than the Renderer. Inherently it's a rendering option.
But we want to avoid the SDL_GL_SwapWindow call too, so we need to modify Main.

The second question is whether we should reload the variable through the JS Interface, rather than just pulling the new value from the config each frame, like we do in the LimitFPS() function of Main.
While a CFG_GET_VAL is dirt cheap, it's still cleaner to avoid doing it each frame.
So ack on that actually and the LimitFPS should receive a similar function in a separate patch.

source/ps/scripting/JSInterface_Main.cpp
1

8

i dont want to go to deep into this. we got a second opinion by @bb if this option can stay in general?

ffffffff updated this revision to Diff 5561.Jan 29 2018, 5:39 AM

Sorry @elexis about my comment in D846!! :(

Here update in pause renderer

ffffffff edited the summary of this revision. (Show Details)Jan 29 2018, 5:40 AM
ffffffff added inline comments.Jan 29 2018, 12:36 PM
binaries/data/mods/public/gui/options/options.json
28–29

Ok, sorry.

Forget my comment. :(

source/ps/scripting/JSInterface_Main.cpp
1

y, sorry

elexis added inline comments.Jan 29 2018, 3:40 PM
binaries/data/config/default.cfg
456

The graphics option should be grouped with the other graphics options, maybe near adaptivefps, nohwcursor or vsync.

The other one could go near pauseonfocusloss... Oh that is already where it is.

binaries/data/mods/public/gui/options/options.json
30

Just leave this hunk untouched, it didn't do any harm

315

"Only Render When Focused"?

CPU hint is not part of the name, so should be moved to the tooltip. Also players will think it helps too if the game is focused unless the statement were constrained.

316

simulation is the accurate developer term, but for the player it's just "match" or "game" I suppose.

So, "Pause rendering if the window is not focused.\n[color=\"yellow\"]Tip[/color]: This can be helpful when fast forwarding."

Don't see the point to advertize people idling as observers in games without actually watching (what was the urban dictionary term for that?)

source/ps/GameSetup/Config.cpp
125

no new function for only 2 of 20 settings.

JSInterface_Renderer.cpp is the path every other renderer option takes

source/ps/GameSetup/Config.h
48

good rename and newname

Is it possible that it renders while syncing when rejoin in a game?

Maybe we can pause rendering when start syncing ingame until sync is complete so it may get faster sync due to low cpu/gpu use because of pause rendering. (Some maps depending on data can be very heavy sync time)

lol we cant do that the user wouldnt be able to click to stop the sync and disconnect or any other button :D

But it may go faster without render. so for the pros enable pause render on unfocus and unfocus window while sync or minimize window so sync might go faster XD

ffffffff updated this revision to Diff 5608.Jan 31 2018, 2:37 PM
ffffffff edited the summary of this revision. (Show Details)
ffffffff added inline comments.
binaries/data/config/default.cfg
131

remove > (simulation continues)

elexis added a comment.Feb 1 2018, 5:16 AM

Looks almost done

binaries/data/mods/public/gui/options/options.json
316

Not a fan of GUI tags in translated strings, every translator will have to get every character right, but they shouldn't be concerned with that formatting. We don't need the "Tip:" either since this is a toolTIP, so a Tip by definition.

source/renderer/scripting/JSInterface_Renderer.cpp
21 ↗(On Diff #5608)

Wondering why this include is needed if the other renderer config settings don't need it.
Main.cpp can query the renderer too if needed

84 ↗(On Diff #5608)

Why implement this differently from all the other renderer booleans? We have the REGISTER_BOOLEAN_SCRIPT_SETTING macro for that

ffffffff added inline comments.Feb 1 2018, 8:30 AM
source/renderer/scripting/JSInterface_Renderer.cpp
21 ↗(On Diff #5608)

only the main cpp use this global option its never used in renderr itself. Though the JSInterface function may go to JSMain Interface to and we leave the renderer completely untouched.

84 ↗(On Diff #5608)

same as above

ffffffff updated this revision to Diff 5618.Feb 1 2018, 10:01 AM
ffffffff updated this revision to Diff 5619.Feb 1 2018, 10:03 AM
elexis added a comment.Feb 1 2018, 7:28 PM

Tested, works as intended for the game.
But can you look into making it work for atlas too?

source/ps/scripting/JSInterface_Main.cpp
112

enabled

121

Guess it's not a terrible to do it JSI_Main, since it is used only by Main.cpp while none of the other config options are used here.
I just hope this isn't taken as a precedent to put unrelated config options here

atlas working no?

ffffffff updated this revision to Diff 5635.Feb 1 2018, 10:38 PM

atlas working no?

it is not seperate main loop files gameloop.cpp, input.cpp under tools/atlas

Maybe atlas can go in a seperate patch as it's completely different code base.
Although atm its not receiving any window notifications update messages from sdl as it is a new window.

elexis added inline comments.Feb 19 2018, 5:04 PM
source/main.cpp
308

D1212 changes the same line, we should figure that out before and afterwards we will still have to test that that solution still works.

ffffffff added inline comments.Feb 19 2018, 5:50 PM
source/main.cpp
308

true should be mergeable

Itms added subscribers: Angen, Itms.Mar 16 2019, 1:50 PM

Related to D1495, so you might want to take a look @Angen.

Angen added a comment.Mar 16 2019, 4:49 PM

D1945 moves fullscreen crash avoid handling inside render function and removes from frame function,so there can be optional do not render as it applies to every situation window and fullscreen.

Changes in D1945 has to stay to handle another callings of this function to prevent postprocessing in case alt-tab ed fullscreen and do not force players to disable postprocessing if this is the only flaw in their case.

I suggest from own experience if variable need_render is not used elswere only just on one place, to move its logic into the one if statement where that variable is called, as crash after minimalisation was fixed by moving g_minimised inside the if statement.

Could be considered to after if 1945 is applied to remove g_minimised from statement.

But in the other hand that variable should stay in the statement to reduce uneeded swap window calls.

Should be rebased on top of rP22314 @ffffffff

Stan added a subscriber: Stan.Tue, Jan 14, 7:35 AM

@vladislavbelov might be of interest to you