LightBulb
LightBulb copied to clipboard
Add customizable max transition duration for settings changes
Closes #290 by implementing a new setting, a max duration for settings config changes. If the duration of a transition would exceed the maximum duration allowed, it speeds up the step size of the transition to reach the target duration instead. This applies to all sliders, improving their smooth behavior to make them more responsive without sacrificing the smoothing.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 96.35%. Comparing base (
a9eb2f7
) to head (427a280
). Report is 9 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #291 +/- ##
=======================================
Coverage 96.35% 96.35%
=======================================
Files 7 7
Lines 192 192
Branches 15 15
=======================================
Hits 185 185
Misses 6 6
Partials 1 1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi, thanks for the contribution.
I'm apprehensive about adding this as a fully supported setting in the app due to maintainability reasons, but I'm okay with making it a "configurable constant" that you can change by editing the settings file directly.
Is that fine with you? Otherwise you're always welcome to keep using the fork with your own changes as well.
Yep, that's totally fine by me, makes sense! I'll update the code later today.
What's a reasonable default value? I can restore the old behavior by making it 15s, or use a lower value to make sliders and configs more responsive by default.
What's a reasonable default value? I can restore the old behavior by making it 15s, or use a lower value to make sliders and configs more responsive by default.
I'd keep the current value as the default.
Fixed and pushed.
Hi, any updates on this? Happy to make any additional changes as needed.
Hi, any updates on this? Happy to make any additional changes as needed.
Hi, sorry, was away for a while. I'm still ruminating on the extra complexity in the UpdateConfiguration()
method, more specifically the additional state it now has to maintain in _brightnessMaxStep
, _temperatureMaxStep
, _lastTarget
. I was trying to think of how to avoid it, but can't keep up with a good solution.
Looking back, your original issue was that the settings sliders were not responsive enough. If that's the case, we may be able to solve this by disabling smoothing when the gamma changes are caused by settings changes. Or even simpler, disable gamma smoothing when the settings dialog is open.
What that solve the issue?
Sorry for the back-and-forth, I'm trying to find how to achieve the desired result in the smallest amount of changes possible.
Looking back, your original issue was that the settings sliders were not responsive enough. If that's the case, we may be able to solve this by disabling smoothing when the gamma changes are caused by settings changes. Or even simpler, disable gamma smoothing when the settings dialog is open.
My biggest issue is actually that, when I disable the filter entirely, it takes forever. If I want to jump into a game, I have to wait 15s for it to change. The sliders are a secondary issue; they also take a while for large changes, but I'm not adjusting them frequently. (Some users may, however).
A quick fix is to disable gamma smoothing entirely, but that causes instant changes, which doesn't give time for the eyes to adjust at all.
So ideally, any solution would hit these points:
- Enabling/disabling the filter is fast, without being instant
- Changing the sliders is more responsive, without being instant
The max duration limit for changes does hit these points, but the code is indeed more complex.
Sorry for the back-and-forth, I'm trying to find how to achieve the desired result in the smallest amount of changes possible.
No problem, we should work to find the best solution!
I wonder if we track _lastConfigurationBeforeSmoothing
(or something) instead of _lastTarget
, we'd be able to re-calculate the steps every time and avoid carrying that stuff as state?
I wonder if we track _lastConfigurationBeforeSmoothing (or something) instead of _lastTarget, we'd be able to re-calculate the steps every time and avoid carrying that stuff as state?
Sure, that works; pushed!