Rockbox

Tasklist

FS#2954 - Scrolling text margin for the wps

Attached to Project: Rockbox
Opened by Ben Basha (paprica) - Saturday, 28 January 2006, 01:47 GMT
Last edited by Paul Louden (Llorean) - Wednesday, 14 November 2007, 20:02 GMT
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

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
This task depends upon

Closed by  Paul Louden (Llorean)
Wednesday, 14 November 2007, 20:02 GMT
Reason for closing:  Rejected
Additional comments about closing:  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.
Comment by Ben Basha (paprica) - Saturday, 28 January 2006, 02:34 GMT

i found a bug

dont download it, i will upload new version in the noon
Comment by Ben Basha (paprica) - Saturday, 28 January 2006, 10:23 GMT

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
Comment by Ben Basha (paprica) - Saturday, 28 January 2006, 11:56 GMT

work with the archos recorder too
Comment by Anonymous Submitter - Wednesday, 01 February 2006, 19:41 GMT

iuoçiuç
Comment by Ben Basha (paprica) - Monday, 06 February 2006, 17:21 GMT
version 2, work with all the targets...
CVS 2006-02-06
Comment by Ben Basha (paprica) - Monday, 06 February 2006, 20:53 GMT

buggy, dont download.
Comment by takka (tfact) - Tuesday, 07 February 2006, 01:11 GMT

old version for CVS 0207
http://takka.style.coocan.jp/wiki/?
plugin=attach&pcmd=open&file=scroll-
margins.patch&refer=RockBox
Comment by Nicolas Pennequin (nicolas_p) - Friday, 17 February 2006, 19:34 GMT

i synched this patch with the latest CVS (17.02.06). get it
here :
http://nicolas.pennequin.free.fr/scroll_margins.patch
Comment by needleboy (needleboy) - Sunday, 19 February 2006, 09:31 GMT

nicolas_p's version doesn't compile.
neither does Paprica's orginal.
Comment by Ben Basha (paprica) - Monday, 03 April 2006, 12:37 GMT

Massa version, synced with the cvs
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 24 April 2006, 11:31 GMT
sync with today's CVS (20060424):
Comment by James Wilson (Pool) - Friday, 12 May 2006, 23:53 GMT
Can we get another sync please with the latest CVS I'm getting HUNK errors 1 out of 9
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 15 May 2006, 15:41 GMT
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...)
Comment by Chris Banes (senab) - Monday, 29 May 2006, 08:51 GMT
Here's a resync made by P.I. Julius
Comment by Nicolas Pennequin (nicolas_p) - Thursday, 15 June 2006, 17:14 GMT
Sync to current CVS
Comment by JP (salival) - Tuesday, 25 July 2006, 14:18 GMT
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.
Comment by JP (salival) - Tuesday, 25 July 2006, 20:35 GMT
My request is already fulfilled thanks to drippydonut @ misticriver
Comment by Max Weninger (maxwen) - Saturday, 12 August 2006, 14:52 GMT
Updated patch to work agains CVS HEAD from 12.08.2006
Comment by Norbert Preining (norbusan) - Thursday, 24 August 2006, 07:47 GMT
Hi Max!
The recent changes in cvs (23.8,20:07,Mark Arigo):
Allow scrolling lines with different x-margins. The margin at the time of the call
to puts_scroll will be used as the margin for that line.
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
Comment by Max Weninger (maxwen) - Thursday, 24 August 2006, 11:35 GMT
I am still at CVS from 20060820 where the patch still applies
Will check with CVS HEAD asap
Comment by Max Weninger (maxwen) - Thursday, 24 August 2006, 21:50 GMT
IHMO the change is not changing this patch
just the order of two lines has switched

Updated patch to work against CVS from 20060824
Comment by Norbert Preining (norbusan) - Friday, 25 August 2006, 06:06 GMT
Hi Max!
Thanks a lot. The patch does work after some small changes:
drivers/lcd-h100-remote.c:1398: error: `xmargin' undeclared (first use in this function)
I changed it to leftmargin. And
debug_menu.c:1240: error: too few arguments to function `lcd_setmargins'
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
Comment by Max Weninger (maxwen) - Friday, 25 August 2006, 08:41 GMT
Ooops!

