- Status Closed
- Percent Complete
- 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
FS#2954 - Scrolling text margin for the wps
The patch adds the possibility to restrict the width of a (scrolling) line.
It enables you to position the scroll text inside the line and give him a
starting and ending position.
It extents the %s WPS tag to the following:
%s%m|x1|x2|
where x1 is the x-position where the line text starts and x2 is the
ending x-position of the scroll text (actually, when I look at the
code, it should also work for nonscrolling text…)
Only one margin is allowed per line.
Thanks Massa for the description
Closed by Llorean
2007-11-14 20:02
Reason for closing: Rejected
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
2007-11-14 20:02
Reason for closing: Rejected
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
The desired parts of this task have been accepted, and the remainder is rejected, to be done via viewports instead.
Any patch that seeks acceptance should
work in this direction and be a new
task.
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
i found a bug
dont download it, i will upload new version in the noon
the bug fixed and now you can use also %ar & %ac
if you want a regular margin after a irregular one,
just use %m without nothing
work with the archos recorder too
iuoçiuç
version 2, work with all the targets… CVS 2006-02-06
buggy, dont download.
old version for CVS 0207
http://takka.style.coocan.jp/wiki/?
plugin=attach&pcmd=open&file=scroll-
margins.patch&refer=RockBox
i synched this patch with the latest CVS (17.02.06). get it
here :
http://nicolas.pennequin.free.fr/scroll_margins.patch
nicolas_p's version doesn't compile.
neither does Paprica's orginal.
Massa version, synced with the cvs
sync with today's CVS (20060424):
Can we get another sync please with the latest CVS I'm getting HUNK errors 1 out of 9
Sorry, I don't have time atm. - so somebody else has to do it.
But beware, I think it's not only synching - there has to be some rework to make it properly work again… (I tried it a week ago and it had some weird effects…)
Here's a resync made by P.I. Julius
Sync to current CVS
Would someone be so kind to sync it to a recent cvs? since I can't get it to work properly myself. Thanks in advance.
My request is already fulfilled thanks to drippydonut @ misticriver
Updated patch to work agains CVS HEAD from 12.08.2006
Hi Max!
The recent changes in cvs (23.8,20:07,Mark Arigo):
have broken the last patch. Does this mean that a similar functionality will be included in CVS soon, or can we adjust the patch to work with the new CVS code.
Thanks a lot and all the best
Norbert
I am still at CVS from 20060820 where the patch still applies
Will check with CVS HEAD asap
IHMO the change is not changing this patch
just the order of two lines has switched
Updated patch to work against CVS from 20060824
Hi Max!
Thanks a lot. The patch does work after some small changes:
I changed it to leftmargin. And
there is a lcd_setmargins(0,0) left over, I added in the middle the LCD_WIDTH. This way it did compile.
I attach a fixed patch against current cvs for those who want to have it.
Thanks a lot and all the best
Norbert
Ooops!
I only tried it for the simulator and x5
Thanks for fixing it
updated for current CVS.
I currently notice a problem when combining this patch with the albumart patch (
FS#3045):I tried to use margins with a bitmap right to it (the margins starts at x=1 and ends one
pixel before the bitmap begins)
If a line left of the bitmap starts to scroll, the line flickers inside the bitmap…
Any idea what can cause this behaviour?
patching file apps/plugin.c
Hunk #1 succeeded at 560 (offset -1 lines).
Hunk #2 succeeded at 574 (offset -1 lines).
Hunk #3 FAILED at 600.
Hunk #4 FAILED at 610.
2 out of 4 hunks FAILED – saving rejects to file apps/plugin.c.rej
patching file apps/sound_menu.c
Hunk #1 succeeded at 742 (offset 1 line).
Hunk #2 succeeded at 790 (offset 1 line).
Hunk #3 FAILED at 1041.
1 out of 3 hunks FAILED – saving rejects to file apps/sound_menu.c.rej
And your point is?
The point appears to be that the patch is out of sync with CVS and needs updating.
sync
Small bugfix (old declaration in plugin.h and wrong function calls in viewer and solitaire-plugins)
and sync to today's CVS…
sync to today's CVS
sync to current CVS
sync again
seems to be out of sync again.
Sync to today's CVS
The patch applied successfully, but I get the following errors when trying to compile for the Gigabeat F/X port 20070210-build.
debug_menu.c: In function 'dbg_lcd_power_off':
debug_menu.c:2069: error: too few arguments to function 'lcd_setmargins'
debug_menu.c: In function 'dbg_buttonlights':
debug_menu.c:2111: error: too few arguments to function 'lcd_setmargins'
I am able to compile a clean build and also after adding the album art patch.
Thanks for any help.
This one should work.
Breaks the 4G Grayscale and iPod Mini 2G builds, seems to be okay with 5G and Nano (problem with b/w targets?)
Better now ? :)
Cheers ^^
Don't think this is quite working on the 5G, for example,
%s%m|18|209|%al%?ia<%ia|%?d2<%d2|(Artist Unknown)»
the |18|209| actually come up on screen?
Hi, great patch. Recent changes to bookmark.c seem to have broken it though. It patches fine, but when I try to compile, I get the following error:
bookmark.c: In function 'select_bookmark':
bookmark.c:553: error: too few arguments to function 'screens[i]setmargins'
I don't get the error in compiling when I remove the patch. I'm compiling for a Gigabeat F.
Thanks for any help.
edit the line to include 'screens[i].width' as second to last argument
So it reads: screens[i].setmargins(XXXX, screens[i].width, XXXX);
Just a question… will this ever be committed? The progress shows 100% complete, and the last "small bugfix" was on Monday, 23 October 2006… Are there any known bugs left? What are they?
Sync'ed to latest svn
You patch is for revision 12701
Since revision 12704 this patch is not working due to changes in the files list.c and solitaire.c
Don't worry about that, those changes to list.c and solitaire.c were reverted in svn anyway (as 12719). I just tried repatching latest svn (12723) and it worked fine.
Hello, I am new to this lark and am having some trouble using your patch. This is probably not the place to ask but ohwell. When I try to patch and compile using cygwin from the latest build for my iRiver H320 I get the following error:
CC drivers/lcd-remote-1bit-v.c
drivers/lcd-remote-1bit-v.c:82: error: conflicting types for 'lcd_remote_setmargins'
export/lcd-remote.h:145: error: previous declaration of 'lcd_remote_setmargins' was here
drivers/lcd-remote-1bit-v.c:82: error: conflicting types for 'lcd_remote_setmargins'
export/lcd-remote.h:145: error: previous declaration of 'lcd_remote_setmargins' was here
make[1]: * [dir] Error 1
make: * [all] Error 2
This is doubly annoying since I don't own an LCD remote so really don't care what is displayed on it!
I have the following code in my wps-file:
%s%m|16|144|%?it<%it|%d2>
%s%m|16|144|%alNext: %?It<%It|%Fn>
The first line scrolls to the end of the text, stops and scrolls back to the beginning.
In the second line it seems the text will get copied several time.
Like:
strcpy(showntext, "Test ");
for(a=0;a<UNKNOWN_NUMBER;a++)
⇒ showntext="Test Test Test Test Test Test Test" (or something like that).
The result is, that the text keeps scrolling without changing direction.
Is this a bug or a feature?
I'm pretty sure its supposed to keep scrolling one way. The 'back and forth' scroll is another FS patch, can't remember which number
Chris,
The scrolling does change direction in this patch, at least it should :D
The last time I tried it didn't =/
The scrolling "type" depends on the length of the string
But can be influenced from the setting "Bidirectional Scroll Limit"
(which is 50% by default IMHO)
I just found that one out.
so it's a feature :)
Dano: I'll take a look at updating in a minute or two, I confess I didn't try all targets
Dano: hm, the H3xx lcd driver seems to build ok here. Did you get a fresh update of source from svn and did you apply any other patches first (and/or were any conflicts or failures reported when you applied the scroll_margins_20070310 patch?)
Is the code for the file sound_menu.c needed?
This file has been removed from the current svn-tree.
Yeah I just noticed that when I went to try and confirm stripwax's questions with the latest build.
Anyway previously I had got the 20070312 source from the archive daily builds page, and I had also applied the latest album art patch and eq wps tags patch, and there weren't any errors or conflicts applying any of the patches.
Thanks very much for your help.
Will there be a patch for the current svn-build?
Rasmusma: Sync'd to svn as-of a couple of hours ago
Dano: hopefully this time the patch will work for you also, I've manually updated the 1bit remote driver
It works beautifully now, thank you very much for your help stripwax!
I am not 100% confident in my sync to 12944. Someone probably should test with a LCD remote. (Or just manually sync stripwax's lastest work.)
It appears to work fine on my iPods. (Famous last words.)
With some minor failed hunks in gwps/gui/gwps-common.c that can easily be manually added in by hand, Soap's patch works the best right now.
Would there be any problems to add this to the svn-tree?
I can't seem to get this to work in the new SVN.
I get "gui/gwps-common.c: In function ‘get_line’:
gui/gwps-common.c:1421: error: ‘fmt’ undeclared (first use in this function)
gui/gwps-common.c:1421: error: (Each undeclared identifier is reported only once
gui/gwps-common.c:1421: error: for each function it appears in.)"
And 1421 is the whole "case 'm': /* margins */" hunk of code.
Can someone upload a sync'd patch, or at least tell me what I've done wrong?
Some parts of the patch will need to be rewritten to be made compatible with the WPS tokenizer.
I'm working on the rewrite.
Sync'd. Added new WPS virtual tokens to handle the left and right margins. Shout if you find any problems with scrolling margins in WPSes that used to work but break with this new patch.
Sync'd again.
Sync'd again. Unfortunately the new WPS parser is a moving target, and the underlying function definitions keep changing..
Sync'd again. Also fixes problem of dynamic data on non-scrolling lines not updating correctly now that gwps-common has been fixed in svn to handle WPS_REFRESH_SCROLL properly (the %m tag should NOT have had WPS_REFRESH_SCROLL flag in wps_parser.c in the first place - apologies)
Needs another sync. Applies with many failed hunks.
I think you may be doing something wrong. I just updated to revision 13174 just now and this patch applied without incident.
Hmm. I'll try to apply to a clean SVN then. Maybe some other patch is messing it up.
Sync'd and fixed. Today's wps parser fix in svn revealed an off-by-one error in my parse function which was now caught by the improved error check (meaning wps that used scroll margins were no longer loadable!) Also needed resyn due to bookmark.c change in svn
hmm
I ask again: Would there be any problems to ADD this patch to the svn tree?
for anyone building for a gigabeat and getting errors on debug_menu.c
edit line 1140 in apps/debug_menu.c so it reads:
lcd_setmargins(0, LCD_WIDTH, 0);
sync'd and updated. If using albumart too, use latest albumart patch too to avoid sync conflicts.
Has anyone tried this? Because I'm having trouble loading up WPS's. It's either this or the Album Art patch.
Senab - I have been running my own build using this patch and the album art patch without any problems.
Out of sync- I fixed one simple update hunk but someone who knows the code needs to look at the others and see if they're needed
Hunk 1 of apps/onplay.c and Hunk 20 of apps/debug_menu.c
Attached is the otherwise synced version.
I wish I could say whether or not those 2 hunks are truly necessary, but for what it's worth I haven't had any problems whatsoever using the broken patch.
Same here.
FS #7158 being commited breaks this patch.
Are you sure it's broken? I took a look at the .rej and it's trying to add rb→lcdsetmargins(0,LCD_WIDTH, 0) so I took a look at bookmark.c for
rb→lcdsetmargins, this function (or whatever its called) isn't in bookmark.c anymore, so I tried compiling and it works. Did you try compiling?
Ah, I should probably do my homework before posting.
I think it's out of sync
I think this works.
I get two hunks failing and it won't make.
i too get a failure at lcd.h (hunk #2 at 393) and at lcd-16bit.c (hunk #7 at 947)
what to do…
Sync again then.
While the patch is working for me, I seem to be having trouble using the regular %m tag, such that it is a normal line. No matter what I do, the margin designated with just %m retains the same left position as the custom lines. Is anyone else having this issue, or perhaps I'm doing something wrong?
To clarify, when you say custom lines are those custom lines as in jbuild %e.. lines? If so I think that's the problem, mixing %e with %m on the same wps. If that's not the case then I have no idea.
I am actually not making use of the %e tag, but I found a solution to my problem anyways. The solution was simply to use a %m tag along with |x1|x2| positions related to the width of my wps, which was |0|240|. This does make sense, although it isn't what I had expected.
That's because that is how the scrolling margins patch works. Every text line needs to have a margin applied.
Out of sync.
Sync Please :)
Resync :)
It's just the older version with the hunks that didn't patch taken out (they weren't needed anymore).
Changed to apply cleanly following this morning's SVN changes.
My first re-sync :)
Only one small change; it should work with SVN as of 2007-08-15.
The last patch does not work. It misses the changes in alarm_menu.c (which were included in the previous patch).
That's odd. It applies just fine for me on a Gigabeat F40.
It builds fine, but I think it might be causing some weird behavior. Perhaps someone should do a proper re-sync based on the previous patch.
When trying to compile for the iPod Video with 64MB this patch still does results in the following compiling error:
… CC alarm_menu.c
alarm_menu.c: In function 'alarm_screen':
alarm_menu.c:73: error: too few arguments to function 'screens[i].setmargins'
make[1]: * [/home/user/rockbox/build/apps/alarm_menu.o] Error 1
make: * [build] Error 2
When including the previously included changes on alarm_menu.c it compiles fine.
sync
resync and fix buggy jumping strings
Hmm.. out of curiosity what exactly was it doing and what lines did you change? I pretty much just did a resync of 2007-08-15 with the hunk for alarm_menu.c put back in. I never noticed it doing anything on my iPod and I tested it on various themes some plain, some customline.
I added setmargins(0, LCD_WIDTH, 0) in get_line().
Please test follow wps with 2 patches.
— %wd
%s%m|25|100|%?it<%it |%fn>
%ar%pc/%pt
%arTrack %?in<%in|unknown> (%pp/%pe)
%s%m|100|175|%?it<%it |%fn>
—
I change from LCD_WIDTH to gwps→display→width for rwps.
I can definitely see that bug now with that test wps! Thanks for the fix!
sync
sync
A long string was displayed protruding to the right when new track info was loaded.
So I fix it.
more suitable for rwps.
When setting margins for a non-scrolling line (i.e. just %m without %s), still the complete line is cleared by write_line()/gui_wps_refresh(), destroying e.g. album art that might be next to the line and which is supposed to be protected by the margins.
The attached patch fixes this for me. Does anybody see a reason why it shouldn't be merged into the scroll-margins patch?
add lcd-charcell.c bogus support.
Now all target can be supposed to use this patch…
@Martin, I agree that should be merged too, and on another note thanks for posting that. Looking at that area of code made me rediscover something I meant to do a long time ago on another patch and just forgot to.
Sorry for the double post but this just occurred to me, I'd still go with display→width on that instead of scroll_width.
Does this patch fix that problem?
Fix clearing the center aligned string.
Sorry for above buggy patch…
@idak, what do you think of something like:
if (left_width || center_width || right_width) {
@tdtooke, Can I have your test wps, please?
I don't understand what has occurred…
Nothing happened, the patch performs flawlessly. I just thought that would be easier. Well….actually my motivation is a little bit selfish. If that part looked that way then all I'd have to do to sync another patch would be to just manually edit the patch, otherwise I have to do a lot of typing.
Out of sync, I tried to figure out the problem but it looks like a code change and I have no experience coding.
sync
getting a error on main_menu.c, I looked for the proper line but there must have been another code change.
I just igonired that HUNK, with success :)
it compiled but does it work?
If you mean, that all my themes are working as they should, yes! ;)
You don't' have to worry about it. They're doing things a different way now on main_menu.c that doesn't need that. Now if they had just moved that you'd not just get a warning, it wouldn't compile at all since you'd be calling setmargins with too few parameters.
Along with main_menu.c failing, debug_menu.c and scroll_engine.h now fail. Could someone please resync?
sync
Sync errors on todays SVN
8 out of 14 hunks FAILED – saving rejects to file apps/gui/gwps-common.c.rej
1 out of 1 hunk FAILED – saving rejects to file apps/gui/gwps.h.rej
1 out of 3 hunks FAILED – saving rejects to file apps/gui/wps_parser.c.rej
1 out of 16 hunks FAILED – saving rejects to file apps/debug_menu.c.rej
that's because part of the patch was committed. Should be an easy sync.
Yeah I saw that but why only left margins why just commit this patch?
Adding support for the right margin is the intrusive part of this patch. The left margin part is pretty trivial in comparison and only affect the WPS code. More than what has been committed isn't wanted because viewports are planned.
With a bit of a rewrite this could be made to assume that no declaration of margins means %m|0|320| and %s|leftmargin| means %s%m|leftmargin|320|. This might be the best compromise with the latest commit. Thoughts?
or should I say where I listed what it'd be on an iPod 5.5 it should be %m|0|LCD_WIDTH| or something like that
Very quick and dirty. With this if you're using a new WPS which might make use of %s|leftmargin| that will work and as before it works with %m|leftmargin|rightmargin|. As always if you set a margin for one line you set a margin for all lines (with %m that is). I have noticed that placement of the %s tag in the middle somewhere as opposed to the beginning of the line can cause problems so if you're using partial-scroll be aware of that. Also if you use the scrollwheel acceleration patch for the iPod that patch will add a setmargins call that you will have to manually fix. I didn't include that because that would introduce a dependency.
Another quick and dirty. This one basically just extends the %m functionality just committed to include the rightmargin. As in SVN it will work with just %m or %m|leftmargin| but will also work with %m|leftmargin|rightmargin| as before. I changed the name because it really isn't scroll-margins anymore. Maybe this should have a separate page on flyspray? Also as before of you use the iPod scrollwheel acceleration patch there is a setmargins call in debug_menu.c that you need to manually set. I would have included that but that would introduce a dependency.