Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#5907 - WPS partial line scroll

Attached to Project: Rockbox
Opened by Mark Arigo (lowlight) - Wednesday, 30 August 2006, 18:54 GMT+2
Last edited by Nicolas Pennequin (nicolas_p) - Saturday, 17 November 2007, 23:27 GMT+2
Task Type Patches
Category WPS
Status Closed
Assigned To No-one
Player type All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Currently %s scrolls the whole wps line. With this patch, only the text following (to the right) of the %s is scrolled.
So, you can have lines like "Title: %s%it".

Testing & comments welcomed.
(application/octet-stream)    wps_scroll.patch (4.5 KiB)
This task depends upon

Closed by  Nicolas Pennequin (nicolas_p)
Saturday, 17 November 2007, 23:27 GMT+2
Reason for closing:  Out of Date
Additional comments about closing:  A similar but more flexible feature is now in SVN.
Comment by Dominik Riebeling (bluebrother) - Wednesday, 30 August 2006, 22:28 GMT+2
Just tried it on h120 target. Works fine and I like it -- it makes it possible having a description left of a scrolling line. Nice :)
Comment by Eddy (bascule) - Wednesday, 01 November 2006, 23:36 GMT+2
I'd really like to try this patch, but I can't get it to complie against the current CVS (2006-11-01). I get about 4-5 HUNK failures and 2-3 successes, but with offsets. However, I've only just set up my development env, and this is the first patch I've tried, so it may be something I've got wrong...
Comment by Mark Arigo (lowlight) - Tuesday, 07 November 2006, 19:17 GMT+2
Updated patch.
FYI, here's the wps I use for my h140.

Artist: %s%?ia<%ia|%?d2<%d2|(root)>>
Album: %s%?id<%id|%?d1<%d1|(root)>>
Title: %s%?it<%it|%fn>

%pb
%al%pc%ac%pp/%pe%ar%pt

Next: %s%?It<%It|title not available>
(application/octet-stream)    wps_partial_scroll.patch (4.1 KiB)
Comment by Eddy (bascule) - Wednesday, 08 November 2006, 22:25 GMT+2
I've done some testing on this patch using an H120 and it seems rock solid

Alternating sublines work OK, even with different length text prior to the scroll point.
It can't be broken by putting two %s's in the line (it uses the last one) or by the length of text prior to the scroll point exceeding the visible width of the screen.
It also works OK when using aligned text (%al, %ac, %ar)
The bi-directional scroll setting always works as intended, whatever the size of the line to be scrolled is.
I've only checked the one target, but I assume that there are no target-dependencies in the WPS code, so I would expect that it would be OK in all circumstances.
I also checked all the default Rockbox included WPS's. Most have the %s at the left of the line, so are unaffected. Any others appear to be in sublines, so should also be unaffected.

For me, this is intuitively how the scroll function should work by default and it would be really good if this could get committed.
Comment by Mark Arigo (lowlight) - Tuesday, 14 November 2006, 22:37 GMT+2
I found a bug caused by conflicts with the alignment. When I fix it I'll submit this patch.
Comment by Eddy (bascule) - Wednesday, 15 November 2006, 09:47 GMT+2
I wait with bated breath!
Comment by Mark Arigo (lowlight) - Wednesday, 15 November 2006, 16:22 GMT+2
Here's the latest version. The problem I noticed was that %ac%s worked, but %s%ac did not. Turns out the alignment markers terminate the string buffer. I *think* I fixed this issue and the bundled wps's seem to work properly, but more testing would be appreciated for my own peace of mind.
(application/octet-stream)    wps_partial_scroll.patch (4.9 KiB)
Comment by Eddy (bascule) - Thursday, 16 November 2006, 13:28 GMT+2
I'll get on with some additional testing over the weekend, hopefully.
Comment by Eddy (bascule) - Tuesday, 28 November 2006, 22:13 GMT+2
Finally got round to testing, but it is still broken, I'm afraid, with the same error. The following line displays OK up to the scroll point, but then the conditional just displays as text.

