Rockbox

Tasklist

FS#4733 - Multifont

Attached to Project: Rockbox
Opened by Ben Keroack (bk) - Sunday, 26 February 2006, 01:46 GMT
Last edited by Paul Louden (Llorean) - Thursday, 02 October 2008, 21:02 GMT
Task Type Patches
Category Applications
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 2
Private No

Details

This is a preliminary test version of multifont support (v01) as a diff against current CVS HEAD. This allows for multiple fonts to be loaded and used non-simultaneously with 3-5 defined contexts depending upon target.

Fonts are specified in the theme .cfg with the following tags: browserfont, wpsfont, menufont, tunerfont, recordfont. The old "font" tag is ignored. A font selected through General Settings->Display->Browse Fonts override all current theme settings, emulating the old behavior. Plugins use sysfont.

NOTE: Recording screen drawing is currently somewhat broken. Actual recording should be unaffected.
This task depends upon

Closed by  Paul Louden (Llorean)
Thursday, 02 October 2008, 21:02 GMT
Reason for closing:  Later
Additional comments about closing:  This task is no longer attempting to address getting Multifont in a state for inclusion. The idea of displaying multiple fonts is good, but this implementation is not progressing. If someone wishes to fix this patch to work, or start a new one, come to IRC or the -dev mailing list and it can be reopened for you, and the hurdles remaining discussed.
Comment by Paul van der Heu (paulheu) - Thursday, 16 March 2006, 13:04 GMT
You should add remotefont to that list IMO..
Comment by Paul van der Heu (paulheu) - Sunday, 19 March 2006, 09:42 GMT
I ran into a problem which is probably related to this patch; when in settings the font will always switch to SYSFONT when displaying/changing settings. when you backup to the settings menu or exit the set font is used again..
Comment by Ben Keroack (bk) - Sunday, 19 March 2006, 17:46 GMT
Are you using the status bar? The patch posted here doesn't deal properly with status bar font settings and so can corrupt certain screens with SYSFONT.
Comment by Ben Keroack (bk) - Monday, 03 April 2006, 22:01 GMT
Now with summer coming along in the northern hemisphere I'll not have as much time to work on this (and it works pretty well for me in my private builds that there's less motivation to get the mergable version out the door).

Attached is an updated patch that should apply cleanly against today's CVS. It includes 45k font buffers, bumped config version and a small convbdf fix.

Here's a rough TODO list for multifont-2.0 which may or may not be ready for 3.1:

* Font cache overhaul
- Unified font cache buffer
- LRU/cache code allows variable-length entry data payloads (for use w/ mixed fonts)
- Make sure not to break unicode or non-Western codepages
* UI
- GUI font choosers for each font type
- Stop ignoring the old "font:" cfg tag, make it work as before
- Fix the FONT_PLUGIN stuff so that plugins consistently use the plugin font
* Other
- Implement an optional remotefont, figure out if a GUI chooser is workable for this
Comment by P.I.Julius (pijulius) - Tuesday, 29 August 2006, 18:57 GMT
Here it is my sync for the latest cvs ver, please note that I didn't tested it fully but it seems to work pretty well, at least for me.
Comment by P.I.Julius (pijulius) - Friday, 29 September 2006, 12:28 GMT
Sync again
Comment by P.I.Julius (pijulius) - Saturday, 30 September 2006, 11:27 GMT
Fix the theme loader so if you load a theme with only "font" declaration in the .cfg file all the custom fonts will be set to that one, this way you can use the new themes which support different fonts for list/menus/wps and old themes which only have the "font" declaration in their config file.
Comment by Max Weninger (maxwen) - Saturday, 30 September 2006, 11:32 GMT
Just tried to post the same fix :-)
You are to fast for me
Comment by Max Weninger (maxwen) - Tuesday, 31 October 2006, 01:01 GMT
Since the config space becomes to small if you use many fonts
I changed the settings storage format of all fonts.

Instead of always allocating MAX_FILENAME and wasting space
the fonts are stored in the following format

%02d:%s e.g. 07:helvR14 which is <length>:<font>

This saves in this example 10 bytes for a font
I also added reseting all fonts if a theme is loaded

This requires the mutlifont patch
Comment by Max Weninger (maxwen) - Monday, 13 November 2006, 01:39 GMT
Found a bug in the my last patch if font file name has exactly the
now maximum allowed length of 17 chars
Comment by Billy (ipodfoo) - Saturday, 16 December 2006, 17:47 GMT
Seem to have a problem with the text viewer (viewer.c) (See attached pic).The text seems spaced at 8 pixels for some reason. Im not 100% sure which patch causes it, I use:

custom display width (#5898),
multifont (#4733),
multiple user fonts (#5901),
custom list menu (#5899)
custom line (#5900)

Text is fine in the menu of the plugin.
Comment by Billy (ipodfoo) - Saturday, 16 December 2006, 18:06 GMT
Forgot to mention. Text gets clipped only on the right side of ALL plugins (including the viewer.c). (See pic for examples.)
Comment by Billy (ipodfoo) - Sunday, 17 December 2006, 16:51 GMT
Hi. Did some updates to my build. Seems the spacing issue in viewer.c is solved now but the clipping still occurs on the right side.
Comment by Billy (ipodfoo) - Sunday, 17 December 2006, 19:39 GMT
By the way, seems the squished text happens in the sim but not on the actual player.
Comment by Max Weninger (maxwen) - Sunday, 28 January 2007, 16:06 GMT
first try of supporting the new settings code
tried to keep it backward compatible with old themes
by supporting single font themes and multifont themes
Comment by Max Weninger (maxwen) - Sunday, 28 January 2007, 16:43 GMT
same for the userfonts patch
Comment by Max Weninger (maxwen) - Sunday, 28 January 2007, 17:02 GMT
oops tunerfont was stored wrong
Comment by Max Weninger (maxwen) - Sunday, 28 January 2007, 17:11 GMT
forgot to include the changes needed in settings_list.c
Comment by Flake (Flake) - Thursday, 01 February 2007, 03:01 GMT
can anybody adapt multifont patch for the current code, please
with the build date it for
Comment by Robert Cotey (coteyr) - Monday, 05 February 2007, 04:47 GMT
I think the 20070126 patch is a bit off. It looks like it expects another patch to be present. For instance

--- rockbox_svn.orig/apps/filetree.c
+++ rockbox_svn/apps/filetree.c
@@ -528,7 +528,7 @@ int ft_enter(struct tree_context* c)
font_load(buf, FONT_RECORD);
set_file(buf, (char *)global_settings.recordfont, MAX_FILENAME);
#endif
-
+ /* dont set userfonts because of the buffer problem */
break;

case TREE_ATTR_KBD:

looks for FONT_RECORD but that shouldn't exist yet?
perhaps I am applying the patch incorectly.

Comment by Max Weninger (maxwen) - Monday, 05 February 2007, 10:52 GMT
multifont-userfonts require multifont to be applied first
Comment by Mads Michelsen (madsic) - Thursday, 22 February 2007, 03:54 GMT
I tried patching using the 20070131 source using first customdisplaywidth (20070116) and then multifont (20070128) (and then onto mf-uf etc.). I can get every other julius patch to work (more or less) but multifont is giving rejects in five different files and (I suspect) is ultimately the cause of several errors. I would try with the source from the 28th if it wasn't five in the morning.... I have also tried manually inserting the rejects which seemed doable but as I said the end result doesn't hold up.
Comment by Max Weninger (maxwen) - Sunday, 11 March 2007, 06:59 GMT
Quick and dirty sync for 20070310
Comment by Max Weninger (maxwen) - Sunday, 11 March 2007, 06:59 GMT
same for userfonts
Comment by Hepdog (007quick) - Thursday, 15 March 2007, 20:00 GMT
Does not patch correctly anymore, needs a sync to svn!
Comment by Max Weninger (maxwen) - Tuesday, 20 March 2007, 07:43 GMT
sync against 20070320
Comment by Hepdog (007quick) - Wednesday, 21 March 2007, 17:04 GMT
Question, Which patches do I need in order to get this to work! A list would be great. I am trying to patch this with no succes!
Comment by Hepdog (007quick) - Monday, 26 March 2007, 17:48 GMT
Needs a sync again! I think some files are missing now!
Comment by Travis Tooke (tdtooke) - Sunday, 01 April 2007, 07:30 GMT
I have a quick question. I check out an older revision so I can use these most excellent patches and on the multifont-userfonts patch I always get a malformed patch error for the very first section of that patch. Looking at it it seems to me that all that part does is add the comment "/* dont set userfonts because of the buffer problem */". Is that correct? I work around this problem by just removing that section from the patch and everything works out fine. Since I don't see anyone else mentioning this happening I will assume I'm somehow doing something wrong. Thoughts?
Comment by Max Weninger (maxwen) - Sunday, 01 April 2007, 08:18 GMT
correct
Comment by Travis Tooke (tdtooke) - Sunday, 01 April 2007, 21:08 GMT
Not to be a nag, but.. Is that correct as in my assumption that the first section just adds that comment, or correct as in I'm doing something wrong?
Comment by Flake (Flake) - Thursday, 05 April 2007, 23:20 GMT
need a sync please
Comment by TheKind (TheKind) - Saturday, 21 April 2007, 08:25 GMT
The WPS code is definitely different than before.
Maybe a good coder could try to upadte this patch ?
Comment by Max Weninger (maxwen) - Thursday, 10 May 2007, 05:53 GMT
synced against 20070430
Comment by danny (sldonmtns) - Saturday, 12 May 2007, 23:25 GMT
I'm not able to apply the userfonts patch. I think it may be because the patch points towards rockbox_svn. Also, is this patch still needed, or do you just need the mutifont patch now?
Comment by Max Weninger (maxwen) - Sunday, 13 May 2007, 21:07 GMT
Use the correct -p<num> (-p1 should do it) option to patch to strip the rockbox_svn prefix
And you need the userfonts patch only if you want to use the customline patch
Comment by Max Weninger (maxwen) - Tuesday, 15 May 2007, 08:56 GMT
synced
Comment by Josh Drizin (krazykit) - Monday, 04 June 2007, 17:26 GMT
this no longer patches cleanly. It fails on recent svn:

firmware/drivers/lcd-remote-2bit-vi.c
apps/plugins/chessbox/chessbox.c
Comment by Max Weninger (maxwen) - Friday, 22 June 2007, 14:09 GMT
synced
Comment by Travis Tooke (tdtooke) - Monday, 02 July 2007, 02:20 GMT
synced again
Comment by Matthew Schneider (mschneider) - Monday, 02 July 2007, 19:22 GMT
Just a quick question:
Why is it that the userfonts patch never has to be synced? Is it just that it's dependent on the multifont patch?
Comment by Travis Tooke (tdtooke) - Monday, 02 July 2007, 21:54 GMT
I think that's probably it; it does seem odd though it never needing a sync.
Comment by Max Weninger (maxwen) - Tuesday, 03 July 2007, 10:13 GMT
Yes fortunately the code around the multifont-userfonts patch has not changed for some time :)
Maybe we should merge the two patches
Comment by Nick Brackley (darksaboteur) - Wednesday, 11 July 2007, 11:11 GMT
Merge sounds good. Multifont needs a sync
Comment by Travis Tooke (tdtooke) - Friday, 13 July 2007, 10:10 GMT
What patches are you using in order Nick? This still patches for me. Here's a merged patch:
Comment by Nick Brackley (darksaboteur) - Friday, 13 July 2007, 11:15 GMT
i just tried to apply your patch to the newest SVN source and got
patching file rockbox_svn/firmware/drivers/lcd-remote-2bit-vi.c
Reversed (or previously applied) patch detected! Assume -R? [n]

any ideas?
Comment by Travis Tooke (tdtooke) - Friday, 13 July 2007, 18:26 GMT
I believe you're missing scroll-margins Nick, this patch depends on it. The sync I did before would work with the scroll-margins from the tracker and Max's, I'm pretty sure this one will too since the involved code hasn't changed since then. The exact order I apply my patches is ymargin_scrollinfo, Max's scroll-margins(if you use ymargin_scrollinfo this is the one you need), album art, bmpresize, and then multifont_complete.
Comment by Nick Brackley (darksaboteur) - Saturday, 14 July 2007, 02:29 GMT
I applied the patches in that order and now i get:
patching file apps/gui/gwps-common.c
Reversed (or previously applied) patch detected! Assume -R? [n]

Here are the patches i applied in order with their flyspray numbers (just to check that i applied the right ones)
6796(ymargin) --> 2954(scroll_margin) --> 3045(album art) --> 5697(bmp resize) --> multifont

Thank you so much for your help so far, it is very appreciated :)
Comment by Travis Tooke (tdtooke) - Saturday, 14 July 2007, 23:24 GMT
I can't duplicate that error on my end Nick. I was thinking that maybe the problem was that you were using scroll-margins from the tracker(you need Max's version if you use ymargin_scrollinfo) with ymargin but when I tried it that way it still applied cleanly. The only thing I can suggest is for you to completely erase all your source code and redownload it from svn. It's possible that somewhere along the line something was added from previous patches that you weren't able to remove with -R since the patch failed(just guessing at this point).
Comment by Nick Brackley (darksaboteur) - Sunday, 15 July 2007, 14:24 GMT
The problem appears to have fixed itself :) I updated to latest SVN (i was only a couple of revisions behind) and it has all patched and compiled successfully. Thank you for your help :D
Comment by Travis Tooke (tdtooke) - Thursday, 19 July 2007, 00:24 GMT
Small update, removed display->setfont(FONT_UI); from gui_quickscreen_draw in apps/gui/quickscreen.c. This fixes the bug where the font changes upon changing anything in the quickscreen or simply exiting back into the WPS.
Comment by Nick Brackley (darksaboteur) - Thursday, 19 July 2007, 06:40 GMT
New patch fails to apply with:

patching file firmware/export/lcd.h.orig
Hunk #2 FAILED at 395.
1 out of 2 hunks FAILED -- saving rejects to file firmware/export/lcd.h.orig.rej

Just to check that its not an error at my end, where can I get Max's scroll-margins from?
Thanks again, Nick
Comment by Travis Tooke (tdtooke) - Thursday, 19 July 2007, 06:51 GMT
You can get it from http://rockbox.webhop.org/ under Builds based on SVN repository from 20070622. You should bookmark that page, Maxland always has the best goodies :)
Comment by Travis Tooke (tdtooke) - Thursday, 19 July 2007, 06:54 GMT
Oh, btw, it's not absolutely necessary for you to use ymargin_scrollinfo, and Max's scrolling-margins. I personally recommend that because putting ymargin into the scrollinfo struct just makes sense to me, but if you're having problems with that you can not use ymargin_scrollinfo and just use scrolling-margins from the tracker.
Comment by Nick Brackley (darksaboteur) - Thursday, 19 July 2007, 06:56 GMT
I will look at that thank you.
My concern was that it is failing on a .orig file which to my knowledge should be unaffected by patches, when looking at the code it seems "int y_margin". As in my opinion it would be best if either Max's or the trackers patch could be used and by the looks of it, it shouldn't be too hard to fix. If I'm completly wrong here please let me know so I can stop pestering people....
Comment by Travis Tooke (tdtooke) - Thursday, 19 July 2007, 07:08 GMT
Major oops on my part Nick. I was very sloppy, it shouldn't be trying to do anything with .orig files. Don't worry about that hunk failure .orig files are just there for reference and aren't actually used for anything. I'll get a clean version up as soon as possible.
Comment by Travis Tooke (tdtooke) - Thursday, 19 July 2007, 07:19 GMT
clean version that doesn't do silly things like patch .orig files.
Comment by Nick Brackley (darksaboteur) - Thursday, 19 July 2007, 07:22 GMT
Patch applies cleanly :)
Thanks for the quick fix Travis
Comment by Max Weninger (maxwen) - Thursday, 19 July 2007, 08:06 GMT
> Small update, removed display->setfont(FONT_UI); from gui_quickscreen_draw in apps/gui/quickscreen.c.
> This fixes the bug where the font changes upon changing anything in the quickscreen or simply exiting back into the WPS.

thanks! :-)
Comment by Max Weninger (maxwen) - Thursday, 19 July 2007, 08:31 GMT
sorry about the scroll_margins confusion. I know its bad style to post patches that depend on
others that are not in the tracker. I started my own scroll_margins patch long time ago
because the original version hat so many problems and failed to sync it with the version in the tracker
Comment by Nick Brackley (darksaboteur) - Saturday, 21 July 2007, 06:09 GMT
Fails On SVN 13948
patching file apps/plugins/rockpaint.c
Hunk #6 FAILED at 933.
1 out of 11 hunks FAILED -- saving rejects to file apps/plugins/rockpaint.c.rej

