Show active preset in lib module header #18468
Conversation
zisoft
force-pushed
the
lib-module-preset-name
branch
from
25d0718 to
4ffa84e
Compare
zisoft
force-pushed
the
lib-module-preset-name
branch
from
4ffa84e to
f03df1a
Compare
src/libs/filtering.c
Outdated
| @@ -2252,6 +2250,12 @@ void gui_init(dt_lib_module_t *self) | |||
| DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_IMAGES_ORDER_CHANGE, _dt_images_order_change); | |||
| } | |||
|
|
|||
| void gui_update(dt_lib_module_t *self) | |||
| { | |||
There was a problem hiding this comment.
Either make gui_update a DEFAULT member function in lib_api (and provide a default_ implementation) or test if module->gui_update exists before calling it. No need for empty dummy implementations everywhere.
There was a problem hiding this comment.
That was just after a quick look at the code. I can't test or fully analyze at the moment, but beware, the reason for gui_update for libs is different from gui_update for iops. The former is meant to avoid (possibly) costly updates of the content of a module if it is collapsed or otherwise not visible. For these changes, presumably you need to update the header even if the module is collapsed. If that means you are now always calling gui_update for each signal received, that may cause a more sluggish experience.
Again, I haven't analyzed your code.
There was a problem hiding this comment.
Still some fixes needed.
- The preset label on metadata & collection filter are not kept, select a preset it is displayed and then cleared just after if the module is expanded.
Note that in the console I get a huge amount of:
(darktable:364759): Gtk-CRITICAL **: 21:48:12.549: gtk_label_set_text: assertion 'GTK_IS_LABEL (label)' failed
And at some point I get this message continuously on the console.
The image information and export modules are ok.
zisoft
force-pushed
the
lib-module-preset-name
branch
from
6220cd7 to
c816fa7
Compare
There was a problem hiding this comment.
Thanks, the gtk-critical messages are fixed. Two remaining issue:
- The collection filter module doesn't keep the preset label when it is expanded, select for example Square. If collapse it keeps the label, but as soon as you expand the module the label is cleared.
- Likewise for metadata editor module
TurboGit
added
priority: low
|
@TurboGit: I am still working on this but I must admit that I'm thinking about giving up. The metadata module is the killer here, no other module refreshes its whole gui when hovering over the lighttable images. Updating the preset label in the module header works fine if the module is expanded. If the module is collapsed the gui data is not updated. This is all well designed code (why should the gui data be updated if the gui is not visible?). Updating the preset label requires to compare the module params with the presets. All expensive operations. For now I have not found a proper way to get the params for a collapsed module. However, I have fixed a bug in the metadata module which can be reproduced in master:
I have fixed that with commit 2ff0dba which can be cherry-picked for master if we drop this PR. |
I have checked this scenario and it is still not working on my side after having cherry-picked the commit. Though it works when applying this whole PR, so another part is needed. |
|
After looking at the set of commit it seems that there was no other commit that could fix the issue. So I tried again and now just using 2ff0dba works. I'll merge this. |
We have a general problem with the collection filter presets, tested in master: I'll make some analysis on that. |
|
Can we have this either use the existing preference (darkroom > automatically update module name) to toggle the feature or implement a new one for lib modules |
|
Moved for 5.4 at this stage. |
This PR adds the currently active preset to the header of lib modules, in a similar way as it is done in the iop modules:
The preset label is named
lib-module-name(analogous to the iop moduleiop-module-name) so it can be styled separately in css.closes #18430
closes #17442
closes #16964
closes #19020