FS#10636 - Quickscreen doesn't work correctly for integer menus with a negative step

Attached to Project: Rockbox
Opened by Jeffrey Goode (Blue_Dude) - Saturday, 03 October 2009, 02:14 GMT
Last edited by Jeffrey Goode (Blue_Dude) - Saturday, 03 October 2009, 13:01 GMT
Task Type Bugs
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


When assigning an integer choosing menu with a negative step (i.e. it counts down instead of up), the quickscreen button goes to the lowest value (the bottom value) and stops there when scrolling up the menu, and goes to the highest value (the top value) and stops when scrolling down the menu. The quickscreen behaves correctly when redefining the menu with a positive step, but this isn't ideal.

To test: assign the compressor threshold to a quickscreen. The menu starts at Off (zero) and counts down by threes to -24 db. When assigned to the top quickscreen, the value locks onto -24 db and when assigned to the bottom quickscreen, the value locks to Off. If the menu is redefined in settings_list.c to start at -24 at the top and count up by threes to 0, the quickscreen behavior is correct. I'd rather not do this since the normal menu behavior would be counter-intuitive, i.e. maximum effect at the top of the menu scrolling towards minimum effect/off at the bottom.

Please look into handling negative steps correctly in the quickscreen code.
This task depends upon

Closed by  Jeffrey Goode (Blue_Dude)
Saturday, 03 October 2009, 13:01 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed in r22887
Comment by Jeffrey Goode (Blue_Dude) - Saturday, 03 October 2009, 12:54 GMT
The bug is in option_select.c. There was an attempt at programming in negative step behavior but the logic was incorrect. The solution is to change the direction of the comparisons that choose when to wrap the menu. With a negative sign, min is a larger value than max, so the menu was always out of range and wrapped. The step should not change sign.

I also found a bug where in some cases the menu callback was called with incorrect information. Some menu callbacks throw away the value argument and operate on the stored value directly. So the value has to be stored before the callback. This has been corrected.