I only tried it for the simulator and x5 :-(

Thanks for fixing it
Comment by Nicolas Pennequin (nicolas_p) - Tuesday, 29 August 2006, 02:59 GMT
updated for current CVS.
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 04 September 2006, 21:27 GMT
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?
Comment by Jon (ace214) - Monday, 09 October 2006, 15:34 GMT
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

Comment by Paul van der Heu (paulheu) - Monday, 09 October 2006, 16:20 GMT
And your point is?
Comment by Dave Chapman (linuxstb) - Monday, 09 October 2006, 19:44 GMT
The point appears to be that the patch is out of sync with CVS and needs updating.
Comment by Jon (ace214) - Tuesday, 10 October 2006, 19:14 GMT
sync
Comment by Matthias Mohr (aka Massa) (mmohr) - Monday, 23 October 2006, 08:40 GMT
Small bugfix (old declaration in plugin.h and wrong function calls in viewer and solitaire-plugins)
and sync to today's CVS...
Comment by Matthias Mohr (aka Massa) (mmohr) - Wednesday, 25 October 2006, 08:40 GMT
sync to today's CVS
Comment by Nicolas Pennequin (nicolas_p) - Wednesday, 08 November 2006, 21:18 GMT
sync to current CVS
Comment by Nicolas Pennequin (nicolas_p) - Tuesday, 14 November 2006, 14:42 GMT
sync again
Comment by Johannes Voggenthaler (ZincAlloy) - Monday, 11 December 2006, 14:14 GMT
seems to be out of sync again.
Comment by Nicolas Pennequin (nicolas_p) - Friday, 05 January 2007, 19:31 GMT
Sync to today's CVS
Comment by Nathan Hale (Incubus-1) - Sunday, 11 February 2007, 20:49 GMT
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.
Comment by Nicolas Pennequin (nicolas_p) - Monday, 12 February 2007, 21:04 GMT
This one should work.
Comment by Chris (decayed.cell) - Tuesday, 13 February 2007, 12:10 GMT
Breaks the 4G Grayscale and iPod Mini 2G builds, seems to be okay with 5G and Nano (problem with b/w targets?)
Comment by Nicolas Pennequin (nicolas_p) - Tuesday, 13 February 2007, 12:44 GMT
Better now ? :)
Comment by Chris (decayed.cell) - Tuesday, 13 February 2007, 20:33 GMT
Cheers ^^
Comment by Chris (decayed.cell) - Wednesday, 14 February 2007, 10:57 GMT
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?
Comment by Derek Van Ittersum (derekvan) - Monday, 19 February 2007, 15:28 GMT
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.
Comment by Paul van der Heu (paulheu) - Monday, 19 February 2007, 17:26 GMT
edit the line to include 'screens[i].width' as second to last argument

So it reads: screens[i].setmargins(XXXX, screens[i].width, XXXX);
Comment by Bobby Graese (TrueJournals) - Friday, 09 March 2007, 02:35 GMT
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?
Comment by Dave Hooper (stripwax) - Saturday, 10 March 2007, 17:33 GMT
Sync'ed to latest svn
Comment by Marc R. (rasmusma) - Saturday, 10 March 2007, 20:10 GMT
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
Comment by Dave Hooper (stripwax) - Sunday, 11 March 2007, 14:13 GMT
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.
Comment by Dan Train (Dano) - Monday, 12 March 2007, 10:10 GMT
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!
Comment by Marc R. (rasmusma) - Monday, 12 March 2007, 10:42 GMT
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++)
strcat(showntext, "Test ");

=> 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?
Comment by Chris (decayed.cell) - Monday, 12 March 2007, 10:45 GMT
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
Comment by Chris Banes (senab) - Monday, 12 March 2007, 10:50 GMT
Chris,

The scrolling does change direction in this patch, at least it should :D
Comment by Chris (decayed.cell) - Monday, 12 March 2007, 10:58 GMT
The last time I tried it didn't =/
Comment by Max Weninger (maxwen) - Monday, 12 March 2007, 11:41 GMT
The scrolling "type" depends on the length of the string
Comment by Max Weninger (maxwen) - Monday, 12 March 2007, 11:43 GMT
But can be influenced from the setting "Bidirectional Scroll Limit"
(which is 50% by default IMHO)
Comment by Marc R. (rasmusma) - Monday, 12 March 2007, 12:02 GMT
> The scrolling "type" depends on the length of the string

I just found that one out.
so it's a feature :)

Comment by Dave Hooper (stripwax) - Monday, 12 March 2007, 20:48 GMT
Dano: I'll take a look at updating in a minute or two, I confess I didn't try all targets
Comment by Dave Hooper (stripwax) - Monday, 12 March 2007, 21:06 GMT
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?)
Comment by Marc R. (rasmusma) - Wednesday, 14 March 2007, 09:55 GMT
Is the code for the file sound_menu.c needed?
This file has been removed from the current svn-tree.
Comment by Dan Train (Dano) - Wednesday, 14 March 2007, 10:15 GMT
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.
Comment by Marc R. (rasmusma) - Friday, 16 March 2007, 04:45 GMT
Will there be a patch for the current svn-build?
Comment by Dave Hooper (stripwax) - Friday, 16 March 2007, 23:48 GMT
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
Comment by Dan Train (Dano) - Sunday, 18 March 2007, 18:29 GMT
It works beautifully now, thank you very much for your help stripwax!
Comment by David Hall (Soap) - Wednesday, 28 March 2007, 00:40 GMT
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.)
Comment by Chris Olin (Landus) - Thursday, 05 April 2007, 04:21 GMT
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.
Comment by Marc R. (rasmusma) - Thursday, 05 April 2007, 04:31 GMT
Would there be any problems to add this to the svn-tree?
Comment by Jordan (aqtrans) - Thursday, 05 April 2007, 21:25 GMT
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?
Comment by Nicolas Pennequin (nicolas_p) - Friday, 06 April 2007, 01:09 GMT
Some parts of the patch will need to be rewritten to be made compatible with the WPS tokenizer.
Comment by Dave Hooper (stripwax) - Saturday, 07 April 2007, 12:57 GMT
I'm working on the rewrite.
Comment by Dave Hooper (stripwax) - Saturday, 07 April 2007, 16:14 GMT
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.
Comment by Dave Hooper (stripwax) - Sunday, 08 April 2007, 09:54 GMT
Sync'd again.
Comment by Dave Hooper (stripwax) - Monday, 09 April 2007, 10:05 GMT
Sync'd again. Unfortunately the new WPS parser is a moving target, and the underlying function definitions keep changing..
Comment by Dave Hooper (stripwax) - Thursday, 12 April 2007, 21:24 GMT
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)
Comment by Oleg G (MadCow15) - Sunday, 15 April 2007, 22:06 GMT
Needs another sync. Applies with many failed hunks.
Comment by Travis Tooke (tdtooke) - Monday, 16 April 2007, 03:59 GMT
I think you may be doing something wrong. I just updated to revision 13174 just now and this patch applied without incident.
Comment by Oleg G (MadCow15) - Monday, 16 April 2007, 11:31 GMT
Hmm. I'll try to apply to a clean SVN then. Maybe some other patch is messing it up.
Comment by Dave Hooper (stripwax) - Tuesday, 24 April 2007, 12:59 GMT
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
Comment by Marc R. (rasmusma) - Tuesday, 24 April 2007, 14:58 GMT
hmm
I ask again: Would there be any problems to ADD this patch to the svn tree?
Comment by Gary Light (evilg123) - Wednesday, 25 April 2007, 01:24 GMT
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);

Comment by Dave Hooper (stripwax) - Thursday, 26 April 2007, 21:33 GMT
sync'd and updated. If using albumart too, use latest albumart patch too to avoid sync conflicts.
Comment by Chris Banes (senab) - Saturday, 28 April 2007, 22:17 GMT
Has anyone tried this? Because I'm having trouble loading up WPS's. It's either this or the Album Art patch.
Comment by Dave Hooper (stripwax) - Sunday, 29 April 2007, 19:21 GMT
Senab - I have been running my own build using this patch and the album art patch without any problems.
Comment by Jon (ace214) - Thursday, 10 May 2007, 02:42 GMT
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.
Comment by Travis Tooke (tdtooke) - Tuesday, 15 May 2007, 06:48 GMT
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.
Comment by Jon (ace214) - Tuesday, 15 May 2007, 13:15 GMT
Same here.
Comment by Travis Tooke (tdtooke) - Monday, 28 May 2007, 04:33 GMT
FS #7158 being commited breaks this patch.
Comment by Gary Light (evilg123) - Monday, 28 May 2007, 14:36 GMT
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?
Comment by Travis Tooke (tdtooke) - Tuesday, 29 May 2007, 06:34 GMT
Ah, I should probably do my homework before posting.
Comment by Matthew Schneider (mschneider) - Tuesday, 19 June 2007, 22:20 GMT
I think it's out of sync
Comment by Tan Yu Sheng (Farpenoodle) - Wednesday, 20 June 2007, 12:33 GMT
I think this works.
Comment by Matthew Evans (onocentaur) - Friday, 22 June 2007, 18:45 GMT
I get two hunks failing and it won't make.
Comment by Andrej W. (ch_604) - Saturday, 23 June 2007, 02:22 GMT
i too get a failure at lcd.h (hunk #2 at 393) and at lcd-16bit.c (hunk #7 at 947)

what to do...
Comment by Tan Yu Sheng (Farpenoodle) - Saturday, 23 June 2007, 19:39 GMT
Sync again then.
Comment by Jake Melvin (Monkeytamer) - Sunday, 01 July 2007, 09:12 GMT
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?
Comment by Travis Tooke (tdtooke) - Monday, 02 July 2007, 02:24 GMT
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.
Comment by Jake Melvin (Monkeytamer) - Monday, 02 July 2007, 07:04 GMT
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.
Comment by Matthew Schneider (mschneider) - Monday, 02 July 2007, 17:55 GMT
That's because that is how the scrolling margins patch works. Every text line needs to have a margin applied.
Comment by Dieter (dip) - Friday, 06 July 2007, 14:47 GMT
Out of sync.
Comment by Nick Brackley (darksaboteur) - Wednesday, 11 July 2007, 11:11 GMT
Sync Please :)
Comment by Chris Banes (senab) - Thursday, 12 July 2007, 15:06 GMT
Resync :)

It's just the older version with the hunks that didn't patch taken out (they weren't needed anymore).
Comment by Phil Light (phillight) - Saturday, 28 July 2007, 11:32 GMT
Changed to apply cleanly following this morning's SVN changes.
Comment by Mike Kasberg (digerati1338) - Thursday, 16 August 2007, 04:02 GMT
My first re-sync :)

