Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add zoom function to slide show and write new usage prompts when entering the interface #17913

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

C-CH621
Copy link
Contributor

@C-CH621 C-CH621 commented Dec 1, 2024

Hello, I am the person who proposed to add two buttons to the slideshow interface before. After some exploration, I found that adding buttons would not make the interface as beautiful and elegant as it is now (possibly due to my limited UI design ability). Therefore, I gave up the idea of adding buttons and only added usage prompts for slideshow when entering the interface. In addition, I have added a function to zoom in and out of the projection room images, which is bound to the letter keys q and e and does not have a new write button. For my changes, my local compilation runs well without any issues and does not affect the use of the original features. I hope my contribution is accepted by you! Wishing Darktable continued success!
a0f804e3fdb110181a24d93b0be8403
815a79d1bdcacda2ddd98cc5dc79a37

@TurboGit TurboGit added this to the 5.2 milestone Dec 1, 2024
@dterrahe
Copy link
Member

dterrahe commented Dec 1, 2024

First of all; thank you for your contribution and for getting involved with development!

Would you please explain a little what the added value of zooming is in the slideshow? There is no panning functionality in that view, so one could only zoom into the center of the image. Adding panning would make the slideshow more and more similar to the lighttable in preview mode. There you have further functions that might be helpful to you, like the home/end keys to go to first and last image.

My understanding of the slideshow view is that is meant to provide an extremely simple interface where the chance of accidentally pressing a wrong button and getting into a state that a novice wouldn't know how to get out of is very limited. And of course it has automatic timed progression (which possibly is also not that useful if you occasionally want to zoom into a detail).

In reviewing the code in this PR, I believe that locking/unlocking the mutex is not needed because both the shortcut handling and the expose function are running in the ui thread.

More fundamentally, instead of implementing specific (and limited) functions to zoom in or out, you could consider creating a (hidden) slider that would allow much more flexibility in setting up shortcuts. You could then assign one key that in combination with the mouse wheel zooms in or out, adjust the speed of the shortcut, or in combination with a double click reset to 100%. Just pressing the key would give you the normal slider precision adjustment popup, where you could type in a zoom percentage. This slider could also be accessed from lua to directly set a specific percentage or read the currently active one.

The code for this would be

  dt_slideshow_t *d = self->data;
  GtkWidget *zoom = dt_bauhaus_slider_new_action(DT_ACTION(self), .1, 10, .3, 1, 2);
  ac = dt_bauhaus_widget_set_label(zoom, NULL, _("zoom"));
  dt_bauhaus_slider_set_format(zoom, "%");
  dt_bauhaus_slider_set_log_curve(zoom);
  dt_bauhaus_widget_set_field(zoom, &d->zoom_level, DT_INTROSPECTION_TYPE_FLOAT);
  g_signal_connect(zoom, "value-changed", G_CALLBACK(dt_control_queue_redraw_center), NULL);

  dt_shortcut_register(ac, 0, DT_ACTION_EFFECT_UP, GDK_KEY_q, 0);
  dt_shortcut_register(ac, 0, DT_ACTION_EFFECT_DOWN, GDK_KEY_e, 0);

(replacing your dt_action_register calls so that the default shortcuts don't conflict).

This would require the zoom_level to be a float (bauhaus sliders don't support doubles yet).
And, really, an additional zoom widget variable so it can be correctly destroyed in cleanup.
Also #include "bauhaus/bauhaus.h".
And in accelerators.c in the function dt_action_widget_invisible the addition of a line
if(!p) return FALSE;
immediately after
GtkWidget *p = gtk_widget_get_parent(w);
so that this unattached ("virtual") slider does not get ignored.

@C-CH621
Copy link
Contributor Author

C-CH621 commented Dec 2, 2024

Thank you very much for your suggestion!
I completely agree with your idea of adding zoom in and out functions in a slideshow not to see more details of the image, but rather to display the entire image on the entire screen as much as possible. I think this approach creates a sense of "oppression", especially when used to view "horizontal" images, which occupy every corner of the screen and give people a feeling of "nowhere to escape". Of course, this may be related to my personal usage habits, leaving a little border is meaningful. So the zoom in and zoom out functions are actually adjusting the size of the entire space occupied by the image playback, rather than observing the details of the image. Since I wasn't sure if everyone would like to have borders like me, I didn't fundamentally change its rendering range, but instead added two buttons. And the reason why I didn't bind these two functions to the scroll wheel is because I think some people may think that images need to be zoomed in and out to see the details of the image in the future. At this time, using Ctrl+scroll wheel with horizontal movement of the image is very appropriate. So I don't want to occupy the scroll wheel on this interface to achieve the effect of adjustment.
Regarding the somewhat unnecessary design in the code you mentioned, it may be due to my limited programming skills, resulting in imperfect code.
Thank you again for your guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants