Add option to invert the behavior of mouse scroll up/down. #10241

TurboGit · 2021-10-21T17:11:45Z

No description provided.

elstoc · 2021-10-21T17:27:32Z

data/darktableconfig.xml.in

    <longdescription>sets level for smoothing of brush strokes. stronger smoothing leads to less nodes and easier editing but with lower control of accuracy.</longdescription>
  </dtconfig>
+  <dtconfig prefs="darkroom" section="general">
+    <name>masks_updown</name>


this could probably have a clearer name - based on the description, how about masks_scroll_up_increases or something

elstoc · 2021-10-21T17:28:18Z

src/develop/imageop_gui.c

  return button;
 }

+int dt_mouse_scroll_updown(int up)


similarly name this dt_mask_scroll_up_increases?

AlicVB

completely agree !

just one small point about default value...

as the original author of all this, I remember that I disagreed with that (up to decrease) but some devs were absolutely against doing it the other way... Sometimes you have to make concession ;)

AlicVB · 2021-10-21T17:54:30Z

data/darktableconfig.xml.in

+  <dtconfig prefs="darkroom" section="general">
+    <name>masks_updown</name>
+    <type>bool</type>
+    <default>false</default>


ideally, I would have done it this way :

for "old" user (those with darktablerc already in place => default to false in order to not break habits.

for "new" users (those without darktablerc) => default to true, which is way better imho

not sure if it's easy to do, thought... maybe we can set it to true at the same place we fill the performance prefs at first run ?

Yeah I'm not sure if we can do this either. Much as I think up=up is the best, we should default to the status quo if we can't be smarter than that.

Status quo is wrong and also inconsistent with pretty much any desktop environment and with dt's sliders, which use scroll up to increase values.

Almost impossible because at the time the darktableconfig.xml is loaded the missing keys are created. So to achieve that we will need many changes into the loaded at different stages.

Much as I think up=up is the best

Now version doing just that and current users can revert to previous behavior. Is that ok for everyone ?

hmm... I've done a quick test, and it seems that if in control/conf.c:487 you write something like

if(defaults) { dt_configure_performance(); dt_conf_set_bool("masks_updown", TRUE); }

that do the job... but maybe this has some unexpected drawbacks... I don't know this part of code very well...

Now version doing just that and current users can revert to previous behavior. Is that ok for everyone ?

oops, sorry, I've not seen this msg... for me that's perfect indeed (and simpler than my proposal)

@TurboGit : sorry to be annoying but with your last change, I think we need to change the pref names and descriptions... or maybe better to default it to true and inverse the routine in imageop_gui.c

@AlicVB on reflection I think you might be right. It should be "select to do something non-standard". So the preference should be named "scroll down to increase mask parameters" and should default to false.

TurboGit · 2021-10-21T18:32:46Z

New version just pushed with the proposed changes.

elstoc · 2021-10-21T19:14:52Z

A few things

I think you have the logic inverted. If I tick the "up increases" preference then scrolling up decreases and vice versa.
The scroll logic for creating a shape and editing a shape are different. I'm not sure you've captured all instances. For example, changing opacity of ellipses and circles appears to not work as per the preference.
I think the effect on gradient curvature should be reversed. In up=up mode I would expect (in the default horizontal placement) for the ends of the gradient line to raise when I scroll up
In the preference you should put a brief description in the prefs window (scroll up to increase mask parameters) and list everything impacted in the tooltip.

Edit: see my reply to @AlicVB above - I think point 1 is correct but the preference should be renamed (down to increase).

TurboGit · 2021-10-22T06:56:03Z

@elstoc :

should be fixed
seems working now
done indeed
should be done also

And as we are at it:

should we invert the liquify ctrl-scrool for rotating the strength when placing a node ?
also for the brush should we change the behavior (that I have never really liked) where scroll change the hardness and not the size. All shapes are resized with a simple scroll up/up, why brush should be different ? So I propose to change the size with scroll and hardness with shift-scroll. How does that feel?

elstoc · 2021-10-22T08:41:22Z

The option is still not working when changing opacity before placing a shape on the canvas - ctrl+scrolling up decreases opacity regardless of the selected option.

The tooltip on the option should state something like "when using the mouse scroll wheel to change mask parameters, scroll down to increase the mask size, feather size, opacity, brush hardness and gradient curvature\nby default scrolling up increases these parameters"

I don't really have an opinion on your two questions (I never entirely understood what hardness was and how it differs from size for brushes, and I don't use liquify)

TurboGit · 2021-10-22T09:54:16Z

The option is still not working when changing opacity before placing a shape on the canvas - ctrl+scrolling up decreases opacity regardless of the selected option.

That's one of the thing I did not test :) Will fix.

TurboGit · 2021-10-22T09:56:34Z

and I don't use liquify

The strength vector is rotated counter clockwise when scrolling up currently. And I'm wondering if it shouldn't be the opposite.

TurboGit · 2021-10-22T11:21:04Z

All changes done now. Plus I have changed the brush handling as it was not consistent with other shapes and more importantly it was not consistent with itself has the handling was inverted in creation or editing mode!

Last, I have inverted the liquify rotation to be more natural (at least to me). Let's see if this is ok with others.