Sync Please :)
Comment by Travis Tooke (tdtooke) - Sunday, 22 July 2007, 04:56 GMT
not really a sync, just changed one line in the patch so that hunk wouldn't fail.
Comment by Nick Brackley (darksaboteur) - Monday, 23 July 2007, 05:36 GMT
Looking through the patch I found that it modifies screen.c.orig, am I ok to remove this?
Comment by Travis Tooke (tdtooke) - Tuesday, 24 July 2007, 03:16 GMT
Yes, looking at the patch I'm not finding it trying to do that so I can only assume that's some strange side effect from me just manually editing it instead of doing a fresh diff.
Comment by Travis Tooke (tdtooke) - Wednesday, 25 July 2007, 04:23 GMT
proper resync
Comment by Anonymous Submitter - Friday, 31 August 2007, 17:53 GMT
I can't apply the patch to a untouched svn (as of yesterday). Can someone fix/resync it? I can't do that, I don't have the knowledge of the programming languages used in RB. Thanks
Comment by Travis Tooke (tdtooke) - Saturday, 01 September 2007, 11:38 GMT
sync
Comment by Anonymous Submitter - Saturday, 22 September 2007, 20:17 GMT
This needs a sync, due to the changes made in apps/screen_access.c.
Comment by Travis Tooke (tdtooke) - Sunday, 23 September 2007, 19:02 GMT
sync
Comment by Travis Tooke (tdtooke) - Sunday, 30 September 2007, 03:04 GMT
@maxwen, There is a theme in development in the forums that is a mockup of a zune theme that has this weird problem. Occasionally the font color of the menu will take on one of the userfonts from the WPS. I just started looking into this myself, but I thought I'd mention it to you since you're "the pro" here so to speak and would definitely know more about this sort of thing than I would.
Comment by Travis Tooke (tdtooke) - Sunday, 30 September 2007, 06:17 GMT
It's looking like this is in customline instead. It seems that only scrolling customlines can do this. I'm tinkering around in lcd-16.c with the way fgcolor is handled to see what I can do.
Comment by Travis Tooke (tdtooke) - Sunday, 30 September 2007, 14:54 GMT
Nope, it's multifont. I added "gui_list->display->set_foreground(global_settings.fg_color);" to case GUI_LIST_CONTEXT_MENU and case GUI_LIST_CONTEXT_BRWSR in list.c. This seems to fix that, but I cannot stress enough that: I'm completely open to suggestions for a better way of doing this though.
Comment by Thomas Martitz (kugel.) - Monday, 01 October 2007, 15:05 GMT
What does it fix? The fonts get messed up when going from wps context menu back to wps?

Did you monologize? You seemed to talk to someone, but here are only posts from you :O
Comment by Travis Tooke (tdtooke) - Monday, 01 October 2007, 22:32 GMT
I very well may be monologin', but I was aiming all of that at Max (he's forgotten more about multifont than I will every know). There is a theme under development that is a mockup of a zune theme in the forums that had a bug where if you hit menu to escape out of the WPS and go to the menu the font color there would be the font color of one of the customlines from the WPS instead of the default color set by the themes .cfg file. This fixed that. Technically this bug shouldn't have happened, but lately for whatever reason iPods have been very sensitive about this sortof thing with these patches. If you don't have an iPod or never used a theme with noticeably different colors in the WPS then you probably never saw this bug.
Comment by Thomas Martitz (kugel.) - Monday, 01 October 2007, 22:43 GMT
Ok, no I never noticed that bug, I neither own an iPod nor I used that particular theme.

AnywayCan the bug I addressed be fixed?
Comment by Travis Tooke (tdtooke) - Tuesday, 02 October 2007, 00:00 GMT
I'd be more than happy to look at any problem you may be having with this patch, but after scrolling up on this page I can't seem to find a post by you saying what that problem is.
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 00:17 GMT
I have no problem except this one. But this is a common problem, many (if not all) people are having this.

Note: I can only speak for e200, it might be, that this is only present on the e200.

Basically, when you have set a different menufont and wpsfont this problem occurs: When you go from wps to the wps' context menu, the font changes properly (i.e. snap to nimbus), but when you go back from that context menu(or ay submenu) to the wps by pressing play, the font does not change. It remains the menufont, messing the wps up. Only way to get around is to visit the main menu shortly.
Comment by Travis Tooke (tdtooke) - Tuesday, 02 October 2007, 01:11 GMT
I was unaware of that, basically all of the themes that I use with multifont also use customline and customline has a switch case statement setting the userfont that makes that impossible so I never saw this bug. I'm on it, with luck I'll have an updated version up tonight fixing that along with another customline patch which hopefully will kill the last of it's bugs (crossing fingers).
Comment by Travis Tooke (tdtooke) - Tuesday, 02 October 2007, 02:56 GMT
This should fix that, got tired of writing out multifont_complete all the time so I'm just calling this one multifont.
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 07:34 GMT
Cool Thanks.

Just a note: I use custom line as well, however I have this bug. Maybe the problem is caused by custom line?

Anyway, thanks for your effort. I'm gonna test the patch as soon as I get home.
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 19:44 GMT
sync
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 20:03 GMT
F**k that, I can't even copy and paste one single line.

Sorry, this is the real sync.

Note: I had to run in twice (normal, -R, normal again) to apply it as a single patch. No problems with album art, bmpresize, ymargin, scroll-margins and custom line applied before.
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 20:10 GMT
Uhh it doesn't compile. Just 1 line changed in SVN, and such a big impact ;;
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 20:28 GMT
You might not believe it. I failed at syncing again.

I begin to hate 1 line SVN changes...

Ok, this time it's deeply tested. It's applying and compiling.
Comment by Travis Tooke (tdtooke) - Tuesday, 02 October 2007, 22:57 GMT
Heh, I've made a hatchet job of syncing myself a few times ;) So is that bug gone now for you?
Comment by Thomas Martitz (kugel.) - Wednesday, 03 October 2007, 00:02 GMT
It seems to work, the sim doesn't have this bug with only multifont applied.

However, I can't test it in a full custom build with some other patches(including custom line), as ymargin doesn't work atm.

Good job anyway, thanks.
Comment by Travis Tooke (tdtooke) - Monday, 08 October 2007, 07:43 GMT
Sync, adds support for the new robotkitten plugin.
Comment by Victor Karpunin (kva) - Monday, 08 October 2007, 09:08 GMT
I have problems with multifont patch (version from multifont_complete-20070930.patch and later).
After loading simulator StatusBar is empty and then simulator fall. I found that problem with strings "gui_list->display->set_foreground(global_settings.fg_color);" in list.c. Simulator works without them.
Last I tested h3xx simulator with patches: album_art_20071001.patch, scroll-margins_20071006-2.patch, bmpresize-20070430.patch, multifont-20071002c.patch.
Comment by Victor Karpunin (kva) - Monday, 08 October 2007, 09:50 GMT
I have problems with multifont patch (version from multifont_complete-20070930.patch and later).
After loading simulator StatusBar is empty and then simulator fall. I found that problem with strings "gui_list->display->set_foreground(global_settings.fg_color);" in list.c. Simulator works without them.
Last I tested h3xx simulator with patches: album_art_20071001.patch, scroll-margins_20071006-2.patch, bmpresize-20070430.patch, multifont-20071002c.patch.
Comment by Travis Tooke (tdtooke) - Monday, 08 October 2007, 10:36 GMT
I could wrap those lines in some #ifndefs so you won't have them if you compile a simulator but I have to have those lines to fix a bug that iPods have.
Comment by Travis Tooke (tdtooke) - Friday, 12 October 2007, 03:26 GMT
Ok, this should fix you up kva. You won't have to remove those lines now. I believe this may fix some problems non-ipod targets have been having as well since I've added some things that address issues only iPods seem to have.
Comment by Travis Tooke (tdtooke) - Tuesday, 23 October 2007, 02:48 GMT
very slight revision.
Comment by Travis Tooke (tdtooke) - Tuesday, 23 October 2007, 14:17 GMT
update to go with my tentative slight rewrite of customline. If you have problems with this patch just revert to the old one and let me know.
Comment by Travis Tooke (tdtooke) - Wednesday, 24 October 2007, 05:48 GMT
fix warning, fix bug where the font color for the quickscreen can be one of the colors from the WPS. Also back to the old way of doing things since that slight rewrite of customline is not working as I'd hoped.
Comment by Travis Tooke (tdtooke) - Saturday, 27 October 2007, 03:42 GMT
Further work on the bug Thomas reported a while back where if a WPS was multifont but not customline the WPS would not display the proper font. While using this I noticed that going to the quickscreen and back to the WPS when the font reset there would be text artifacts left over. Hopefully this will fix that.
Comment by Konstanin (eK3eKyToPa) - Saturday, 10 November 2007, 11:22 GMT
Please resync the multifont patch, and for what need is the multifont-userfonts patch , is it included in the last releases of multifont patch ( if not please resync it too )
They gives some error when compiling, there is something about alarm clock , I dont understend it at all.
Comment by Travis Tooke (tdtooke) - Sunday, 11 November 2007, 03:31 GMT
sync, and yes, userfonts is included in this.
Comment by harry tu (bookshare) - Tuesday, 25 December 2007, 00:21 GMT
multifont needs a sync
Comment by Thomas Martitz (kugel.) - Sunday, 06 January 2008, 00:02 GMT
tdooke, before we start to adapt this to viewports, let me upload a synced and cleaned up version. Note that I heavily cleaned it up, as well as deleted 1 hunk (the one in tools/convbdf.c), because I couldn't figure out what it does.
Comment by Travis Tooke (tdtooke) - Sunday, 06 January 2008, 10:03 GMT
Ok, here is a rough draft which probably definitely needs improving, but it's a start. This patch now only has one dependency: Viewports http://www.rockbox.org/tracker/task/8385 With the userfonts if you want userfont1 you need to put a 2 for the font because 0 and 1 were used for FONT_SYSFIXED and FONT_UI the way parse_viewports was set up so now 2 through 8 are your userfonts in the wps code corresponding to userfonnts 1 through 7. I tested this out with rayboradio replacing the %e tags with:
%V|265|9|41|15|2|B5B5B5|000000|
%cl:%cM%cp
%V|135|52|175|15|4|F58105|000000|
%al%s%?ia<%ia|%?d2<%d2|>>
%V|135|71|175|15|2|B5B5B5|000000|
%al%s%?id<%id|%?d1<%d1|>>%?iy< (%iy)|>
%V|150|98|160|15|3|F58105|000000|
%al%s%?it<%it|%fn>
%V|135|117|175|15|2|B5B5B5|000000|
%alTrack %pp of %pe - %pc [%pt]
%V|135|135|175|15|2|B5B5B5|000000|
%s%al%?It<Next: %It|%?Fn<Next: %Fn|>>
%V|238|215|62|15|5|333333|000000|
%acVolume: %pv

