Activate 100% zoom limit again by da-phil · Pull Request #21031 · darktable-org/darktable

da-phil · 2026-05-15T00:07:18Z

Activate 100% scroll limit again and also make scroll limit configurable through config parameter darkroom/ui/constrain_zoom, which is TRUE by default, keeping the previous
behavior:

zooming in is limited to 100%
zooming out is limited to screen-fit

This is an effort to satisfy both use-cases discussed in #21027.

Disclaimer: this work was co-created with Claude.

da-phil · 2026-05-15T08:42:05Z

It would be great to receive a review from @dterrahe, as he wrote most of the GTK touchpad / mouse input code and introduced the constrain flag.

I'm not sure if we use the constrain flag in the right way which was intended. My understanding was that it constrains the zoom levels in the following way:

limit zoom to "fit to screen" when zooming out
limit zoom to 100% when zooming in

Please correct me if this assumption was wrong.

da-phil · 2026-05-16T13:03:17Z

@TurboGit let's wait merging this PR, I wanted to know if there is a simpler solution to re-activating the zoom limit, see message: #21027 (comment)

I'm not very happy with the current approach in this PR, which is a kind of a hack to limit zooming. It does not work well, when you do the following:

zoom in to 100%
press CTRL and zoom in one more step to >100%
try to zoom in again -> you'll then fall back to 100% again

I believe only reverting #21011 will bring back the old behavior.

CC: @masterpiga

da-phil · 2026-05-16T21:10:39Z

I pushed a proper fix by revering #21011 and reverting my previous approach of introducing a hard limit and break the soft-steps.
Now it also works properly for pinch-zooming.

The more I deal with this functionality and code, the less I like it. But let's keep this well-known behavior for release 5.6 and keep the discussion in #21027 going ;)
If we would change it, we might also need to update some docs over at https://github.com/darktable-org/dtdocs.

jenshannoschwalm · 2026-05-18T08:28:37Z

I just checked latest version of this PR and for me it works!
If this is ok for @TurboGit to be merged i would prefer the commits to be squashed so one commit for possibly need to bisect.
I think the darkroom/ui/constrain_zoom misses a restart hint.
We now have (too) many options in misc->interface. Maybe a new tab would be preferable including all thouse mouse behaviour related stuff? "Mouse interface" maybe?

da-phil · 2026-05-18T08:49:43Z

If this is ok for @TurboGit to be merged i would prefer the commits to be squashed so one commit for possibly need to bisect.

Works for me, will do.

I think the darkroom/ui/constrain_zoom misses a restart hint.

Ah, I see, I didn't pay attention to this little detail in the data/darktableconfig.xml.in file. Will add it then.

We now have (too) many options in misc->interface. Maybe a new tab would be preferable including all thouse mouse behaviour related stuff? "Mouse interface" maybe?

Also sounds good, but I think this should be scoped out for this PR into a separate one, right?

TurboGit · 2026-05-18T08:56:57Z

@da-phil : Do not squash the revert though.

jenshannoschwalm · 2026-05-18T09:19:42Z

Also sounds good, but I think this should be scoped out for this PR into a separate one, right?

yup.

This reverts commit b04e36e.

* make scroll limit configurable through config parameter `darkroom/ui/constrain_zoom`, which is TRUE by default, keeping the previous behavior. * refactored the soft-step zooming logic in order to re-use it for pinch-zoom gestures.

da-phil · 2026-05-18T09:58:38Z

@TurboGit is the PR tag "default-behavior-change" even correct, given that we keep the same behavior as in the previous release?

da-phil · 2026-05-19T12:48:10Z

@TurboGit @jenshannoschwalm I pushed a new commit with the DT_SIGNAL_PREFERENCES_CHANGE approach, however it does not seem to work for me, can you help me pointing out the issue?
The new callbacks are never called, no matter how often I change both params.

TurboGit · 2026-05-19T15:27:00Z

Let me have a look...

