pqiv icon indicating copy to clipboard operation
pqiv copied to clipboard

High cpu usage with fading enabled

Open dtop129 opened this issue 4 years ago • 4 comments

When fading is enabled and pqiv is opened, until the first image that is opened changes the CPU is really high, and as soon as I change image, or even reload the first one, the usage drops back to 0, and the cpu usage spikes stop.

I found the culprit in fading_timeout_callback, that apparently gets continuosly called until the first image changes or is reloaded, but I have no idea how to fix this

dtop129 avatar Aug 22 '21 16:08 dtop129

Thanks for reporting this! The callback shouldn't be invoked at all for the first image, that's the proper way to fix this. I'll try to take a look myself asap but can't make any promises on when that'll be.

phillipberndt avatar Aug 22 '21 16:08 phillipberndt

If that fixes it than I think I came up with a solution. Basically, before the portion of code activating fading, count how many images are loaded, and use fading only if more than 1 image is loaded. Apparently it fixes the issue, but I have no idea if it silently breaks something under the hood

diff --git a/pqiv.c b/pqiv.c
index 69cf173..7dd6d69 100644
--- a/pqiv.c
+++ b/pqiv.c
@@ -2321,8 +2321,11 @@ gboolean image_loaded_handler(gconstpointer node) {/*{{{*/
 		return FALSE;
 	}
 
+	int n = 0;
+	for(GList *node_list = loaded_files_list; node_list; node_list = g_list_next(node_list), n++);
+
 	// Activate fading
-	if(option_fading) {
+	if(option_fading && n > 1) {
 		if(fading_surface) {
 			cairo_surface_destroy(fading_surface);
 			fading_surface = NULL;

dtop129 avatar Aug 22 '21 20:08 dtop129

I think this will break if low-memory mode is enabled. But try and see what happens if you use last_visible_surface (or fading_surface, which at that point references the former) as a condition instead: If it is NULL, the callback should not be scheduled and the fading related variables should not be initialized.

phillipberndt avatar Aug 23 '21 05:08 phillipberndt

You were right about my solution breaking fading in low memory mode, so I tried checking for NULL in last_visible_surface and fading_surface.

Checking fading_surface breaks fading, while checking last_visible_surface seems to do the trick(and doesn't break low memory mode)

diff --git a/pqiv.c b/pqiv.c
index d46d795..b80fa7f 100644
--- a/pqiv.c
+++ b/pqiv.c
@@ -2334,14 +2334,12 @@ gboolean image_loaded_handler(gconstpointer node) {/*{{{*/
 	}
 
 	// Activate fading
-	if(option_fading) {
+	if(option_fading && last_visible_surface) {
 		if(fading_surface) {
 			cairo_surface_destroy(fading_surface);
 			fading_surface = NULL;
 		}
-		if(last_visible_surface) {
-			fading_surface = cairo_surface_reference(last_visible_surface);
-		}
+		fading_surface = cairo_surface_reference(last_visible_surface);
 
 		if(fading_current_alpha_stage > 0 && fading_current_alpha_stage < 1.) {
 			// If another fade was already active, don't start another one.

dtop129 avatar Aug 23 '21 09:08 dtop129