bauhaus: fix scroll step calculation for large-range sliders - #21856
Open
kadykov wants to merge 1 commit into
Open
bauhaus: fix scroll step calculation for large-range sliders#21856kadykov wants to merge 1 commit into
kadykov wants to merge 1 commit into
Conversation
The 2022 refactor of dt_bauhaus_slider_get_step() introduced a guard: ranges >= 100 always produce step = 1, bypassing the logarithmic formula that computes a proportional, round-number step. This works for medium-range sliders (spans 100-315 still yield step = 1 via the formula anyway) but is far too fine for sliders with large ranges. Most notably, the color calibration temperature slider (soft range 3000-7000 K, span = 4000) gets step = 1 K per scroll tick, requiring 100 scroll steps to move 100 K — barely perceptible. Remove the guard and let the log formula run for all ranges: span 100-315 → step 1 (no change, same as the old guard) span 316-794 → step 5 (e.g. 0-360° hue: 72 steps per revolution) span 795-3162 → step 10 (e.g. radius 0-500 px, sigmoid white target) span 3163+ → step 50+ (e.g. temperature soft 3000-7000 K → 50 K) Modules that genuinely need a non-formula step can still override via dt_bauhaus_slider_set_step().
There was a problem hiding this comment.
Pull request overview
This PR fixes mouse scroll-wheel step sizing for Bauhaus sliders with large visible (soft) ranges by removing a hard-coded step = 1 shortcut and always using the existing logarithmic “~1% of range” rounding logic. This improves usability for large-unit sliders like color temperature (Kelvin) while keeping behavior unchanged for common small-range sliders.
Changes:
- Remove the
top >= 100guard that forcedstep = 1.ffor large soft ranges. - Always compute the scroll step from the proportional/log-rounded formula for all ranges.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
dt_bauhaus_slider_get_step()function computes how much a slider valuechanges per scroll-wheel tick. Since the February 2022 refactor
(commit
f7561c5e79), the function contains this branch:For any slider whose visible span (soft range) is ≥ 100, the step is always
hardcoded to 1 — bypassing the logarithmic formula that produces a proportional,
round-number step.
This is unnoticeable for sliders where "1 unit" is a natural granularity (e.g.
a 0–100 % slider, a 0–360° hue slider). However it produces unusable behaviour
for sliders measured in large-valued units. The clearest example is the
color temperature parameter in the color calibration module
(soft range 3000–7000 K, span = 4000):
slider (soft range ~0.7–3.0, span ~2.3) scrolls at 0.01 per tick (~0.4 % of span).
Root cause
The log formula correctly produces "~1 % of visible span, rounded to the nearest
1/2/5 × 10ⁿ". It was always intended to cover all ranges; the
>= 100guardis an unnecessary short-circuit that happens to give the same answer as the
formula for spans 100–315, but diverges above that.
Fix
Remove the
>= 100guard. The formula now runs for all ranges:Impact
Modules that need a non-formula step can still call
dt_bauhaus_slider_set_step()to override (as
contrastntexture.calready does for a very fine 0.0001 step).Fix #21855
Testing
(soft range 3000–7000 K active), comparable to Lightroom's default WB step.
range): scroll moves in 100 K increments.