Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category User Interface → Themes
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by lowlight - 2006-08-30
Last edited by nicolas_p - 2007-11-17

FS#5907 - WPS partial line scroll

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.

Closed by  nicolas_p
2007-11-17 22:27
Reason for closing:  Out of Date
Additional comments about closing:  

A similar but more flexible feature is now in SVN.

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 :)

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…

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>

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.

I found a bug caused by conflicts with the alignment. When I fix it I’ll submit this patch.

I wait with bated breath!

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.

I’ll get on with some additional testing over the weekend, hopefully.

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

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.

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).

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!

Hunk error with SVN from today.

jac0b commented on 2007-07-28 15:04

Is anyone looking at this? It would be a great feature.

no, like all the custom scroll patches, we are waiting for viewports to be implemented which handles all these correctly.

jac0b commented on 2007-10-02 15:37

Is this patch the same as the scrolling margins patch below.

[http://www.rockbox.org/tracker/task/2954][

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.

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?

jac0b commented on 2007-10-19 21:07

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.

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??

jac0b commented on 2007-10-19 23:23

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.

Sorry, this patch is completely broken. Needs to be rewritten.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing