LightBulb icon indicating copy to clipboard operation
LightBulb copied to clipboard

Add customizable max transition duration for settings changes

Open LilithSilver opened this issue 10 months ago • 8 comments

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.

LilithSilver avatar Apr 19 '24 08:04 LilithSilver

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.

codecov[bot] avatar Apr 19 '24 14:04 codecov[bot]

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.

Tyrrrz avatar Apr 21 '24 15:04 Tyrrrz

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.

LilithSilver avatar Apr 21 '24 18:04 LilithSilver

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.

Tyrrrz avatar Apr 21 '24 23:04 Tyrrrz

Fixed and pushed.

LilithSilver avatar Apr 22 '24 00:04 LilithSilver

Hi, any updates on this? Happy to make any additional changes as needed.

LilithSilver avatar Apr 30 '24 02:04 LilithSilver

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.

Tyrrrz avatar May 01 '24 18:05 Tyrrrz

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!

LilithSilver avatar May 01 '24 20:05 LilithSilver

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?

Tyrrrz avatar May 05 '24 17:05 Tyrrrz

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!

LilithSilver avatar May 05 '24 22:05 LilithSilver