From d71cd9d277b107e6796441a17f0500ce32943dba Mon Sep 17 00:00:00 2001 From: Jehan Date: Sun, 24 Sep 2023 23:39:58 +0200 Subject: [PATCH] Issue #9604: "view-zoom-*" actions should not be radio actions. A first attempt at fixing this was going through the idea of changing the concept of radio actions, such as allowing an active action in an action group to be called again. Or having some action in the radio group which can be called but never set active. But in fact, I just realized that these zoom actions are simply not meant to be radio actions. They are not stateful actions, nor are they exhaustive. I also updated the `other_scale` storage logic. Instead of updating it each time the zoom changed (which was even broken in some cases, like when changing zoom through another action), I simply save the last custom zoom value. This is the one which is reused across calls. I don't think always resetting to the current zoom value is very useful (if you call this dialog, it's not to zoom to the current zoom!). Also there was some concept of flagging this stored zoom value as "dirty" by making it negative, but this was never actually used, which makes me believe that current logic was not the original intent anyway. Saving the previous custom zoom explicitly set seems to be a good enough behavior, so let's go with it. --- app/actions/view-actions.c | 118 ++++++++------------ app/actions/view-commands.c | 17 +-- app/actions/view-commands.h | 4 - app/display/gimpdisplayshell-scale-dialog.c | 51 +++++++-- app/display/gimpdisplayshell.h | 2 - 5 files changed, 88 insertions(+), 104 deletions(-) diff --git a/app/actions/view-actions.c b/app/actions/view-actions.c index 052096cd3a..4bbf23711b 100644 --- a/app/actions/view-actions.c +++ b/app/actions/view-actions.c @@ -367,66 +367,86 @@ static const GimpEnumActionEntry view_zoom_actions[] = GIMP_HELP_VIEW_ZOOM_IN } }; -static const GimpRadioActionEntry view_zoom_explicit_actions[] = +static const GimpEnumActionEntry view_zoom_explicit_actions[] = { { "view-zoom-16-1", NULL, - NC_("view-zoom-action", "1_6:1 (1600%)"), NULL, { "5", "KP_5", NULL }, + NC_("view-zoom-action", "Zoom 16:1 (1600%)"), + NC_("view-zoom-action", "1_6:1 (1600%)"), + { "5", "KP_5", NULL }, NC_("view-zoom-action", "Zoom 16:1"), - 160000, + 160000, FALSE, GIMP_HELP_VIEW_ZOOM_IN }, { "view-zoom-8-1", NULL, - NC_("view-zoom-action", "_8:1 (800%)"), NULL, { "4", "KP_4", NULL }, + NC_("view-zoom-action", "Zoom 8:1 (800%)"), + NC_("view-zoom-action", "_8:1 (800%)"), + { "4", "KP_4", NULL }, NC_("view-zoom-action", "Zoom 8:1"), - 80000, + 80000, FALSE, GIMP_HELP_VIEW_ZOOM_IN }, { "view-zoom-4-1", NULL, - NC_("view-zoom-action", "_4:1 (400%)"), NULL, { "3", "KP_3", NULL }, + NC_("view-zoom-action", "Zoom 4:1 (400%)"), + NC_("view-zoom-action", "_4:1 (400%)"), + { "3", "KP_3", NULL }, NC_("view-zoom-action", "Zoom 4:1"), - 40000, + 40000, FALSE, GIMP_HELP_VIEW_ZOOM_IN }, { "view-zoom-2-1", NULL, - NC_("view-zoom-action", "_2:1 (200%)"), NULL, { "2", "KP_2", NULL }, + NC_("view-zoom-action", "Zoom 2:1 (200%)"), + NC_("view-zoom-action", "_2:1 (200%)"), + { "2", "KP_2", NULL }, NC_("view-zoom-action", "Zoom 2:1"), - 20000, + 20000, FALSE, GIMP_HELP_VIEW_ZOOM_IN }, { "view-zoom-1-1", GIMP_ICON_ZOOM_ORIGINAL, - NC_("view-zoom-action", "_1:1 (100%)"), NULL, { "1", "KP_1", NULL }, + NC_("view-zoom-action", "Zoom 1:1 (100%)"), + NC_("view-zoom-action", "_1:1 (100%)"), + { "1", "KP_1", NULL }, NC_("view-zoom-action", "Zoom 1:1"), - 10000, + 10000, FALSE, GIMP_HELP_VIEW_ZOOM_100 }, { "view-zoom-1-2", NULL, - NC_("view-zoom-action", "1:_2 (50%)"), NULL, { "2", "KP_2", NULL }, + NC_("view-zoom-action", "Zoom 1:2 (50%)"), + NC_("view-zoom-action", "1:_2 (50%)"), + { "2", "KP_2", NULL }, NC_("view-zoom-action", "Zoom 1:2"), - 5000, + 5000, FALSE, GIMP_HELP_VIEW_ZOOM_OUT }, { "view-zoom-1-4", NULL, - NC_("view-zoom-action", "1:_4 (25%)"), NULL, { "3", "KP_3", NULL }, + NC_("view-zoom-action", "Zoom 1:4 (25%)"), + NC_("view-zoom-action", "1:_4 (25%)"), + { "3", "KP_3", NULL }, NC_("view-zoom-action", "Zoom 1:4"), - 2500, + 2500, FALSE, GIMP_HELP_VIEW_ZOOM_OUT }, { "view-zoom-1-8", NULL, - NC_("view-zoom-action", "1:_8 (12.5%)"), NULL, { "4", "KP_4", NULL }, + NC_("view-zoom-action", "Zoom 1:8 (12.5%)"), + NC_("view-zoom-action", "1:_8 (12.5%)"), + { "4", "KP_4", NULL }, NC_("view-zoom-action", "Zoom 1:8"), - 1250, + 1250, FALSE, GIMP_HELP_VIEW_ZOOM_OUT }, { "view-zoom-1-16", NULL, - NC_("view-zoom-action", "1:1_6 (6.25%)"), NULL, { "5", "KP_5", NULL }, + NC_("view-zoom-action", "Zoom 1:16 (6.25%)"), + NC_("view-zoom-action", "1:1_6 (6.25%)"), + { "5", "KP_5", NULL }, NC_("view-zoom-action", "Zoom 1:16"), - 625, + 625, FALSE, GIMP_HELP_VIEW_ZOOM_OUT }, { "view-zoom-other", NULL, - NC_("view-zoom-action", "Othe_r zoom factor..."), NULL, { NULL }, NC_("view-zoom-action", "Set a custom zoom factor"), - 0, + NC_("view-zoom-action", "Othe_r zoom factor..."), + { NULL }, + NC_("view-zoom-action", "Set a custom zoom factor"), + 0, TRUE, GIMP_HELP_VIEW_ZOOM_OTHER } }; @@ -648,8 +668,6 @@ static const GimpEnumActionEntry view_scroll_vertical_actions[] = void view_actions_setup (GimpActionGroup *group) { - GimpAction *action; - gimp_action_group_add_actions (group, "view-action", view_actions, G_N_ELEMENTS (view_actions)); @@ -663,12 +681,10 @@ view_actions_setup (GimpActionGroup *group) G_N_ELEMENTS (view_zoom_actions), view_zoom_cmd_callback); - gimp_action_group_add_radio_actions (group, "view-zoom-action", - view_zoom_explicit_actions, - G_N_ELEMENTS (view_zoom_explicit_actions), - NULL, - 10000, - view_zoom_explicit_cmd_callback); + gimp_action_group_add_enum_actions (group, "view-zoom-action", + view_zoom_explicit_actions, + G_N_ELEMENTS (view_zoom_explicit_actions), + view_zoom_explicit_cmd_callback); gimp_action_group_add_toggle_actions (group, "view-action", view_flip_actions, @@ -710,15 +726,6 @@ view_actions_setup (GimpActionGroup *group) G_N_ELEMENTS (view_scroll_vertical_actions), view_scroll_vertical_cmd_callback); - /* connect "activate" of view-zoom-other manually so it can be - * selected even if it's the active item of the radio group - */ - action = gimp_action_group_get_action (group, "view-zoom-other"); - - g_signal_connect (action, "activate", - G_CALLBACK (view_zoom_other_cmd_callback), - group->user_data); - g_signal_connect_object (group->gimp->config, "notify::check-type", G_CALLBACK (view_actions_check_type_notify), group, 0); @@ -983,55 +990,20 @@ static void view_actions_set_zoom (GimpActionGroup *group, GimpDisplayShell *shell) { - const gchar *action = NULL; GimpImageWindow *window; GimpMenuModel *model; gchar *str; gchar *label; - guint scale; g_object_get (shell->zoom, "percentage", &str, NULL); - scale = ROUND (gimp_zoom_model_get_factor (shell->zoom) * 1000); - - switch (scale) - { - case 16000: action = "view-zoom-16-1"; break; - case 8000: action = "view-zoom-8-1"; break; - case 4000: action = "view-zoom-4-1"; break; - case 2000: action = "view-zoom-2-1"; break; - case 1000: action = "view-zoom-1-1"; break; - case 500: action = "view-zoom-1-2"; break; - case 250: action = "view-zoom-1-4"; break; - case 125: action = "view-zoom-1-8"; break; - case 63: - case 62: action = "view-zoom-1-16"; break; - } - - if (! action) - { - action = "view-zoom-other"; - - label = g_strdup_printf (_("Othe_r (%s)..."), str); - gimp_action_group_set_action_label (group, action, label); - g_free (label); - - shell->other_scale = gimp_zoom_model_get_factor (shell->zoom); - } - - gimp_action_group_set_action_active (group, action, TRUE); - window = gimp_display_shell_get_window (shell); model = gimp_image_window_get_menubar_model (window); label = g_strdup_printf (_("_Zoom (%s)"), str); gimp_menu_model_set_title (model, "/View/Zoom", label); g_free (label); - - /* flag as dirty */ - shell->other_scale = - fabs (shell->other_scale); - g_free (str); } diff --git a/app/actions/view-commands.c b/app/actions/view-commands.c index 88bf0f569b..da3863e752 100644 --- a/app/actions/view-commands.c +++ b/app/actions/view-commands.c @@ -284,22 +284,7 @@ view_zoom_explicit_cmd_callback (GimpAction *action, (gdouble) factor / 10000, GIMP_ZOOM_FOCUS_RETAIN_CENTERING_ELSE_BEST_GUESS); } -} - -/* not a GimpActionCallback */ -void -view_zoom_other_cmd_callback (GimpAction *action, - gpointer data) -{ - GimpDisplayShell *shell; - return_if_no_shell (shell, data); - - /* check if we are activated by the user or from - * view_actions_set_zoom(), also this is really a GtkToggleAction - * NOT a GimpToggleAction - */ - if (gimp_toggle_action_get_active (GIMP_TOGGLE_ACTION (action)) && - shell->other_scale != gimp_zoom_model_get_factor (shell->zoom)) + else { gimp_display_shell_scale_dialog (shell); } diff --git a/app/actions/view-commands.h b/app/actions/view-commands.h index 6ade1b55e7..c7c7e930e3 100644 --- a/app/actions/view-commands.h +++ b/app/actions/view-commands.h @@ -49,10 +49,6 @@ void view_zoom_explicit_cmd_callback (GimpAction *action, GVariant *value, gpointer data); -/* not a GimpActionCallback */ -void view_zoom_other_cmd_callback (GimpAction *action, - gpointer data); - void view_show_all_cmd_callback (GimpAction *action, GVariant *value, gpointer data); diff --git a/app/display/gimpdisplayshell-scale-dialog.c b/app/display/gimpdisplayshell-scale-dialog.c index a3bf926c45..87f8b29977 100644 --- a/app/display/gimpdisplayshell-scale-dialog.c +++ b/app/display/gimpdisplayshell-scale-dialog.c @@ -28,6 +28,7 @@ #include "core/gimp.h" #include "core/gimpviewable.h" +#include "widgets/gimpaction.h" #include "widgets/gimphelp-ids.h" #include "widgets/gimpviewabledialog.h" @@ -50,6 +51,9 @@ typedef struct GtkAdjustment *scale_adj; GtkAdjustment *num_adj; GtkAdjustment *denom_adj; + + gdouble prev_scale; + gdouble *other_scale; } ScaleDialogData; @@ -77,6 +81,8 @@ static void update_zoom_values (GtkAdjustment *adj, void gimp_display_shell_scale_dialog (GimpDisplayShell *shell) { + /* scale factor entered in Zoom->Other*/ + static gdouble other_scale = 0.0; ScaleDialogData *data; GimpImage *image; GtkWidget *toplevel; @@ -94,19 +100,21 @@ gimp_display_shell_scale_dialog (GimpDisplayShell *shell) return; } - if (SCALE_EQUALS (shell->other_scale, 0.0)) + data = g_slice_new (ScaleDialogData); + data->prev_scale = other_scale; + data->other_scale = &other_scale; + + if (SCALE_EQUALS (other_scale, 0.0)) { /* other_scale not yet initialized */ - shell->other_scale = gimp_zoom_model_get_factor (shell->zoom); + other_scale = gimp_zoom_model_get_factor (shell->zoom); } image = gimp_display_get_image (shell->display); - data = g_slice_new (ScaleDialogData); - data->shell = shell; data->model = g_object_new (GIMP_TYPE_ZOOM_MODEL, - "value", fabs (shell->other_scale), + "value", fabs (other_scale), NULL); g_set_weak_pointer @@ -185,7 +193,7 @@ gimp_display_shell_scale_dialog (GimpDisplayShell *shell) _("Zoom:"), 0.0, 0.5, hbox, 1); - data->scale_adj = gtk_adjustment_new (fabs (shell->other_scale) * 100, + data->scale_adj = gtk_adjustment_new (other_scale * 100, 100.0 / 256.0, 25600.0, 10, 50, 0); spin = gimp_spin_button_new (data->scale_adj, 1.0, 2); @@ -215,7 +223,10 @@ gimp_display_shell_scale_dialog_response (GtkWidget *widget, { if (response_id == GTK_RESPONSE_OK) { - gdouble scale; + GAction *action; + gchar *label; + gchar *zoom_str; + gdouble scale; scale = gtk_adjustment_get_value (dialog->scale_adj); @@ -223,14 +234,32 @@ gimp_display_shell_scale_dialog_response (GtkWidget *widget, GIMP_ZOOM_TO, scale / 100.0, GIMP_ZOOM_FOCUS_BEST_GUESS); + + g_object_get (dialog->shell->zoom, + "percentage", &zoom_str, + NULL); + + /* Change the "view-zoom-other" label. */ + action = g_action_map_lookup_action (G_ACTION_MAP (dialog->shell->display->gimp->app), + "view-zoom-other"); + + label = g_strdup_printf (_("Othe_r (%s)..."), zoom_str); + gimp_action_set_short_label (GIMP_ACTION (action), label); + g_free (label); + + label = g_strdup_printf (_("Custom Zoom (%s)..."), zoom_str); + gimp_action_set_label (GIMP_ACTION (action), label); + g_free (label); + + g_free (zoom_str); } else { /* need to emit "scaled" to get the menu updated */ gimp_display_shell_scaled (dialog->shell); - } - dialog->shell->other_scale = - fabs (dialog->shell->other_scale); + *(dialog->other_scale) = dialog->prev_scale; + } gtk_widget_destroy (dialog->shell->scale_dialog); } @@ -267,6 +296,8 @@ update_zoom_values (GtkAdjustment *adj, gtk_adjustment_set_value (dialog->num_adj, num); gtk_adjustment_set_value (dialog->denom_adj, denom); + + *(dialog->other_scale) = scale / 100.0; } else /* fraction adjustments */ { @@ -274,6 +305,8 @@ update_zoom_values (GtkAdjustment *adj, gtk_adjustment_get_value (dialog->denom_adj)); gtk_adjustment_set_value (dialog->scale_adj, scale * 100); + + *(dialog->other_scale) = scale; } g_signal_handlers_unblock_by_func (dialog->scale_adj, diff --git a/app/display/gimpdisplayshell.h b/app/display/gimpdisplayshell.h index f4c404a644..e9786a0dbf 100644 --- a/app/display/gimpdisplayshell.h +++ b/app/display/gimpdisplayshell.h @@ -83,8 +83,6 @@ struct _GimpDisplayShell gint last_offset_x; /* offsets used when reverting zoom */ gint last_offset_y; - gdouble other_scale; /* scale factor entered in Zoom->Other*/ - gint disp_width; /* width of drawing area */ gint disp_height; /* height of drawing area */