Only one small change; it should work with SVN as of 2007-08-15.
Comment by Dieter (dip) - Saturday, 18 August 2007, 20:29 GMT
The last patch does not work. It misses the changes in alarm_menu.c (which were included in the previous patch).
Comment by Jake Melvin (Monkeytamer) - Tuesday, 21 August 2007, 00:04 GMT
That's odd. It applies just fine for me on a Gigabeat F40.
Comment by Mike Kasberg (digerati1338) - Thursday, 30 August 2007, 01:45 GMT
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.
Comment by Dieter (dip) - Sunday, 02 September 2007, 08:27 GMT
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.
Comment by Travis Tooke (tdtooke) - Friday, 14 September 2007, 03:10 GMT
sync
Comment by Akio Idehara (idak) - Friday, 14 September 2007, 15:14 GMT
resync and fix buggy jumping strings
Comment by Travis Tooke (tdtooke) - Friday, 14 September 2007, 17:37 GMT
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.
Comment by Akio Idehara (idak) - Friday, 14 September 2007, 22:39 GMT
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>
---
Comment by Akio Idehara (idak) - Friday, 14 September 2007, 23:38 GMT
I change from LCD_WIDTH to gwps->display->width for rwps.
Comment by Travis Tooke (tdtooke) - Saturday, 15 September 2007, 05:45 GMT
I can definitely see that bug now with that test wps! Thanks for the fix!
Comment by Anonymous Submitter - Monday, 24 September 2007, 14:07 GMT
sync
Comment by Chris Banes (senab) - Thursday, 27 September 2007, 19:01 GMT
sync
Comment by Akio Idehara (idak) - Saturday, 29 September 2007, 04:02 GMT
A long string was displayed protruding to the right when new track info was loaded.
So I fix it.
Comment by Akio Idehara (idak) - Tuesday, 02 October 2007, 00:48 GMT
more suitable for rwps.
Comment by Martin Buck (foobar12345) - Thursday, 04 October 2007, 12:24 GMT
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?
Comment by Akio Idehara (idak) - Friday, 05 October 2007, 16:33 GMT
add lcd-charcell.c bogus support.
Now all target can be supposed to use this patch...
Comment by Travis Tooke (tdtooke) - Saturday, 06 October 2007, 00:01 GMT
@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.
Comment by Travis Tooke (tdtooke) - Saturday, 06 October 2007, 00:10 GMT
Sorry for the double post but this just occurred to me, I'd still go with display->width on that instead of scroll_width.
Comment by Akio Idehara (idak) - Saturday, 06 October 2007, 01:26 GMT
Does this patch fix that problem?
Comment by Akio Idehara (idak) - Saturday, 06 October 2007, 03:41 GMT
Fix clearing the center aligned string.
Sorry for above buggy patch...
Comment by Travis Tooke (tdtooke) - Sunday, 07 October 2007, 10:58 GMT
@idak, what do you think of something like:
if (left_width || center_width || right_width) {
display->set_drawmode(DRMODE_SOLID|DRMODE_INVERSEVID);
display->fillrect(left_xpos, ypos, display->width, string_height);
display->set_drawmode(DRMODE_SOLID);
}
Comment by Akio Idehara (idak) - Sunday, 07 October 2007, 11:27 GMT
@tdtooke, Can I have your test wps, please?
I don't understand what has occurred...
Comment by Travis Tooke (tdtooke) - Sunday, 07 October 2007, 17:18 GMT
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.;-)
Comment by Jacob Brooks (jac0b) - Thursday, 18 October 2007, 23:36 GMT
Out of sync, I tried to figure out the problem but it looks like a code change and I have no experience coding.
Comment by Akio Idehara (idak) - Friday, 19 October 2007, 08:07 GMT
sync
Comment by Jacob Brooks (jac0b) - Sunday, 21 October 2007, 20:37 GMT
getting a error on main_menu.c, I looked for the proper line but there must have been another code change.
Comment by Thomas Martitz (kugel.) - Sunday, 21 October 2007, 20:49 GMT
I just igonired that HUNK, with success :)
Comment by Jacob Brooks (jac0b) - Sunday, 21 October 2007, 23:29 GMT
it compiled but does it work?
Comment by Thomas Martitz (kugel.) - Sunday, 21 October 2007, 23:41 GMT
If you mean, that all my themes are working as they should, yes! ;)
Comment by Travis Tooke (tdtooke) - Sunday, 21 October 2007, 23:48 GMT
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.
Comment by Nick Brackley (darksaboteur) - Wednesday, 24 October 2007, 01:48 GMT
Along with main_menu.c failing, debug_menu.c and scroll_engine.h now fail. Could someone please resync?
Comment by Akio Idehara (idak) - Wednesday, 24 October 2007, 10:40 GMT
sync
Comment by Jacob Brooks (jac0b) - Monday, 12 November 2007, 23:15 GMT
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
Comment by Nicolas Pennequin (nicolas_p) - Monday, 12 November 2007, 23:19 GMT
that's because part of the patch was committed. Should be an easy sync.
Comment by Jacob Brooks (jac0b) - Monday, 12 November 2007, 23:25 GMT
Yeah I saw that but why only left margins why just commit this patch?
Comment by Nicolas Pennequin (nicolas_p) - Monday, 12 November 2007, 23:31 GMT
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.
Comment by Travis Tooke (tdtooke) - Tuesday, 13 November 2007, 01:03 GMT
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?
Comment by Travis Tooke (tdtooke) - Tuesday, 13 November 2007, 01:07 GMT
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
Comment by Travis Tooke (tdtooke) - Tuesday, 13 November 2007, 04:22 GMT
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.
Comment by Travis Tooke (tdtooke) - Wednesday, 14 November 2007, 10:36 GMT
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.

Loading...