and everything seemed to work, also I switched to cabbie_unifont to make sure regular themes worked and that worked as well so I think this is ready for you guys to test out.
Comment by Thomas Martitz (kugel.) - Sunday, 06 January 2008, 13:24 GMT
in rockbox_svn/tools/convbdf.c:
- if (sscanf(buf, "COPYRIGHT \"%[^\"]", copyright) != 1) {
+ if (sscanf(buf, "COPYRIGHT \"%s\"", copyright) != 1) {

Can you tell what this is for?
Also, sad that you didn't use my cleaned up version. You have so many trailing spaces and stuff.
Couldn't test yet.
Comment by Thomas Martitz (kugel.) - Sunday, 06 January 2008, 13:41 GMT
Don't you think it's time to combine GUI_LIST_CONTEXT_MENU and GUI_LIST_CONTEXT_BRWSR? I think it's a bit overkill to have both.

Note: I want to make this committable. The font-caching needs to be changed.
Comment by Dave Chapman (linuxstb) - Sunday, 06 January 2008, 14:19 GMT
I´ve only looked through this patch briefly, but it doesn´t appear to implement one of the most important uses for multifont in Rockbox - different fonts on the main LCD and remote LCD in the various screens. Is this possible with this patch?
Comment by Thomas Martitz (kugel.) - Sunday, 06 January 2008, 14:22 GMT
Why not? I think this would be great. I just imagine an extra setting in the themes settings menu, set remote font or something.
Comment by Travis Tooke (tdtooke) - Monday, 07 January 2008, 04:31 GMT
You can have different fonts on the main LCD as it is. Menufont, Browserfont, Pluginfont, WPSfont, Recorderfont, probably some others I can't think of as well. You'll be happy to know that I just updated with your cleaned up patch Thomas. I'll look into adding remote settings, right now this is just cleaned up per Thomas' patch with some other cleanups (me hunting down occurrences of FONT_UI where it shouldn't be).
Comment by Dave Chapman (linuxstb) - Monday, 07 January 2008, 08:15 GMT
I've been thinking a little about different ways to specify fonts, and I wonder what people think about the following:

Replace the screen-specific fonts with a list of fonts, and references to fonts in that list. i.e. the global settings (.cfg file) would contain a "font:" entry "font: fontname1,fontname2,fontname3" (to ensure that there are no missing fonts in the array, and to maintain compatibility with existing .cfg files), and then separate cfg entries specifying which fonts are used for each screen, such as "menufont: 1", "remote_menufont: 2" etc. (0 is reserved as the font number for the system font, so the user-defined fonts start at 1).

This would (I think) simplify the implementation - the core font code (in firmware/) wouldn't care what the fonts are being used for, it would just deal with a variable-sized array of fonts, and if the same font is used multiple times, it would only be loaded once. I think this would also make using a single font cache simpler.

All calls to, for example, display->set_font(FONT_BROWSER) would then be replaced with something like display->set_font(global_settings.font_browser), and the impact of adding a further place where a custom font can be used would just be an integer in the global settings, rather than a font filename.
Comment by Dave Chapman (linuxstb) - Monday, 07 January 2008, 08:30 GMT
Hmm, reading my comment again, it seems that lines such as "display->set_font(global_settings.font_browser)" won't work well - we want different fonts for different displays...

Although when the apps/ code is fully adapted to use viewports, the code probably won't need to call set_font any more - it can just specify the font directly in the viewport struct, and we would have separate viewport structs for each screen.
Comment by Travis Tooke (tdtooke) - Monday, 07 January 2008, 09:01 GMT
I suspect (since separate viewports did indeed solve all my customline woes) that would solve the one gnawing problem this patch has. For some odd reason switching from rayboradio to a non-multifont theme will cause weird behavior, however switching from literally any other multifont theme to a non-multifont one will not. It's a freak of nature, if there is a bug in your code anywhere that theme will find it! I have a small update, updated that switch-case in wps-parser.c and added a default so I didn't have to have that big ugly if statement setting the font if a wrong number was entered for it in the %V tag. I think it might be best if I wait for further developments on viewports for now so I can see what I'm really going to be working with.
Comment by Travis Tooke (tdtooke) - Monday, 07 January 2008, 09:07 GMT
@Thomas, as far as GUI_LIST_CONTEXT_MENU and GUI_LIST_CONTEXT_BRWSR go I agree completely, I do think having both of those is overkill, but as it is if you're going to have both a menufont and a browserfont you have to have that. If it's what is wanted I can kill browserfont.
Comment by Travis Tooke (tdtooke) - Monday, 07 January 2008, 09:15 GMT
I really wish I could edit my posts, sorry for the triple post, but that hunk you were wondering about: no idea why that is there. Unless you're creating fonts you'll never use that if it's what I think it is. I believe that code has to do with checking if you're trying to create a rockbox font from a copyrighted font. At any rate I have no idea why that would need to change.
Comment by David Maliniak (major_works) - Monday, 07 January 2008, 18:30 GMT
As far as the 20080107 version of the patch goes, there's another gnawing problem, which is that it causes make errors in mpegplayer.c.
Comment by Travis Tooke (tdtooke) - Tuesday, 08 January 2008, 05:55 GMT
This one won't do that, also took out getcurfont and made use of Dave's getfont.
Comment by David Maliniak (major_works) - Tuesday, 08 January 2008, 21:58 GMT
Still getting the same errors with this latest version, I'm afraid:

CC mpegplayer.c
mpegplayer.c: In function 'draw_putsxy_oriented':
mpegplayer.c:461: error: too few arguments to function 'rb->font_get_width'
mpegplayer.c:468: error: too few arguments to function 'rb->font_get_bits'

This was with clean SVN as well as bmp-resize patch and Viewports patch (latest version reflecting yesterday's commits).

Yesterday on IRC, Kugel opined that there's a hunk missing that's causing these errors.
Comment by Travis Tooke (tdtooke) - Tuesday, 08 January 2008, 23:07 GMT
Oops, now I know what your problem is, on line 461 change it to:
width = rb->font_get_width(pf, ch, current_vp->font);
and on 468 make it:
bits = rb->font_get_bits(pf, ch, current_vp->font);

Sorry about that, here's a good patch:
Comment by David Maliniak (major_works) - Wednesday, 09 January 2008, 02:42 GMT
Here's what I get with the latest version:

CC mpegplayer.c
mpegplayer.c: In function 'draw_putsxy_oriented':
mpegplayer.c:461: error: 'current_vp' undeclared (first use in this function)
mpegplayer.c:461: error: (Each undeclared identifier is reported only once
mpegplayer.c:461: error: for each function it appears in.)
Comment by Thomas Martitz (kugel.) - Wednesday, 09 January 2008, 02:44 GMT
Try FONT_UI instead of current_vp->font. This is what I have.
Comment by Travis Tooke (tdtooke) - Wednesday, 09 January 2008, 04:33 GMT
What target are you guys on, sounds like I missed more than mpegplayer.c. FONT_PLUGIN might be a better choice, but FONT_UI will always work.
Comment by David Maliniak (major_works) - Wednesday, 09 January 2008, 15:43 GMT
FONT_UI in mpegplayer.c did the trick for me, thanks.

I'm compiling for Sansa e200.
Comment by David Maliniak (major_works) - Wednesday, 09 January 2008, 16:44 GMT
FONT_UI in mpegplayer.c did the trick for me, thanks.

I'm compiling for Sansa e200.
Comment by Travis Tooke (tdtooke) - Monday, 14 January 2008, 12:30 GMT
If you want JDGordon's new list patch to work remove the hunks for list.c and list.h. As soon as I can I will update this. It's official, FONT_BROWSER will die.
Comment by Travis Tooke (tdtooke) - Monday, 14 January 2008, 12:44 GMT
Before I do that though just to be sure, is that what is wanted? Basically the deal is that FONT_BROWSER comes into play when you're in the database or the file browser where the lists tend to be longish so in those instances you may want a smaller font than what you have for FONT_MENU which is mostly used in instances where the list does not need to scroll.
Comment by Travis Tooke (tdtooke) - Monday, 14 January 2008, 17:46 GMT
Use this patch if you're using the aforementioned patch, just be sure to apply that one since this now depends on it, if not you can just use the older one. I'll get this properly sorted out later. I'm thinking I will use menufont for the old cases of menu and browser and maybe create remotefont.
Comment by Josh Dionne (JTD121) - Saturday, 19 January 2008, 05:29 GMT
So....Do we use the 2008108a and the 20081114? Or just one? Attempting to build (first timer) on H300 as target.
Comment by Travis Tooke (tdtooke) - Saturday, 19 January 2008, 11:18 GMT
Use the latest one, you'll want to manually change FONT_UI to FONT_MENU in apps/gui/list.c for the viewports and have it set the font to FONT_MENU in apps/gui/quickscreen.c. I haven't gotten around yet to putting up a proper sync which will be viewports_list friendly. Will probably do that within the next few days.
Comment by Travis Tooke (tdtooke) - Sunday, 20 January 2008, 19:49 GMT
This one depends on viewports and viewports_list. If you don't use viewports_list you should be able to just ignore the hunk failures for apps/gui/list.c since what they want to change isn't there anyway.
Comment by Jacob Brooks (jac0b) - Wednesday, 23 January 2008, 02:47 GMT
Resynced I think.
Comment by Travis Tooke (tdtooke) - Thursday, 07 February 2008, 11:38 GMT
Browserfont has been completely removed in this one, do not use browserfont anymore in your cfgs. Use menufont. For the viewports list if you specify "system" you will get FONT_SYSFIXED, anything else will get FONT_MENU. Fully adapting that to viewports list so that you can use a real font name is on my todo list, this is just a start.
Comment by Travis Tooke (tdtooke) - Thursday, 07 February 2008, 12:49 GMT
I'm wanting to remove FONT_PLUGIN as well now that I'm thinking about it. It looks like the patch author had intended to add that to the settings so it would be configurable, but as it is now it's just setting it to FONT_SYSFIXED. I really don't want to add it to the settings myself because we're really pushing it on loading fonts with what we have and the userfonts as it is. Thoughts?
Comment by Jacob Brooks (jac0b) - Tuesday, 12 February 2008, 03:12 GMT
I got some errors on this one. Should I use this then the viewports or viewports then this?

http://pastebin.com/m35b25191
Comment by Travis Tooke (tdtooke) - Tuesday, 12 February 2008, 18:02 GMT
You do have to apply the viewports patch first. If you're going to use the viewports list patch you need to apply that before as well, but if you don't use that one you can just ignore the 2 hunk failures on apps/gui/list.c since you don't need them in that case.
Comment by Jacob Brooks (jac0b) - Tuesday, 12 February 2008, 23:09 GMT
I get this error when patching with the "viewports-wps-v1.diff" already applied.

can't find file to patch at input line 325
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff -u -r apps/gui/viewport.c apps/gui/viewport.c
|--- apps/gui/viewport.c 2008-02-07 05:12:48.000000000 -0600
|+++ apps/gui/viewport.c 2008-02-07 05:09:32.000000000 -0600
--------------------------
File to patch:
Comment by Jacob Brooks (jac0b) - Tuesday, 12 February 2008, 23:18 GMT
I think I fixed it. I just took out the viewport.c entry on line 322 and it compiles fine.
Comment by Sasha Khamkov (sanusart) - Tuesday, 12 February 2008, 23:23 GMT
Jacob, all the pathes are fine (vieports_wps, multifont, albomart-smooth-resize).
It's your SVN messed up I think.
Try to download a fresh copy or at least revert it and patch again.
Comment by Michael Hahn (disorganizer) - Wednesday, 13 February 2008, 22:44 GMT
ive got the same problem as jacob with a fresh svn.
viewport.c is nonexistant.

and that file is not created by viewports-wps1 patch, so where should it come from?
Comment by Michael Hahn (disorganizer) - Wednesday, 13 February 2008, 22:46 GMT
found it. viewport.c is added by the viewports-list patch.
Comment by Jacob Brooks (jac0b) - Wednesday, 13 February 2008, 23:01 GMT
So it can be disregarded if you are not using the viewports list patch?
Comment by Michael Hahn (disorganizer) - Wednesday, 27 February 2008, 15:15 GMT
patch is out of sync with 16431. it is easy to fix most of the problems, but:
in list.c the patch tries to find a line where the font is defied for the parent-viewports of the remote and the lcd-display.
those lines do not exist in the list.c on my fresh installation (with the  FS#8385 =viewports-wps-v1.diff and  FS#8457 =listvp.3.diff applied).
the definition line of the file you want to patch looks like:
.y = 8,
.width = LCD_WIDTH,
.height = (LCD_HEIGHT-8),
.font = FONT_UI,
.drawmode = DRMODE_SOLID,
.xmargin = 0,
.ymargin = 0,
but original coresponding line (i think) looks only like:
.x = 0,
.y = 0,
.width = LCD_WIDTH,
.height = LCD_HEIGHT

and for the remote, you expect:
.y = 8,
.width = LCD_REMOTE_WIDTH,
.height = (LCD_REMOTE_HEIGHT-8),
.font = FONT_UI,
.drawmode = DRMODE_SOLID,
.xmargin = 0,
.ymargin = 0,
but no line with LCD_REMOTE_WIDTH or LCD_REMOTE_HEIGHT exist in list.c

Comment by Michael Hahn (disorganizer) - Thursday, 28 February 2008, 16:09 GMT
saying the above, wouldnt it make sense to just not patch list.c at all?
instead, mf should set FONT_UI=FONT_MENU whenever FONT_MENU is set and just leave modules only using FONT_UI alone.
Comment by Michael Hahn (disorganizer) - Thursday, 28 February 2008, 20:12 GMT
next question:
correct me if i dont understand the code right:
you use maxfonts*fontsize as buffer, and put all fonts in the memory.
(userfont1-7, font-wps, font-menu, font-tuner, ...).

and then the font-wps variables are used to index the fontcache, correct?

from my experience as rb user normally you have multiple identical fonts defined in the config. for example font-tuner, font-menu, userfont-1 as nimbus-12.

now as far as i understood the code, you buffer every font in advance, but dont check whether it is already buffered.
wouldnt it be a good idea to check each font whether it is already buffered, and then just use the index to the already buffered font as index for the new font type?

an example would be:
userfont1=nimbus-12
userfont2=nimbus-14
userfont3=nimbus-12
menu-font=nimbus-12

cache would look like:
1: nimbus-12
2: nimbus-14

indexes would look like:
userfont1=1
userfont2=2
userfont3=1
menu-font=1
Comment by Travis Tooke (tdtooke) - Friday, 29 February 2008, 05:08 GMT
You are exactly right, and I agree that would be an excellent idea. I recently took out browserfont to ease up on the buffer and something as obvious as that didn't even occur to me. :-/
Comment by Travis Tooke (tdtooke) - Friday, 29 February 2008, 05:12 GMT
On defining FONT_UI as FONT_MENU I was thinking of just getting rid of FONT_MENU and using FONT_UI since FONT_BROWSER is gone and we're no longer checking context and all that. By all means feel free to post a patch with changes if you want to change something.
Comment by Michael Hahn (disorganizer) - Friday, 29 February 2008, 06:46 GMT
in fact i have no idea of how to program c... im just able to understand what i see in the code a bit so i can understand the algorithms, but i dont know the syntax or how to do it myself *g*

the only thing i vote for is that we get a decission on which fonts to use for what, because we need vp-list, vp, multifont etc to be consistent in their font usage. if we decide to use font_ui that would be a good idea because we dont have to care about compatiblility. but that cant be decided for mf alone, but the other dependent patches must also know about it.
Comment by Michael Hahn (disorganizer) - Friday, 29 February 2008, 23:00 GMT
now i think i managed to sync multifont-20080212.patch to work with listvp3 and viewports-wps1.
Comment by Michael Hahn (disorganizer) - Wednesday, 05 March 2008, 19:04 GMT
hmm. i tried to sync it and remove all menufont and font_menu references. (aka replacing them back with font_ui).
but somehow it compiled well but the simulator just crashes.

so obviously someone else with much more programming knowledge needs to do this.
what i dont understand: why dont i get an error message from the simulator? it just ends without any error message.... :-(
Comment by Michael Hahn (disorganizer) - Friday, 07 March 2008, 08:40 GMT
maybe we should not go through the same approach as the viewport-list patch went through.

propbably we should minimize the patch to only the minimum functionality (loading and saving of the fontsettings, giving other functions procedures to identify fonts and help parsing etc, define variable and set them).
so with almost no visual impact except that the functions are inside rockbox to handle multiple fonts.
giving the rest of rockbox a "framework" to use for working with multiple fonts.
font_ui should be maintained as primary font instead of font_menu.

after that "minimized" version is committed, we can do patches for each open part of the work:
one for the wps-multifonting, one for list-multifonting, one for plugin-multifonting, ...

but this is just a proposal, as im not a programmer and thus cant do this anyways :-)
Comment by Michael Hahn (disorganizer) - Saturday, 08 March 2008, 02:56 GMT
ok, back to the original plan for the moment until some more capable people fill in:

i managed to patch the original patch to:
1) compile with the svn after the vp-list commit
2) need only the wps1-patch
and most important:
3) not use menufont! only use fontui (font tag in configfile).

Comment by Michael Hahn (disorganizer) - Sunday, 09 March 2008, 13:48 GMT
synced to 16589
Comment by Jacob Brooks (jac0b) - Monday, 10 March 2008, 16:12 GMT
The list on my build is using the system font and not the font I specified. I tried the fontui & menufont. Am I doing something wrong?
Comment by David Maliniak (major_works) - Monday, 10 March 2008, 19:41 GMT
Out of sync yet again, unfortunately.
Comment by Michael Hahn (disorganizer) - Thursday, 13 March 2008, 16:38 GMT
should be synced to 16652. but no guarantee as i cant test with a remote.
Comment by Jacob Brooks (jac0b) - Saturday, 15 March 2008, 15:07 GMT
Updated the mpegplayer section of the patch. It was causing it to crash if you set the "font:" tag in the theme.cfg and tried to turn up the volume or FF the video.
Comment by Thomas Martitz (kugel.) - Saturday, 15 March 2008, 17:14 GMT
Please take look at my latest comment in Viewports. I maybe prepare a patch for that.
Comment by Michael Hahn (disorganizer) - Monday, 17 March 2008, 21:07 GMT
hopefully synced to revision 16685 WITH THE viewports-wps-v2 PATCH!

note that the newer syncs will need at least the wps-vp patch "viewports-wps-v2.diff" to work.
im not checking the syncs against the old version of the wps vp patch.
Comment by Michael Hahn (disorganizer) - Friday, 21 March 2008, 23:09 GMT
out of sync since the wps patch got commited.
i cant resync it, so unless someone else kicks in this will stay broken for the moment.
Comment by Thomas Martitz (kugel.) - Saturday, 22 March 2008, 04:18 GMT
I'm gonna uploade a sync'd version tomorrow. I plan to revert "3) not use menufont! only use fontui (font tag in configfile).", since it basically breaks all themes. menufont is better to seperate from the other font types anyway.
Comment by Travis Tooke (tdtooke) - Saturday, 22 March 2008, 05:31 GMT
This version only has font_ui and the userfonts. As I understand it the various fonts for this and that can be implemented for those screens via the appropriate viewports as viewports is further implemented. It should be really easy to add that as more things are converted to viewports. Apply with -p0, this patch depends on NOTHING! yay
Comment by Travis Tooke (tdtooke) - Saturday, 22 March 2008, 05:34 GMT
It might be a good idea to make 0 font_ui and 1 and up the userfonts instead of 0 being font_sysfixed and 1 being font_ui, or we could go with the actual names font_ui, userfont1, etc.. If you guys want to go with that feel free to change any and everything or just let me know and I'll do it eventually.
Comment by Dave Chapman (linuxstb) - Saturday, 22 March 2008, 09:32 GMT
IMO, this patch should be removing FONT_UI, and replacing it by multiple fonts. So I think it makes sense to keep FONT_SYSFIXED as 0, and the user fonts as 1 upwards.

In your proposal (making FONT_UI font 0), what would FONT_SYSFIXED be?
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 10:05 GMT
propably the font naming should be dropped completely and replaced by pure font indexing.
sysfixed would be font[0] then (or whatever). all other fonts would be font[1],....

but as to be seen on irc log of yesterday / this morning we first need to think about a good font caching algorythm :-)
Comment by Travis Tooke (tdtooke) - Saturday, 22 March 2008, 11:42 GMT
@Dave Chapman: I was thinking of making FONT_SYSFIXED as default in that switch case so anything other than 0-7 would be it. Really the only reason I was thinking of switching it like I said was because it would be more like customline was with a 1 being userfont1 and so on. It's just what I was used to, I'm extremely bendable on that point. I put FONT_UI back mainly to serve as the menu font for now since it has excellent backwards compatibility. The way it worked before is it would check for the other fonts in the config (menufont, recorderfont, etc.) and set useMultiFont to true if they were there or false if they weren't. If it was false then all those fonts would be loaded as whatever the regular font file specified in the config was. I never could get that to work exactly right all the time, not ashamed to say I have no idea why. I did do alot of dodgy stuff to try and fix customline bugs so it's quite possible I injected that bug myself. If anyone wants to look at one of the older patches to see what the problem was I'd be grateful. I was going to add fonts for the other screens as they were converted over to viewports.

@me: In some ways like with the glyphcache we kinda are doing that...sort of. I think we'd have a riot on our hands if we took font names out of the picture completely though, could be wrong, who knows. If we're headed in the direction I think we're headed in we do need a good font caching system though.

@Thomas Martitz: Why don't you go ahead and up what you have, I have a strong suspicion at this point that your work probably is making more sense than mine..;-)
Comment by Travis Tooke (tdtooke) - Saturday, 22 March 2008, 13:39 GMT
My apologies to those that downloaded that last patch. Apparently I saw fit to zero out the userfonts after you loaded them so when you next started your player you'd have FONT_SYSFIXED instead of your userfonts. No idea why I did that?? Ooops..
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 15:53 GMT
there was some discussion yesterday on irc (see logs) about font caching.
the main thing is: how can we find an efficient algorythm for caching fonts without wasting memory.

i now also agree that using font=font1,font2,font3 and then use font indexes (1,2,3 for font1,font2,font3) would propably be the most flexible way to implement things.
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 20:47 GMT Comment by Thomas Martitz (kugel.) - Saturday, 22 March 2008, 20:59 GMT
tdtooke, am I right that you threw FONT_TUNER etc away and just kept FONT_UI and the userfonts?
Comment by Thomas Martitz (kugel.) - Saturday, 22 March 2008, 21:03 GMT
Ooops, yea, didn't read your previous comment (Saturday, 22 March 2008, 05:31 GMT) properly :/

Well, this will break up themes even more, but that's actually ok. It seems that only userfonts will kept anyway (using a fonts:font1|font2|...|fontn) construct. I'm fine with it.

Though, I'd still like to have costumizable fonts for record screen or radio, so we should reimplement that. Of course in a different way, so that those screens would use userfonts as well.
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 21:44 GMT
i wonder if it would make sense to have seperate directives for loading the fonts and for mapping screens to fonts.
so

font=nimbus-12.fnt,nimbus-14.fnt
tunerfont=1
menufont=2
listfont=0

would assign the tunerfont to nimbus-12 and menufont to nimbus-14. listfont would by the internal font.
this would also allow easy removal of a font from the list (except for wps and other simultanious-multifonted screens).

the wps-problem could only be addressed by also using
wps-font1=1
wps-font2=2
... in the config.
Comment by Thomas Martitz (kugel.) - Saturday, 22 March 2008, 21:49 GMT
I was thinking of doing it in the same way. But I don't see a "wps-problem". The %V tag would refer to the fonts in the same way (1,2 etc). The only problem I'd see is what would happen to the default wps viewport, but I guess it would default to font 1.
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 21:50 GMT
well, wps-font directive could indeed be
wpsfont=1,2 :-)
which makes sense and can be consitent for all uses of font "re"-directives.
example:
font=nimbus-12,nimbus-14,nimbus-16
fontwps=2,3
fontmenu=1
fonttuner=1,2

