don't force-refresh center view on side panel refresh

[ralfbrown]

I figured out why redrawn widgets in the side panels were also causing the center view to be redrawn, resulting in 40+% CPU usage (200+% with an active instance of liquify having a large warp) just from moving the mouse over the list of collections in the collection module or over the steps in the history module (both of which produce a fade-in/fade-out effect on the hovered item): the callback function for the Gtk "draw" signal on the side panels was always invoking a redraw on the center view even though it's entirely unnecessary.... With this patch, CPU usage is in the 4-5% range when moving over those modules.

I think the intent was to change the cursor when over the clickable link to the documentation that gets displayed on an empty lighttable, but it also gets called in the other views, and hovering over that link would not invoke the callback on the side panels anyway.

Since that callback is now a no-op, we could remove it entirely. Your call.

[TurboGit]

Yes, probably better to remove this callback at this point.

[TurboGit]

@AlicVB : Maybe you want to comment about this as it is probably a code done when you rewrote the lighttable ?

[dterrahe]

I think the intent was to change the cursor when over the clickable link to the documentation that gets displayed on an empty lighttable

The intent was to make the "arrows" pointing from paragraphs on the documentation screen to the modules they relate to (import, collections, styles) update if the modules get moved around/scrolled/expanded. I messed up here and don't remember why I thought testing for -1 would be a good indication that the "empty collection" text was currently being displayed (in which case redrawing the central area is quick). Can't look at it today unfortunately.

[ralfbrown]

@dterrahe, you're right, the arrows no longer update as modules are expanded and collapsed. I think I have a fix, let me give it a try.

We do still need the callback.

[ralfbrown]

Release notes:

• eliminated unnecessary GUI refreshes

[dterrahe]

I thought testing for -1 would be a good indication that the "empty collection" text was currently being displayed

It should be set to -1 here, if current collection not empty.

darktable/src/dtgtk/thumbtable.c

Line 1332 in b0d9885

table->manual_button.width = -1;

not sure why you see it being zero always. Maybe needs to be (re)initialised when changing views? I was trying to avoid (possibly costly) full checks for an empty collection for each side panel refresh that you've now had to implement...

[TurboGit]

Given this last implementation I'm more inclined to merge this in 5.0.x. Sounds pretty safe. @ralfbrown what's you view on this? (I'll test soon).

[ralfbrown]

It should be set to -1 here, if current collection not empty.

That line sets .width, the callback code was checking .x .... And I see no other code setting .width.

[ralfbrown]

I'm more inclined to merge this in 5.0.x. Sounds pretty safe. @ralfbrown what's you view on this? (I'll test soon).

Either way is fine by me, since it isn't correcting any erroneous results, just reducing wasted CPU cycles.

[dterrahe]

That line sets .width, the callback code was checking .x ....

Yep, that's an embarrassing mistake. Does changing the check to

if(darktable.gui->ui->thumbtable->manual_button.width != -1)

not solve the issue for you?

And I see no other code setting .width.

That's done in pango_layout_get_pixel_extents(layout, NULL, &lighttable->manual_button);

Anyway, I was worried that calling dt_collection_get_count at every mouse move would generate and run a potentially complex query on the full collection at every mouse move causing sluggishness, but now realise the result gets cached so the improved self-documentation and robustness is worth it. Thanks for cleaning up after me.

[ralfbrown]

Given that I've recently been working to reduce latency in GUI callbacks, I definitely made sure that the added checks were fast.