Bug 327999 - Make pause on minimize optional
Summary: Make pause on minimize optional
Status: RESOLVED NOT A BUG
Alias: None
Product: kmahjongg
Classification: Applications
Component: general (show other bugs)
Version: 0.8
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Unassigned bugs mailing-list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-24 06:54 UTC by Jan-Peter Nilsson
Modified: 2014-12-23 16:09 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch to make pause on minimize optional (2.79 KB, patch)
2013-11-24 06:55 UTC, Jan-Peter Nilsson
Details
Patch for just hiding the tiles when pausing instead of redrawing the entire game board (751 bytes, patch)
2014-12-15 23:19 UTC, Jan-Peter Nilsson
Details
Create tiles when redrawing even if the game is paused (1.22 KB, patch)
2014-12-17 08:21 UTC, Jan-Peter Nilsson
Details
Create tiles when redrawing even if the game is paused (2.25 KB, patch)
2014-12-22 21:42 UTC, Jan-Peter Nilsson
Details
Create tiles when redrawing even if the game is paused (2.25 KB, patch)
2014-12-23 15:44 UTC, Jan-Peter Nilsson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan-Peter Nilsson 2013-11-24 06:54:02 UTC
Newer versionf of KMahjongg (ec7be2a and forward) have started to automatically pauses on Qt::WindowMinimized, to me this is annoying as I do frequent quick switches of desktop to check other things. This significalty delays those switches as the pause/unpause adds quite a bit of time.

I would like to make automatic pause on minimize optional.
I have been using a patch for this for a few months which I will attach.

Reproducible: Always

Steps to Reproduce:
1. Start a game
2. Switch to another virtual desktop
3. Switch back
4. Notice how unpausing and redrawing the board takes a while
Comment 1 Jan-Peter Nilsson 2013-11-24 06:55:37 UTC
Created attachment 83726 [details]
Patch to make pause on minimize optional
Comment 2 Albert Astals Cid 2014-12-14 18:05:04 UTC
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)
Comment 3 Jan-Peter Nilsson 2014-12-14 19:43:12 UTC
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.
Comment 4 Albert Astals Cid 2014-12-14 20:35:58 UTC
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?
Comment 5 Jan-Peter Nilsson 2014-12-15 00:57:57 UTC
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.
Comment 6 Albert Astals Cid 2014-12-15 19:22:33 UTC
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?
Comment 7 Jan-Peter Nilsson 2014-12-15 23:13:47 UTC
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.
Comment 8 Jan-Peter Nilsson 2014-12-15 23:19:01 UTC
Created attachment 89997 [details]
Patch for just hiding the tiles when pausing instead of redrawing the entire game board
Comment 9 Albert Astals Cid 2014-12-16 00:00:02 UTC
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?
Comment 10 Jan-Peter Nilsson 2014-12-17 08:21:05 UTC
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 11 Albert Astals Cid 2014-12-19 23:55:15 UTC
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?
Comment 12 Jan-Peter Nilsson 2014-12-22 21:39:41 UTC
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.
Comment 13 Jan-Peter Nilsson 2014-12-22 21:42:56 UTC
Created attachment 90092 [details]
Create tiles when redrawing even if the game is paused
Comment 14 Jan-Peter Nilsson 2014-12-23 15:44:47 UTC
Created attachment 90099 [details]
Create tiles when redrawing even if the game is paused

Updated the commit message, "redawing" to "redrawing".
Comment 15 Albert Astals Cid 2014-12-23 15:58:32 UTC
Pushed, i also changed the parameter name on the .h file
Comment 16 Albert Astals Cid 2014-12-23 15:58:45 UTC
Now, can we close this bug?
Comment 17 Jan-Peter Nilsson 2014-12-23 16:09:59 UTC
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.