Bug 327999

Created attachment 83726 [details]
Patch to make pause on minimize optional

Why is pause annoying? I mean the window is minimized, so why would you care? I can see there's bug related to the pausing and stuff, but maybe what we should do is just fix those issues? (as you're already doing)

Good question.

I think pause on minimize it is working as intended in here, but it becomes annoying due to the time it takes. When quickly switcing to a different virtual desktop and back as it adds a few seconds where the window is blank (if switching back within a second or two, probably if it is not done pausing) followed by a few seconds of background only before the tiles reappear.

So strictly my problem is not with the automatic pause, it is that pausing/unpausing takes a few seconds and in the meanwhile I'm left with an an unusable window.
Significantly reducing the amount of time pausing/unpausing takes should also solve this, and would probably also benefit manual pause/unpause.

I'll have a look and see what actually takes time.

Pausing/unpausing takes seconds? Which cpu/memory/graphics card do you have? It is virtually instantaneous here. Does it also happen if you press the pause/unpause button or only on virtual terminal switch?

After running the game with callgrind it turns out the reason is that the 3MB limit set on the  QPixmapCache is not enough to hold the background image, so above a specific window size it can't be cached and must be redrawn from svg, which is what takes time.

That's interesting, which resolution are you running? I'm at 1920x1080 and don't have that issue. Why would that matter, the background should not need re-rendering since it doesn't change of size nor is hidden when paused, no?

I'm using a few different resolutions depending on which machine I'm at, but I'm seeing the same thing with 1366x768, 1920x1080 and 3840x2160.
Which is not really surprising, (1366*768*4)/1024 = 4098KB will not fit in a 3072KB cache.

If you are using one of the tiling backgrounds (default or light wood) or plain color you should not have a problem.
Using any of the other backgrounds you should get redraws from svg when a full redraw of the game board happens. Full redraws are triggered by pause, unpause, new game, load game, shuffle, load settings and resize.
I find it is most noticeable when using the 'chinese landscape' background (which happens to be what I have been using).

You are right, the background should not need to be re-rendered, looking at BoardWidget::pause() I don't see any reason for it to need a full redraw, as far as I can tell all we want to do is to show/hide the tiles and iterating over the them and setting their visibility seems to work fine for that.

Ideally I guess only load settings (for an initial load and when background/tileset was changed) and resize should require a full redraw. I will look a bit more at the other functions, but they are a bit off-topic here.

Created attachment 89997 [details]
Patch for just hiding the tiles when pausing instead of redrawing the entire game board

I've pushed the patch, i guess you can close this bug as invalid now since there's no really need to make pausing optional?

Created attachment 90008 [details]
Create tiles when redrawing even if the game is paused

I messed up a bit with the previous patch, I did not think about what will happen if we trigger a drawBoard while paused. drawBoard must also be updated to create tiles even if the game is paused, otherwise we will not have any tiles to show after unpausing.
This can for example be triggered by pausing the game, resizing the window and then unpausing.

Attaching a patch for generating the tiles but not setting them to visible if paused.

Yes, with the pause/unpause delays removed I don't have any reason for it to be optional and this can be closed, thank you for asking questions.

Comment on attachment 90008 [details]
Create tiles when redrawing even if the game is paused

Should the 

-                    if (thissprite) {
+                    if (thissprite && !gamePaused) {

be on all the switches of that code and not only on one?

Yes the condition should be duplicated for all view angles, thank you.

We can reduce the duplication a bit by breaking out parts of updateSpriteMap, the only thing that really have to be different between the different angles is some offsets and the loop order when building the stacking order.
But that is not really part of this bug, so first the small change so that we can close this.

Created attachment 90092 [details]
Create tiles when redrawing even if the game is paused

Created attachment 90099 [details]
Create tiles when redrawing even if the game is paused

Updated the commit message, "redawing" to "redrawing".

Pushed, i also changed the parameter name on the .h file

Now, can we close this bug?

Thank you.
Yes I will set it as resolved, pause on minimize is now fast enough that I don't have any reason to disable it anymore.