TurboGit · 2026-05-19T16:06:13Z

  // connect to preference change for module header button hiding
  DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_PREFERENCES_CHANGE, _preference_changed_button_hide);
+  // keep cached constrain_zoom flag in sync with runtime preference changes
+  DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_PREFERENCES_CHANGE, _preference_changed_constrain_zoom);


You need a DT_CONTROL_SIGNAL_CONNECT and passing as third argument &darktable or DT_CONTROL_SIGNAL_HANDLE as above but in _preference_changed_constrain_zoom use darktable.dev->....

If the later solution is used, it probably make sense to use _preference_changed_button_hide as callback. Just renaming it to something more generic : _preference_changed.

Thanks, I probably just didn't fully understand how it works and that those settings are already part of the preferences menu.
Please have a look again.

We can simplify this... As outlined above a single _preference_changed callback can be used. How it works:

After editing any preferences the signal DT_SIGNAL_PREFERENCES_CHANGE is emitted.

We don't know what has been changed, but we need to reset all the needed preferences to their new value.

So we connect to DT_SIGNAL_PREFERENCES_CHANGE in darkroom.c (a single callback, a single connect of you prefer).

Inside this callback we read all the used preferences and we set the corresponding values in darktable global struct, the pattern is darktable.develop-><FIELD> = dt_conf_get_<TYPE>(<KEY>);.

Let me know if this clarifies things and if the changes to be done is clear to you. TIA.

Yes, I thought so too, but that very callback is called twice every time, because it is registered for 2 color intent widgets. I didn't want to also completely refactor this code. But I agree that ideally we should only have one _preference_changed callback per module, as we have way to many unnecessary callbacks all over the place, as also outlined in my comment below:
#21031 (comment)

I'd prefer a separate PR to clean up this and do it properly. But if you think it's fine to extend the scope of this PR, I can already do it in this PR. Your call 😉

I'd prefer a separate PR to clean up this and do it properly.

I'd prefer in this PR because to me the complexity added here should not be and we can do a clean implementation directly in this PR. Just adding the setting in a current DT_SIGNAL_PREFERENCES_CHANGE callback (just renaming it to be more generic). If I did not missed something it should be as simple as done currently and clean without the need for another PR.

Alrighty, will do once I'm back at my computer 👌

da-phil · 2026-05-19T22:44:39Z

4. We now have (too) many options in misc->interface. Maybe a new tab would be preferable including all thouse mouse behaviour related stuff? "Mouse interface" maybe?

After another look and knowing how it works I fully agree and would propose the following steps:

