Fix infinite loop crash when Levels Gamma is set to 0#2041
Conversation
|
|
||
| private void UpdateGammaByMask (float val) | ||
| { | ||
| val = Math.Clamp(val, 0.1f, 10.0f); |
There was a problem hiding this comment.
It looks like there was an error from the code formatting check in the CI build. In general you can run dotnet format, but the specific fix here is adding a space before the bracket Clamp (val
| private void UpdateGammaByMask (float val) | ||
| { | ||
| val = Math.Clamp(val, 0.1f, 10.0f); | ||
| if (!(mask.R || mask.G || mask.B)) |
There was a problem hiding this comment.
Thanks for looking into this! The logic seems good to me with this fix.
Since these 0.1f and 10.0f values also occur a couple times in UnaryPixelOps.cs, could you extract them into public constants on the Level adjustment's class?
There was a problem hiding this comment.
hey! Ive moved the gamma bounds to MinGamma and MaxGamma constants in UnaryPixelOps.Level and updated all references in UnaryPixelOps.cs and LevelsDialog.cs. I also fixed the formatting in LevelsDialog.cs to resolve the CI failure. Verified the fix locally via Flatpak; let me know if you need anything else!
There was a problem hiding this comment.
Looks good, thanks for the contribution! 🎉
24be8b0 to
8e130f1
Compare
Fixes #2035
Description
This PR fixes a bug where the application permanently freezes if the user drags the gamma (midtones) slider down to 0.
The freeze was caused by an infinite do-while loop in {UpdateGammaByMask(float val)}. The UI allows the user to request a value of 0 , but the underlying engine strictly clamps Gamma to a minimum of 0.1 . Because the target value could never be reached, the loop never exited locking up the main thread.
Changes
Added 'val = Math.Clamp(val, 0.1f, 10.0f);' at the start of {UpdateGammaByMask} in Effects.LevelsDialog.cs to ensure the requested value matches the physical limits before the loop begins.