Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#5182 - One Button hold for vertical screen scroll patch

Attached to Project: Rockbox
Opened by Shachar (Lamed) - Wednesday, 19 April 2006, 19:26 GMT+1
Last edited by Brandon Low (lostlogic) - Wednesday, 19 April 2006, 21:26 GMT+1
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version
Due in Version Version 3.0
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch is to change the vertical screen scroll keybinds from two buttons macro (i.e ON+LEFT/RIGHT on HX00),
to one long (hold) press (LEFT/RIGHT on Hx00), as discussed a long time ago in IRC.
This will help users to operate the vertical scroll as it demands two hand operation as it is now.

the player isn't effected by this patch as it only has two lines lcd.

The ONDIO is the only player that still uses MENU + LEFT / RIGHT combination, because it's RIGHT_REPEAT button is already occupied with the TREE_CONTEXT menu.
It is better if someone checks the ondio before submitting to CVS because I can't get the SDL sims to work :/

There's one small bug solved in all the lcd-bitmapped drivers that caused the inverse bar to crop down in the middle when the user scrolled vertically.

(note, there is a redundant #define TREE_POWER_BTN in tree.h, my assumption is that it was before the gui_list widget was introduced.)
(application/octet-stream)    screen_scroll_oneclick_4.19.06 (11.3 KiB)
This task depends upon

Closed by  Shachar (Lamed)
Saturday, 12 August 2006, 15:43 GMT+1
Reason for closing:  Accepted
Comment by Shachar (Lamed) - Wednesday, 19 April 2006, 21:13 GMT+1
ircnick:sharpe checked an ondio sim for me, the patch is fine :)
Comment by Shachar (Lamed) - Wednesday, 19 April 2006, 21:17 GMT+1
and ho, yeah, someone should put it into the manual as well :)

http://www.rockbox.org/tracker/task/5073
Comment by Shachar (Lamed) - Saturday, 22 April 2006, 12:37 GMT+1
the line:
xrect = MAX(w - offset, 0);
in all LCD driver should be: xrect = xpos + MAX(w - offset, 0);
please do not commit before changing
Comment by Shachar (Lamed) - Thursday, 04 May 2006, 06:15 GMT+1
The right, working file. Btw, this also _really_ fixes http://www.rockbox.org/tracker/task/5267
(I was the one who wrote horizontal screen scroll, a long time ago we had an irc conversation that it is better if screen scrolling would not need to use two keys combination, and a short while a go I had time to write it down. It also fixes a behaviour whereas holding RIGHT key on a folder will enter subfolders very quickly, an unwanted behaviour for sure ( BUTTON_RIGHT | BUTTON_REPEAT is removed) )
(application/octet-stream)    screen_scroll_oneclick_5.4.06 (11.4 KiB)
Comment by Linus Nielsen Feltzing (linusnielsen) - Saturday, 06 May 2006, 08:40 GMT+1
I have committed the cropping fix. I have no opinion about the key mapping, so I'll probably commit that as well, as I'm a big fan of one-handed operation.
Comment by Linus Nielsen Feltzing (linusnielsen) - Saturday, 06 May 2006, 08:43 GMT+1
Here's an updated version without the bug fix.
(application/octet-stream)    screen_scroll_20060506.patch (6.4 KiB) 
 gui/list.c        |    5 +++++
 gui/list.h        |   20 ++++++++++++++------
 menu.c            |   11 +++++++++--
 playlist_viewer.c |    9 ++++++---
 tree.c            |   10 ++++++----
 5 files changed, 40 insertions(+), 15 deletions(-)

Comment by Jens Arnold (amiconn) - Saturday, 06 May 2006, 10:34 GMT+1
@ Linus:

You should have checked the previous commits before committing this. Peter D'Hoye fixed the inverse bar issue 3 days ago.

The only difference between his fix and your commit is that he didn't clamp the value to zero but let it become negative under certain conditions. That doesn't hurt though, as lcd_fillrect() catches this and returns immediately. His version saved a bit of code...
Comment by Linus Nielsen Feltzing (linusnielsen) - Saturday, 06 May 2006, 13:15 GMT+1
Yes, he made an attempt to fix it. It was not 100% succesful though, see http://www.rockbox.org/tracker/task/5267.
Comment by Shachar (Lamed) - Tuesday, 16 May 2006, 06:10 GMT+1
Jens, I'm sorry, you're wrong. because he let the numbers get negative, the Icons (on the left) got color-inverted, at least on the h1x0 (which shouldn't really be any diffrent from any other target, I'm just not l00king at the code right now).
Think about it this way: the filling rect from the right side of the text did not 'stopped' at the left side boarder and 'filled' the icon.


Linus, I believe you haven't submitted the key mapping code,
a. why is that?
b. I think it broke the sudoko menu, but I haven't had time to see what caused me the problem (the menu was not responsive at all, I had to shut down).
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 16 May 2006, 09:28 GMT+1
a) Because I wanted to fix the bug without changing an existing feature.

b) What broke the Sudoku menu?
Comment by Shachar (Lamed) - Sunday, 21 May 2006, 11:22 GMT+1
b. The Sudoku menu caught menu enter downpresses and was activated on button release (MENU_ENTER became BUTTON_NONE, and MENU_ENTER | BUTTON_REL became MENU_ENTER.)
I'm not sure why is was introduced in such a way, other then the comment saying 'next menu doesn't like release'.
This code isn't necessary with the screen scroll hold button patch for targets that actually uses it, but I assume ONDIO's should still use it : "The ONDIO is the only player that still uses MENU + LEFT / RIGHT combination, because it's RIGHT_REPEAT button is already occupied with the TREE_CONTEXT menu."

I would have checked it myself but I can't get Vmware simulators running... they are all corrupted for me. ##$%#%Y$.
so someone should #ifdef it properly or tell me how to solve the Vmware issue :P


a.huh, I guess I have nothing to say about that.
   sudoku_fix.patch (1.2 KiB) 
 sudoku.c |   27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

Comment by Linus Nielsen Feltzing (linusnielsen) - Thursday, 13 July 2006, 10:24 GMT+1
Any news about the Ondio compatibility with this patch?
Comment by Shachar (Lamed) - Thursday, 13 July 2006, 14:54 GMT+1
The patch was fine for ondio, but I added one more lastbutton check so that screen scrolling would work on ondio menu system. (without it, pressing menu would have left the menu on click, now it's on release.)

I do remind sudoku_fix.patch is importent for this patchfile.
(application/octet-stream)    oneclick_screenscroll_02 (6.3 KiB)

Loading...