would in this case use the following font for the viewports
wps:
%V...|1|...=nimbus-14, %V...|2|...=nimbus-16
menu:
%V...|1|...=nimbus-12
tuner:
%V...|1|...=nimbus-12, %V...|2|...=nimbus-14

which could be a consistent easy to parse behaviour for all viewport uses.

sorry for the double post.
Comment by Michael Hahn (disorganizer) - Saturday, 22 March 2008, 21:51 GMT
@kugel: with the wps problem i meant when you remove a font from the list in the wps, you need to manually modify the viewport definitions in the wps as well. with re-directives like mentioned above this wont be needed.
Comment by Thomas Martitz (kugel.) - Saturday, 22 March 2008, 21:58 GMT
I don't see why you want to define the wps fonts in the config again. You just define fonts:font1,font2,font3 and the wps would directly refer to this list. There's no need for something like wpsfont: 1 or something...
Comment by Michael Hahn (disorganizer) - Sunday, 23 March 2008, 00:33 GMT
easy to explain:
if you set
font=nimbus-12,nimbus-13,nimbus-14

and in the wps you use:
%V...|2|...
%V...|3|...

then imagine you want to remove nimbus-12 completely from the config.
if you change the font line to
font=nimbus-13,nimbus-14
your wps will be broken unless you also modify all viewport entries.

