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

use shift+alt+scroll for widget resize instead of ctrl+scroll #13523

Merged
merged 4 commits into from
Feb 6, 2023

Conversation

dterrahe
Copy link
Member

@dterrahe dterrahe commented Feb 3, 2023

REQUEST FOR COMMENTS.

Please vote by adding an emoji to this first post:
❤️ HEART this is great; please merge
👍 THUMBS UP fine by me; I won't complain if merged
👎 THUMBS DOWN please no, this breaks my muscle memory

Now that it is possible to resize many widgets and graphs by dragging the lower border up or down, the old way of doing this by scrolling with the mouse wheel while holding ctrl may not be as necessary anymore and could be replaced by a slightly more awkward shift+alt+scroll for those rare occasions when we can't seem to hit the stretch spot.

image

The advantage of this is that it frees up ctrl so it can be used in the ordinary way, namely to finetune the effect of a scroll (dividing the magnitude by 10, by default).

The modules that benefit are:

  • levels (rgb): moving the levels by scroll can now by sped up and down by holding shift and ctrl respectively
  • histogram: while adjusting the exposure or black level correction (by either scrolling or dragging over the histogram) the accuracy of the adjustment can be increased by holding ctrl.
  • vectorscope: the speed of rotation can be reduced by holding ctrl (shift is for varying the width) and scrolling works over the whole area rather than just the buttons (which was the approach used to avoid conflict with resizing)
  • navigation: holding ctrl while scrolling adjusts the zoom level without boundaries (as it does over the center image)