if interface related settings are only relevant for one DT module (e.g. lighttable / darkroom) move those settings inside this respective menu (e.g. prefs="lighttable" section="interface" and prefs="darkroom section="interface")
consolidate config callbacks a little bit, I think we have way to many callbacks flying around everywhere

wpferguson · 2026-05-19T23:32:15Z

After another look and knowing how it works I fully agree and would propose the following steps:

if interface related settings are only relevant for one DT module (e.g. lighttable / darkroom) move those settings inside this respective menu (e.g. prefs="lighttable" section="interface" and prefs="darkroom section="interface")
consolidate config callbacks a little bit, I think we have way to many callbacks flying around everywhere

How about we make that a separate PR? PRs that grow in scope tend to not end well.

EDIT: Also, if we are talking about messing with callbacks this should probably be for 5.8 and not something we try and shoehorn into 5.6.

da-phil · 2026-05-20T18:41:43Z

After another look and knowing how it works I fully agree and would propose the following steps:
if interface related settings are only relevant for one DT module (e.g. lighttable / darkroom) move those settings inside this respective menu (e.g. prefs="lighttable" section="interface" and prefs="darkroom section="interface")
consolidate config callbacks a little bit, I think we have way to many callbacks flying around everywhere
How about we make that a separate PR? PRs that grow in scope tend to not end well.

EDIT: Also, if we are talking about messing with callbacks this should probably be for 5.8 and not something we try and shoehorn into 5.6.

I never said this should be part of this PR, as I agree with you about not blowing up the scope of this PR, it seems @TurboGit is in favour of doing it though: #21031 (comment)

I'm fine either way but would also prefer a separate PR.

TurboGit · 2026-05-20T19:16:06Z

I never said this should be part of this PR, as I agree with you about not blowing up the scope of this PR, it seems @TurboGit is in favour of doing it though: #21031 (comment)

I'm fine either way but would also prefer a separate PR.

Yes, this change should be part of another PR and probably for 5.8.

da-phil mentioned this pull request May 15, 2026

RFC: How should darkroom zooming-in behave? #21027

Open

1 task

da-phil changed the title ~~Activate 100% scroll limit again~~ Activate 100% zoom limit again May 15, 2026

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch from b3f01dc to a47afa2 Compare May 15, 2026 00:10

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch 2 times, most recently from 1c57c30 to 06965f7 Compare May 16, 2026 11:34

jenshannoschwalm reviewed May 16, 2026

View reviewed changes

Comment thread src/develop/develop.c Outdated

da-phil marked this pull request as ready for review May 16, 2026 12:34

TurboGit added this to the 5.6 milestone May 16, 2026

TurboGit added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: UI user interface and interactions default-behavior-change labels May 16, 2026

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch 3 times, most recently from d66abf9 to 1061f9c Compare May 18, 2026 07:29

da-phil added 2 commits May 18, 2026 11:30

Revert "Fix zoom freezing at named stops"

e2db5fb

This reverts commit b04e36e.

Activate 100% zoom limit again

62af432

* make scroll limit configurable through config parameter `darkroom/ui/constrain_zoom`, which is TRUE by default, keeping the previous behavior. * refactored the soft-step zooming logic in order to re-use it for pinch-zoom gestures.

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch from 1061f9c to 62af432 Compare May 18, 2026 09:55

TurboGit removed the default-behavior-change label May 19, 2026

TurboGit reviewed May 19, 2026

View reviewed changes

Comment thread data/darktableconfig.xml.in Outdated

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch from db791d2 to 1bb2a00 Compare May 19, 2026 12:49

TurboGit reviewed May 19, 2026

View reviewed changes

Make config params run-time configurable

2106a49

da-phil force-pushed the restrict_zooming_to_100_percent_by_default branch from 1bb2a00 to 2106a49 Compare May 19, 2026 22:19

Conversation

da-phil commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

da-phil commented May 15, 2026

Uh oh!

Uh oh!

da-phil commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

da-phil commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenshannoschwalm commented May 18, 2026

Uh oh!

da-phil commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboGit commented May 18, 2026

Uh oh!

jenshannoschwalm commented May 18, 2026

Uh oh!

da-phil commented May 18, 2026

Uh oh!

Uh oh!

da-phil commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TurboGit commented May 19, 2026

Uh oh!

TurboGit May 19, 2026

Choose a reason for hiding this comment

Uh oh!

da-phil May 19, 2026

Choose a reason for hiding this comment

Uh oh!

TurboGit May 20, 2026

Choose a reason for hiding this comment

Uh oh!

da-phil May 20, 2026

Choose a reason for hiding this comment

Uh oh!

TurboGit May 20, 2026

Choose a reason for hiding this comment

Uh oh!

da-phil May 20, 2026

Choose a reason for hiding this comment

Uh oh!

da-phil commented May 19, 2026

Uh oh!

wpferguson commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

da-phil commented May 20, 2026

Uh oh!

TurboGit commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

da-phil commented May 15, 2026 •

edited

Loading

da-phil commented May 16, 2026 •

edited

Loading

da-phil commented May 16, 2026 •

edited

Loading

da-phil commented May 18, 2026 •

edited

Loading

da-phil commented May 19, 2026 •

edited

Loading

wpferguson commented May 19, 2026 •

edited

Loading