with separate redirection of the wps-fontindexes line i explained, the change would just be:
font=nimbus-12,nimbus-13,nimbus-14
fontwps=2,3
to
font=nimbus-13,nimbus-14
fontwps=1,2
and no need to change the wps-files at all.
now imagine maybe we will also have wps-like configfiles for menus, tuner, .....
if you then change the fontline, you would need to go through all files to check for viewport references.
Comment by Travis Tooke (tdtooke) - Sunday, 23 March 2008, 15:58 GMT
@Thomas Martitz: yeah, I probably went overboard removing everything. How about this. If a theme has tunerfont and recorderfont, then it sets maybe userfont1 and userfont2 to those if userfonts are not present in the theme and use those userfonts for that so we might have something close to backwards compatibility with the older versions of the patch without having to load more fonts.
Comment by Travis Tooke (tdtooke) - Sunday, 23 March 2008, 16:03 GMT
On second thought maybe we could load the userfonts first, then when it goes to recorderfont, etc it checks if those fonts are already in the userfonts and if they're not then recorderfont, etc will point to the respective userfonts and only actually load a font if it's not already there. I do kinda want to stay with the grand total of 8 fonts we currently use now though.
Comment by Jacob Brooks (jac0b) - Sunday, 23 March 2008, 17:19 GMT
the current patch breaks mpegplayer
Comment by Thomas Martitz (kugel.) - Sunday, 23 March 2008, 18:25 GMT
I don't think we should focus too much on backwards compability.