This PR adds a couple of additional fixes to histogram;

  • resetting the exposure by double-clicking correctly updates the sliders in the module
  • the tooltip is updated for the different modes
  • exposure adjustments works in "rgb parade" as well.
  • the small jumps when switching between harmonies after recent change to angle\nharmony resolved
  • correct mode name in tooltips (fixes [FR] Change the scope title in the tooltips depending on the view. #13484)

Also fixes some bugs introduced in #13507

@dterrahe dterrahe added bugfix pull request fixing a bug priority: medium core features are degraded in a way that is still mostly usable, software stutters feature: redesign current features to rewrite scope: UI user interface and interactions documentation-pending a documentation work is required release notes: pending labels Feb 3, 2023
@dterrahe dterrahe added this to the 4.4 milestone Feb 3, 2023
@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

BTW I made an attempt to put the nine harmony buttons in a GtkOverlay to have them spread into a second column if the area height is not sufficient to contain them all. This proves not to be trivial because the size allocated to a box in an overlay does not seem to be limited by the size of the main widget in the overlay (the graph itself in this case, which is the one being resized using shift+alt+scroll or dragging) but set to its own natural size. So the buttons still just extend out of sight. It may still be possible, by connecting to the get-child-position, to force the allocation for button_box_main, but the experiment started to become tedious. For one thing,

  d->color_harmony_box = gtk_flow_box_new();
  gtk_orientable_set_orientation(GTK_ORIENTABLE(d->color_harmony_box), GTK_ORIENTATION_VERTICAL);

caused the buttons to be placed in a row, rather than the expected (?) column.

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

Some comments:

  • shift-alt-scroll does not work on navigation, well it seems to be working the first scroll then it changes the zoom level
  • over the histogram we have vectorscope (first line) in the tooltip and the acells description after, fine. When another scope is selected the first line always display histogram. IINM this was already the case before this PR, but could be fixed here. Otherwise I'll be happy to look at this in another PR.

BTW I made an attempt to put the nine harmony buttons in a GtkOverlay to have them spread into a second column if the area height is not sufficient to contain them all.

Even if this is a nice goal I'm not sure it is really blocking issue as anyway to work properly with the vectorscope and guide harmony one really need some big vectorscope for readability not a tiny one. And in this case there is plenty of room for all the guide buttons.

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

Again not part of this PR but I'm wondering if the decreased luminosity of the scope outside of the guides are not too strong. For some subtle colors it is a bit difficult to see them.

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

For last point I tried a 0.6 alpha, it seems better to me:

diff --git a/src/libs/histogram.c b/src/libs/histogram.c
index d1fa861629..9f1152a5e3 100644
--- a/src/libs/histogram.c
+++ b/src/libs/histogram.c
@@ -1168,7 +1168,7 @@ static void _lib_histogram_draw_vectorscope(dt_lib_histogram_t *d, cairo_t *cr,
       // we dim the histogram graph outside the harmony sectors
       cairo_stroke_preserve(cr);
       cairo_push_group(cr);
-      cairo_paint_with_alpha(cr, 0.3);
+      cairo_paint_with_alpha(cr, 0.6);
       cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 1.0);
       cairo_fill(cr);
       cairo_pattern_t *harmony_pat = cairo_pop_group(cr);

@dterrahe dterrahe force-pushed the resize_shift_alt_scroll branch from 7f5ae32 to 5bd0eda Compare February 4, 2023 14:25
@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

  • shift-alt-scroll does not work on navigation

I first thought this might be an interaction with #13507 but even after rebasing I can't seem to reproduce. Except when the cursor is over the dropdown. Otherwise resizing seems to consistently work for me. Both scroll handlers are attached to the area, not the overlay, so there should not be any problems. Any ideas on how to reproduce would be very welcome.

  • [tooltips] could be fixed here.

done

Even if this is a nice goal

Agreed. I just mentioned it here because I had previously suggested to @Mark-64 to try using GtkFlowBox and now just hope I haven't caused him to waste time.

@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

Thanks everybody for taking the time to vote here. It is really appreciated because changing something people have been used to for so long is scary. As far as I can tell, you are mostly the usual suspects, but I'm really hoping some from pixls will make it over here as well and take the extra time to register etc. Welcome if you just did!

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

Strange, for me shit-alt+scroll does not change navigation height but still do a zoom in/out.

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

The other issues are fixed to me. You didn't comment about the alpha & my patch above. But I can do that if you prefer.

@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

shit- alt+scroll

Ah, I see what your problem is!

No seriously, I have no idea :-( Does shift+alt+scroll work for all other graphs? This case really should be no different, unless you are over (or near) the combo. If anybody else can reproduce and has suggestions how I might as well; please! I've already tried switching to French and French keyboard. Makes no difference far as I can tell. Neither does "mouse wheel scrolls side panel".

But I can do that if you prefer.

Yes please. I'd prefer not to pollute the discussion here with an unrelated change. You may have noticed that I've asked people to come and vote here, so it would be reasonable to keep this open for a bit before merging (although the consensus seems to be pretty strong so far).

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

Does shift+alt+scroll work for all other graphs?

Yes, tested on color-zones, histogram, filmicrgb. All are changing height with the key combination.

But if I put my mouse cursor in the middle of the navigation window and do the same I just have the zoom changing.

I found that if I shift-alt-scroll-down a lot at some point the height is changing. But as soon as I do shift-alt-scroll-up I have a zoom in done. It is 100% reproducible on my side.

@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

Maybe it is smooth scrolling related? That still always confuses me. But again, should not make a difference between the different types of graphs.

@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

if I put my mouse cursor in the middle of the navigation window and do the same I just have the zoom changing

curiouser and curiouser. On Windows if I use the touch pad to scroll (with shift and alt pressed) both the widget size and the zoom level change simultaneously. Until either one reaches its limit and then only the other one continues.

I think I may have found it; testing (on Windows) and will submit fix. (this was probably already a regression in #13507)

@dterrahe
Copy link
Member Author

dterrahe commented Feb 4, 2023

Fixed, I hope, and also a problem with the navigation window not getting refreshed after zoom change.

@TurboGit
Copy link
Member

TurboGit commented Feb 4, 2023

Fixed, I hope, and also a problem with the navigation window not getting refreshed after zoom change.

Yes, works for me now!

TurboGit added a commit that referenced this pull request Feb 4, 2023
Also make this an hidden configuration option.

Part of #13523.
@TurboGit TurboGit self-requested a review February 5, 2023 07:03
@TurboGit
Copy link
Member

TurboGit commented Feb 5, 2023

I would remove the RFC from this PR now, we have 19 ❤️ :)

@dterrahe
Copy link
Member Author

dterrahe commented Feb 5, 2023

I would remove the RFC from this PR now, we have 19 ❤️ :)

idk it's only been up for a day, so if we are serious about wanting feedback, we could give everybody another day to find it here.

otoh even if somebody now shows up really disliking it, they would have to come with an even better alternative to please those 19.

But explicitly inviting people to come over here and then "disappearing" the rfc within a day or so... I'm OK with lowering the speed a bit since we're nowhere close to a release. The bugs fixed here and not too crucial.

@dterrahe dterrahe changed the title RFC use shift+alt+scroll for widget resize instead of ctrl+scroll use shift+alt+scroll for widget resize instead of ctrl+scroll Feb 6, 2023
@dterrahe
Copy link
Member Author

dterrahe commented Feb 6, 2023

I think we should be good and the flip side of waiting longer for comments is that people waste more time "reviewing" the current state in master to which this pr applies a little more polish.

@TurboGit TurboGit merged commit 0cb2028 into darktable-org:master Feb 6, 2023
@dterrahe dterrahe deleted the resize_shift_alt_scroll branch February 19, 2023 15:10
@dterrahe
Copy link
Member Author

dterrahe commented Mar 1, 2023

@TurboGit Release notes entry for this, #13496 and #13418:

  • The height of resizeable widgets and lists can now be changed by dragging their bottom. The previous method to achieve this, by scrolling while holding the control key, has been changed to shift+alt+scroll (and a note has been added to all tooltips consistently). This frees up ctrl+scroll to fine-tune changes in RGB Levels or the histogram (to change exposure or black level). In the navigator ctrl+scroll will adjust zoom level without bounds, like it would do over the central area.

@TurboGit
Copy link
Member

TurboGit commented Mar 1, 2023

Thanks!

@elstoc elstoc added documentation-complete needed documentation is merged in dtdocs and removed documentation-pending a documentation work is required labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug documentation-complete needed documentation is merged in dtdocs feature: redesign current features to rewrite priority: medium core features are degraded in a way that is still mostly usable, software stutters question scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Change the scope title in the tooltips depending on the view.
3 participants