Artist: %s%ac?ia<%ia|No Artist Info>%arI
Comment by Mark Arigo (lowlight) - Wednesday, 29 November 2006, 22:17 GMT+2
You missed a % before the ?

Artist: %s%ac%?ia<%ia|No Artist Info>%arI

It works for me, but the scroll start position is wrong. I'll have a look later.
Comment by Mark Arigo (lowlight) - Thursday, 30 November 2006, 20:24 GMT+2
Let's try this patch. I *think* it behaves well with the alignment tags. One thing to keep in mind is that the alignment markers are turned into spaces when the aligned text would overlap. Consider these 3 examples when %ia is long enough to scroll a line. First line is the wps code, second line is the displayed text. The _ indicates the extra space and | marks the scroll position.

Artist: %ac%ia
Artist: _Long artist name

Artist: %s%ac%ia
Artist: |_Long artist name

Artist: %ac%s%ia
Artist: _|Long artist name

I don't know of a way around this (or if it's important avoid).
(application/octet-stream)    wps_partial_scroll-1130.patch (4.8 KiB)
Comment by Eddy (bascule) - Friday, 22 December 2006, 14:53 GMT+2
As normal, it's taken me a long time to re-test this... :(

However, it looks spot on now, I have tried more tag combinations than before, and can't find any errors :)

As regards your last point, the only visual difference it makes is that the space between the end of the static text and the start of the scrolling text is bigger in the last example. I personally don't see it as a problem; it doesn't break anything and if you write your WPS intuitively with the %s where you actually want it (or at least consistently) it will not even be noticeable.

Go for the commit!
Comment by Brock Norman (mightybrick) - Thursday, 25 January 2007, 04:20 GMT+2
Hunk error with SVN from today.
Comment by Jacob Brooks (jac0b) - Saturday, 28 July 2007, 17:04 GMT+2
Is anyone looking at this? It would be a great feature.
Comment by Jonathan Gordon (jdgordon) - Saturday, 28 July 2007, 17:22 GMT+2
no, like all the custom scroll patches, we are waiting for viewports to be implemented which handles all these correctly.
Comment by Jacob Brooks (jac0b) - Tuesday, 02 October 2007, 17:37 GMT+2
Is this patch the same as the scrolling margins patch below.

[http://www.rockbox.org/tracker/task/2954][
Comment by Dominik Riebeling (bluebrother) - Tuesday, 02 October 2007, 17:59 GMT+2
no. If it would be the same it wouldn't be two tasks. It's related, and that's the reason why it is listed as related.
Comment by Thomas Martitz (kugel.) - Friday, 19 October 2007, 17:19 GMT+2
Is this patch basically an enhancement scroll margins?

I mean, if you put %s at the beginning of the line, there should be no difference between this and scroll margins.

Does this patch actually just add the feature to put %s to anywhere in the line?
Comment by Jacob Brooks (jac0b) - Friday, 19 October 2007, 23:07 GMT+2
This patch makes it so wherever you put %s that were it scrolls.
So if I did it this way Artist:%s%ia the "Artist:" would not scroll but anything after that would.
Comment by Thomas Martitz (kugel.) - Saturday, 20 October 2007, 00:01 GMT+2
I know what that patch does, I just wasn't sure if this patch includes scroll margins basically, but I found the answer myself.

My question is now, is this and scroll margins compatible? Can I do i.e. Artis: %s%m|x1|x2|%ia??
Comment by Jacob Brooks (jac0b) - Saturday, 20 October 2007, 01:23 GMT+2
I don't know, you might have to try that yourself to see. Also if you do get it to patch with the current SVN please post it.
Comment by Thomas Martitz (kugel.) - Saturday, 20 October 2007, 21:39 GMT+2
Sorry, this patch is completely broken. Needs to be rewritten.

Loading...