I had this idea today for this patch. We should do it this way:

Define userfonts like normal (1-8). Alternatevly allready define them in a list like mentioned before (fonts: font1,font2,...), but this is a bit over my head for now.
0 is FONT_SYSFIXED.

Then we should have this:

Menufont: 1
Browserfont: 2
Radiofont: 1
Recorderfont: 3
(maybe) Pluginfont: 0

This means, that we threw "font" away, and only keep the userfonts at all. However, to not break default themes, every theme.cfg that contains "font" should default to userfont1: font and all those *font: 1 (you know what I mean).

By the way, I brought Browserfont and Menufont seperated in this idea again. After a short talk in irc, it seemed that some devs are in favor of having the possibilty to seperate between this two contexts.

I really like the concept of only having userfonts.
Comment by Michael Hahn (disorganizer) - Sunday, 23 March 2008, 20:28 GMT
yes, this sounds good.
and we should definitely not care about backward compatibility as we will pay for it with unflexibility and unclear design.

also if we are too backwards compatible, people will use the old methods forever. a clear break is better here imho.

so i think we could all agree on defining userfonts with a font=x.fnt,y.fnt,z.fnt directive.

if we use viewports for the browser and for the menu (well, in fact for all possible contexts), then we in fact wont need seperate font directives for them (except if we use my idea above to use a seperate font mapping for each context-viewport).
we would define a "browser viewport" which could use any font we define from the userfonts.
same for menu, radio, .........

we could even allow plugins to either use a default plugin viewport or alternatively have a own viewport definition file!
(so doom could use another font than pacman).

in fact the more we talk about it, the more i think having a completely viewportified rockbox would be the best approach for customizability.

@kugel:
in fact your idea resembles mine, just that we propably should add the flexibility to allow multiple fonts for each context. with viewportified contexts there may be the need to have this in the future.

and if the viewport only needs one, well we dont have to define multiple "remaps".

so in the above example, the config file would look like:
font=font1.fnt,font2.fnt,font3.fnt
menufont=1
browserfont=2
radiofont=1
recorderfont=3
pluginfont=0
wpsfont=1,2,3

which would translate to:
we load font1.fnt, font2.fnt and font3.fnt as fonts for our theme.
then we use:
menu: font1 as %V..1.. definition in the menu viewport (or as font if no viewport is present)
... for all the other *font definitions...
wps: font1 as %V..1.., font2 as %V..2.., font3 as %V..3.. (or font1 if no viewport definitions are present)
in all viewports the sysfixed can be referenced as %V..0.. and is default if no other font is specified.


Comment by Thomas Martitz (kugel.) - Sunday, 23 March 2008, 20:53 GMT
I still don't see a need to define wpsfont again. All userfonts we've defined can be used in any context. The context defines how to use the fonts. In every case the context defines that in the global config, but in the case of wps it's happening in the wps file. There's absolutely no need to define the wps fonts again, this will decrease the flexibility.

And sure, the user must be aware, that changing the user fonts hits all contexts, so the user must edit the .wps file too in some cases. But that's still better to define wps fonts again.

BTW: I hope we can get it this way:

We defined the user fonts, and after that we define the viewports for the specific contexts (except for the wps of course). Or we create other context specific files like .wps (i.e. .rps (radio-wps) or something).

radioviewport: x|y|height|width|font|foreground|background.

Of course this includes "viewportifing" all contexts. But that's the wish of everyone I think. That's what viewports were made for.
Comment by Michael Hahn (disorganizer) - Sunday, 23 March 2008, 22:34 GMT
but exactly thats the problem:
if we directly use userfont-indexing in the wps-like context files, a user wanting to remove a font manually needs to edit several files to do that.
lets say we have the wps, radiowps, browser, menu and statusline files.
then removing one font would make the user edit all viewport definitions in those files.
(they all could hold several viewport definitions).
if we use reindexing, he could change all that in just 5 lines in the central config file of rockbox.
Comment by Michael Hahn (disorganizer) - Sunday, 23 March 2008, 22:36 GMT
an alternative as compromise would be allowing "empty" font reference, which would load the representing font index with sysfixed.
example:
font=font1,,font3
%V..0..=sysfixed
%V..1..=font1
%V..2..=sysfixed
%V..3..=font3

that way a user can remove a font without having to edit other files.
Comment by Michael Hahn (disorganizer) - Sunday, 23 March 2008, 23:07 GMT
just forgot an advantage of using the font-indexing method:
we will be compatible with all themes not using the multifont patch.
if we use
font: font1.fnt,font2.fnt,font3.fnt
this would be compatible with the normal
font: font1.fnt
method of setting a font.
(do we need to set the full patch there?)

this brings me to another point:
if a viewport uses a font not set, what should we do?
i think we should revert to sysfixed only if NO font is said. if any font is set, we should use font1.
this would also maintain compatiblility with non-mf themes.
Comment by Travis Tooke (tdtooke) - Monday, 24 March 2008, 00:37 GMT
A user trying to use a font not set is the main reason I was against using the actual font names for the viewports in a wps. I do kinda like the idea of doing something like Thomas Martitz mentioned: radioviewport: x|y|height|width|font|foreground|background. For the various views? as they're viewportified. The er.. viewportification. From a coding perspective it'd probably be easiest to do it that way.
Comment by Travis Tooke (tdtooke) - Monday, 24 March 2008, 00:42 GMT
Oh yeah, as far as mpegplayer crashing, basically in a plugin if the old patch did anything more than rb->lcd_setfont then it's probably trying something with not enough parameters. Sorry about that, should have been more careful. I'll try to get to that today if possible.
Comment by Travis Tooke (tdtooke) - Tuesday, 25 March 2008, 02:51 GMT
Hmm.. I just used mpegplayer on my iPod 5.5g and it worked fine. What exactly is happening with yours jac0b? Not compiling, is compiling and player crashes?
Comment by Travis Tooke (tdtooke) - Tuesday, 25 March 2008, 02:57 GMT
I know what it is, replace current_vp->font with FONT_UI in mpegplayer.c. I must've accidentally synced from a much older patch because I do seem to remember that being an issue before.
Comment by Travis Tooke (tdtooke) - Tuesday, 25 March 2008, 03:02 GMT
Nevermind, I'm looking at old patches again.
Comment by Jacob Brooks (jac0b) - Tuesday, 25 March 2008, 11:49 GMT
It started around 03-13 revision, but I corrected the problem on 03-15 by changing FONT_UI to FONT_SYSFIXED in mpegplayer.c, I tried that on the latest revision but it still crashes the mpegplayer.
Comment by Jacob Brooks (jac0b) - Wednesday, 02 April 2008, 12:07 GMT
Did anyone fix the mpegplayer issue?
Comment by Thomas Martitz (kugel.) - Wednesday, 02 April 2008, 12:22 GMT
What is the issue? I was experiencing mpeg player crashes as soon a button is pressed while video watching in a build with this patch (and many more patches). Do you mean that? Have you really tracked it down to the multifont patch? I'm not sure myself-
Comment by Jacob Brooks (jac0b) - Wednesday, 02 April 2008, 20:45 GMT
I believe so, I compiled a build using r16841 & multifont-20080322a just to make sure and yes it does break mpegplayer.
Comment by Michael Hahn (disorganizer) - Thursday, 03 April 2008, 06:45 GMT
as the patch in its current form wont get commited ever, it would propably need major rework anyways.
its a pitty nobody has come up with a efficient caching method yet.
this topic definitely needs a bit more attention.
Comment by Michael Hahn (disorganizer) - Thursday, 03 April 2008, 06:50 GMT
just for the record:
the problem why this wont get commited in its current state is the inefficient font glyph caching.
at the moment every font gets its own glyphcache with a fixed length. thus some fonts may have more cache-memory than then need and others will run out of cachespace soon.
so this method is considered ineffective by the devs.