TurboGit · 2021-10-22T16:22:54Z

@AlicVB : I'd like to have you feedback on the brush + liquify rotation.

AlicVB · 2021-10-22T19:06:48Z

@TurboGit : indeed, I prefer how the liquify rotation works now : up=clockwise let's just hope it's not too culturally dependent... (let's not count mathematics as a culture for this case ;)

for the brush I prefer that too... some small points :

we need to update the help message
if we want to be picky, we are still not completely consistent with how other forms works : in path, shift-scroll define the feather size (the main form stay the same) in brush it's the hardness (the whole form stay the same, the central part change it's size... If we want to make them works the same way, we have to change the brush, as it would be really complex to change path (I'm even not sure it's possible)

If we want to implement point 2., I can have a look if you want, but tbh I'm also ok with the current "inconsistency"

elstoc · 2021-10-22T20:39:09Z

One more thing I hadn't spotted before... You can change curvature of a gradient mask before placing it on the canvas. This currently operates the opposite way round to when it's changed after placement.

At the same time all checks have been changed to be "positive" and make the conditional easier to read. This is especially important since the default behavior is now scroll-up = increase mask parameters.

In creation mode (brush not yet placed) the scoll was changing the size and the shift-scroll was changing the hardness. For brush already on canvas, the actions were inverted. This makes the brush behave consitently in creation or editing mode. This also is more consistent with all other shapes.

Seems more natural with other shapes parameters changes.

TurboGit · 2021-10-23T06:17:11Z

2\. as it would be really complex to change path (I'm even not sure it's possible)

Not only the path but we would need to change all other forms (circle, ellipse...)

I can have a look if you want, but tbh I'm also ok with the current "inconsistency"

@AlicVB Thanks, I let you look at this.

For my part this PR is ready to merge I have fixed the curvature inconsistency reported by @elstoc and the tooltip you have report.

elstoc

All good for me now

TurboGit · 2021-10-23T11:53:07Z

So merged ! Thanks for all the reviews.

TurboGit added this to the 3.8 milestone Oct 21, 2021

TurboGit added documentation-pending a documentation work is required feature: enhancement current features to improve release notes: pending scope: UI user interface and interactions labels Oct 21, 2021

TurboGit requested review from aurelienpierre and elstoc October 21, 2021 17:12

TurboGit force-pushed the po/masks-up-down branch from 843b157 to 22bee17 Compare October 21, 2021 17:18

elstoc reviewed Oct 21, 2021

View reviewed changes

AlicVB approved these changes Oct 21, 2021

View reviewed changes

TurboGit force-pushed the po/masks-up-down branch from 22bee17 to df51e50 Compare October 21, 2021 18:32

TurboGit force-pushed the po/masks-up-down branch from df51e50 to 3f90233 Compare October 21, 2021 18:35

TurboGit force-pushed the po/masks-up-down branch 2 times, most recently from 30d2db7 to d89cb1f Compare October 22, 2021 06:49

TurboGit force-pushed the po/masks-up-down branch from d89cb1f to 3c85705 Compare October 22, 2021 11:09

TurboGit added 3 commits October 23, 2021 08:12

Add option to invert the behavior of mouse scroll up/down.

d48e8ad

At the same time all checks have been changed to be "positive" and make the conditional easier to read. This is especially important since the default behavior is now scroll-up = increase mask parameters.

liquify: invert rotation direction on strength vector.

345b036

Seems more natural with other shapes parameters changes.

TurboGit force-pushed the po/masks-up-down branch from 3a3cef3 to 345b036 Compare October 23, 2021 06:13

elstoc approved these changes Oct 23, 2021

View reviewed changes

TurboGit merged commit 841fcfd into darktable-org:master Oct 23, 2021

TurboGit deleted the po/masks-up-down branch October 23, 2021 11:56

elstoc added documentation-complete needed documentation is merged in dtdocs and removed documentation-pending a documentation work is required labels Oct 30, 2021

TurboGit removed the release notes: pending label Oct 31, 2021

Add option to invert the behavior of mouse scroll up/down. #10241

Add option to invert the behavior of mouse scroll up/down. #10241

Uh oh!

Conversation

TurboGit commented Oct 21, 2021

Uh oh!

elstoc Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elstoc Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlicVB left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TurboGit commented Oct 21, 2021

Uh oh!

elstoc commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboGit commented Oct 22, 2021

Uh oh!

elstoc commented Oct 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboGit commented Oct 22, 2021

Uh oh!

TurboGit commented Oct 22, 2021

Uh oh!

TurboGit commented Oct 22, 2021

Uh oh!

TurboGit commented Oct 22, 2021

Uh oh!

AlicVB commented Oct 22, 2021

Uh oh!

elstoc commented Oct 22, 2021

Uh oh!

TurboGit commented Oct 23, 2021

Uh oh!

elstoc left a comment

Choose a reason for hiding this comment

Uh oh!

TurboGit commented Oct 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

elstoc Oct 21, 2021 •

edited

Loading

elstoc Oct 21, 2021 •

edited

Loading

elstoc commented Oct 21, 2021 •

edited

Loading

elstoc commented Oct 22, 2021 •

edited

Loading