the first thing to get this nearer to commitability would be to find a good and efficient caching algorythm for multiple fonts without wasting cpu, memory and binsize.
Comment by Thomas Martitz (kugel.) - Thursday, 03 April 2008, 20:41 GMT
I guess tdooke clean up a bit too much, this one should work.

The second patch is meant to be applied with my customlist patch at  FS#8799 .
Comment by Hendrik (Eraser212) - Monday, 07 April 2008, 13:49 GMT
Hello,
I recently updated my rockbox build and multifont after a long time.
Unfortunately, I don't quite get what has changed syntax wise in the meantime.
There are so many proposals here,
I don't understand which method is currently used.

The old font declarations don't work anymore, perhaps someone can help me to make it
work again.

Thanks in advance.

At the moment, my font declaration looks like:

menufont: /.rockbox/fonts/nimbus-12.fnt
browserfont: /.rockbox/fonts/nimbus-12.fnt
recordont: /.rockbox/fonts/nimbus-12.fnt
tunerfont: /.rockbox/fonts/nimbus-12.fnt
wpsfont: /.rockbox/fonts/nedore-9.fnt




Comment by David Maliniak (major_works) - Monday, 07 April 2008, 20:18 GMT
I'd like to second Hendrik's comment above. It'd be really helpful if some minimal documentation could be posted here that would explain how to use the multifont capability effectively.
Comment by Thomas Martitz (kugel.) - Monday, 07 April 2008, 20:24 GMT
It would be helpful of a dude with edit permissions would clean this up and make a new proper first post, alternativly we make a new task, but I'd like to avoid that.
Comment by Michael Hahn (disorganizer) - Tuesday, 08 April 2008, 12:36 GMT
again, i must say that we still dont know how fonts will be handled in the future. until now this patch has no chance to get commited.
so why bother with usage guidelines when we know that it will never be implemented in the way it is implemented now?

we need to keep in mind that this is a highly unsupported patch and that a new implementation needs to work in a completely different way.
thus this patch definitely is a dead-end.
Comment by Michael Hahn (disorganizer) - Tuesday, 08 April 2008, 12:38 GMT
@kugel:
maybe to get a clean cut this could get closed and a new task should be opened which explicitly handles a NEW multifont implementation. every effort in this old patch should imho better be put into finding a new glyphcache algorithm.
Comment by Thomas Martitz (kugel.) - Thursday, 15 May 2008, 00:30 GMT
Sync.

Activity seems to be slowed down here...
Comment by Hendrik (Eraser212) - Sunday, 18 May 2008, 16:01 GMT
@kugel

Sorry, that I ask this again,
but could you please post a short example of how to use
multiple fonts with the current patch?

Is it still possible to declare fonts in whileplayingscreens which
don't use viewports?

I want to use two different fonts, one font for the menu and one font
for the wps.

thanks, Hendrik
Comment by Thomas Martitz (kugel.) - Sunday, 18 May 2008, 16:26 GMT
Ok, here you go.

At first, multifont doesn't support this:

menufont: /.rockbox/fonts/nimbus-12.fnt
browserfont: /.rockbox/fonts/nimbus-12.fnt
recordont: /.rockbox/fonts/nimbus-12.fnt
tunerfont: /.rockbox/fonts/nimbus-12.fnt
wpsfont: /.rockbox/fonts/nedore-9.fnt

anymore (themes using this will be broken).

Multiple fonts are set up like this:

userfontn: /path/to/font/font.fnt (with n being 1 upto 8, and /path/to/rockbox = /.rockbox/fonts in most cases)
example:
userfont1: /.rockbox/fonts/nedore-9.fnt
...
userfont8: /.rockbox/fonts/nedore-9.fnt

As of now, you can only use them in the wps, since it is the only screen which supports configurable viewports (except with my custom list patch( FS#8799 ) applied, then the menues/filetree can use a userfont as well).

And yes, this means the wps needs viewport. But, that's not a drawback. Viewports is in SVN, and defining a fullscreen which uses a userfont is easy, e.g.

%V|0|0|176|220|2|FFFFFF|000000| # this example is for e200 and uses userfont1.

As you could see, 2 is userfont1. So, userfont2 is 3, and so on. That's because the default font is 1.
Comment by Hendrik (Eraser212) - Sunday, 18 May 2008, 16:43 GMT
Thank you, this definitely cleared things up for me, it works perfectly :)
Comment by Tri Nguyen (tri170391) - Monday, 19 May 2008, 04:03 GMT
Is there any way to use this patch alongside with the Anti-alisased Font patch?
Comment by Thomas Martitz (kugel.) - Monday, 19 May 2008, 12:10 GMT
Yes, try this one.
Comment by Bob (viperman3) - Monday, 09 June 2008, 21:15 GMT
seems to be broken in build r17704 ... resync please.
Comment by Jacob Brooks (jac0b) - Thursday, 12 June 2008, 00:27 GMT
Resync
Comment by Jacob Brooks (jac0b) - Thursday, 12 June 2008, 00:59 GMT
Disregard the above patch
Comment by Jacob Brooks (jac0b) - Thursday, 12 June 2008, 01:03 GMT
Sorry for the double post but try this one.
Comment by Shiloh Hawley (gree665) - Thursday, 12 June 2008, 18:17 GMT
Just a suggestion/request: how about an additional separate font for the custom keyboard screen?
Comment by Bob (viperman3) - Monday, 16 June 2008, 15:44 GMT
I must be doing something wrong, but the sim I compiled for the iaudio X5 and the iriver H320 both aren't working with the multifont-20080611 patch (using build r17704). The build compiles without prolems, but the wps that once worked is now not working. It's ignoring the userfonts I have specified and only using the font defined by the "font:" definition of the cfg file. I have a build based on r17428 with multifont-20080403 and the same wps works fine.

I have looked for changes in the theme .cfg file but I can't see anything different from what kugal has outlined a few posts above. As I can tell, the wps and cfg files are okay. Anoyone else getting problems?
Comment by Jacob Brooks (jac0b) - Monday, 16 June 2008, 20:47 GMT
You probably didn't do anything wrong my resync might be messed up.

Try this

Comment by Bob (viperman3) - Monday, 16 June 2008, 21:03 GMT
Thank you, Jacob.

The patch works now. I have tested it very briefly but it does seem to work. Thanks again
Comment by Jacob Brooks (jac0b) - Tuesday, 01 July 2008, 00:58 GMT
Resync
Comment by Jason Voegele (jvoegele) - Thursday, 28 August 2008, 14:56 GMT
The latest version of this patch (multifont-20080630.patch) does not compile cleanly with latest Subversion HEAD as of today. Here is the compile error:

CC filetree.c
filetree.c: In function ‘ft_enter’:
filetree.c:510: error: too few arguments to function ‘font_load’
make[1]: *** [/home/jvoegele/src/rockbox/build/apps/filetree.o] Error 1
make: *** [build] Error 2
Comment by Thomas Martitz (kugel.) - Thursday, 28 August 2008, 15:21 GMT
Well, this patch adds 1 argument to font_load(). My guess is,. that the HUNK for filetree.c failed at applying the patch, can you please take a look at this?
Comment by Thomas Martitz (kugel.) - Thursday, 28 August 2008, 15:29 GMT
After having a quick look, replacing "gui_syncsplash(0, ID2P(LANG_WAIT));" with "splash(0, ID2P(LANG_WAIT));" in the filetree.c hunk (line 374 in the patch file) should do the job.
Comment by Jason Voegele (jvoegele) - Thursday, 28 August 2008, 17:24 GMT
Change line 374 of the patch as you instructed did the trick. After updating the patch file I was able to reapply and compile.

Thanks.
Comment by Thomas Martitz (kugel.) - Monday, 29 September 2008, 00:11 GMT
Sync. Not heavily tested though.

Also I'm not sure how to deal with the WPSEDITOR.

Loading...