Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10984 - multifont

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Tuesday, 09 February 2010, 08:43 GMT+2
Last edited by Jonathan Gordon (jdgordon) - Sunday, 14 February 2010, 07:39 GMT+2
Task Type Patches
Category Font/charset
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch aims to add multifont to the core in a workable solution.

this is the plan:
1) remove all the local variables (apart from the absolute necessary) from font.c so it can be more usable
2) change font.c to handle more than one ui font, only a single ui font structure will be statically allocated, any others need to pass in their own font struct and buffer
3) fix skins to be able to request a font be loaded and usable in viewports.
3.1) make the apps/ level font API be simple so plugins can use it also.

NOTE: there is no plan at all to allow anything except skins in the core to use more than 1 ui font.

this first version does part 1. 2
   multifont.diff (16 KiB) 
 tools/convbdf.c               |    4 
 firmware/export/font.h        |   15 +++
 firmware/font.c               |  209 ++++++++++++++++++++----------------------
 firmware/include/font_cache.h |    5 -
 4 files changed, 126 insertions(+), 107 deletions(-)

This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Sunday, 14 February 2010, 07:39 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  r 24644.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 09 February 2010, 09:42 GMT+2
quick update before bed:
- fix the .glyphcache files so each font name gets its own (this does no collision handling, so if you try to load a font twice it will break... DONT do that :p
- remove all the struct font* pf = &font_ui; lines so it is safe to pass a pf around.
- changed font_load() to return the font number instead of the font struct (seen as the return value is never actually used this makes sense)

I need to fix up the MAXFONTS and MAXUSERFONTS names, and work out where we want to make sure we dont load the same font twice. I oribionally planned on having a reference counter (so skins wont try unloading a font if another theme still uses it) in the apps level API which would make the most sense, except the problem with fontcache collisions, if that is actually a problem? (I tihnk it is, but I'm half asleep)
   multifont.diff (21.5 KiB) 
 tools/convbdf.c               |    4 
 apps/settings.c               |    6 
 apps/filetree.c               |    2 
 firmware/export/font.h        |   24 ++-
 firmware/font.c               |  314 +++++++++++++++++++++++-------------------
 firmware/include/font_cache.h |    5 
 6 files changed, 210 insertions(+), 145 deletions(-)

Comment by Alexander Levin (fml2) - Tuesday, 09 February 2010, 14:14 GMT+2
There is a typo in font.c:font_reset(). There you should have "pf->cache_fd = -1;" instead of "pf->cache_fd =1;"

This is just one thing I've spotted. I haven't looked very deeply into it :-P
Comment by Jonathan Gordon (jdgordon) - Tuesday, 09 February 2010, 18:23 GMT+2
crud, thanks.
Comment by Alexander Levin (fml2) - Tuesday, 09 February 2010, 22:16 GMT+2
I'm not crud
Comment by Jonathan Gordon (jdgordon) - Tuesday, 09 February 2010, 22:20 GMT+2
didnt you know crud is the highest form of flattery from an Aussie?

anywho, lets get back to the patch discussion.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 10 February 2010, 09:35 GMT+2
first working version
   test.wps (0.2 KiB)
   multifont.diff (30.6 KiB) 
 tools/convbdf.c                    |    4 
 apps/settings.c                    |    8 
 apps/gui/skin_engine/skin_parser.c |   37 ++++
 apps/gui/skin_engine/skin_fonts.h  |   41 ++++
 apps/gui/skin_engine/skin_fonts.c  |  116 ++++++++++++
 apps/gui/viewport.c                |    5 
 apps/filetree.c                    |    2 
 apps/SOURCES                       |    1 
 firmware/export/font.h             |   25 ++
 firmware/font.c                    |  337 +++++++++++++++++++++----------------
 firmware/include/font_cache.h      |    5 
 11 files changed, 427 insertions(+), 154 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Thursday, 11 February 2010, 06:11 GMT+2
this version adds remote ui font support (rather crudely, but acceptable) that fonts buffer is a seperate 10K block which is only used if a font is actually chosen (and i need to fix this so it needs to be different to the main ui font also)

I have no idea how to do the setting for it though because the regular font setting is changed by just loading a .fnt. maybe a context menu item on .fnts?

if you want to test this add a "remote font: /.rockbox/fonts/my-font.fnt" line to your config
   multifont.diff (34.6 KiB) 
 tools/convbdf.c                    |    4 
 apps/settings.c                    |   19 +
 apps/gui/statusbar-skinned.c       |    4 
 apps/gui/skin_engine/skin_parser.c |   37 +++
 apps/gui/skin_engine/skin_fonts.h  |   41 +++
 apps/gui/skin_engine/skin_fonts.c  |  116 ++++++++++
 apps/gui/viewport.c                |    9 
 apps/settings.h                    |    4 
 apps/settings_list.c               |    4 
 apps/filetree.c                    |    2 
 apps/SOURCES                       |    1 
 firmware/export/font.h             |   32 ++-
 firmware/font.c                    |  392 +++++++++++++++++++++++--------------
 firmware/include/font_cache.h      |    5 
 14 files changed, 509 insertions(+), 161 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Thursday, 11 February 2010, 07:47 GMT+2
this one actually works, and cleans up the MAXFONTS #define

just need to figure out what to do about the glyphcache file(s) and plugins.
   multifont.diff (38.3 KiB) 
 tools/convbdf.c                    |    4 
 apps/plugins/rockpaint.c           |   10 
 apps/settings.c                    |   20 +
 apps/gui/statusbar-skinned.c       |    4 
 apps/gui/skin_engine/skin_parser.c |   40 +++
 apps/gui/skin_engine/skin_fonts.h  |   43 ++++
 apps/gui/skin_engine/skin_fonts.c  |  117 +++++++++++
 apps/gui/viewport.c                |   16 -
 apps/settings.h                    |    4 
 apps/settings_list.c               |    4 
 apps/filetree.c                    |    2 
 apps/SOURCES                       |    1 
 apps/plugin.h                      |    2 
 firmware/export/font.h             |   36 +++
 firmware/font.c                    |  392 +++++++++++++++++++++++--------------
 firmware/include/font_cache.h      |    5 
 firmware/powermgmt.c               |    2 
 17 files changed, 531 insertions(+), 171 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Friday, 12 February 2010, 05:36 GMT+2
new menu to choose which screen to load a .fnt onto

also change back to a sensible cacheing mechanism. only the FONT_UI font glyphs are cached. all fonts when loaded read that glyphcache file and it stands to reason that most of the same glyphs will be used in skin fonts as the main font, and the main and remote font will always show the same.

edit: I'd be happy to see this version commited. the only thing missing is plugin support which could wait, or even be ignored imo.

   multifont.diff (39.4 KiB) 
 tools/convbdf.c                    |    4 
 apps/lang/english.lang             |   34 +++
 apps/plugins/rockpaint.c           |   10 -
 apps/settings.c                    |   20 +-
 apps/gui/statusbar-skinned.c       |    4 
 apps/gui/skin_engine/skin_parser.c |   40 +++-
 apps/gui/skin_engine/skin_fonts.h  |   43 ++++
 apps/gui/skin_engine/skin_fonts.c  |  126 ++++++++++++
 apps/gui/viewport.c                |   16 +
 apps/settings.h                    |    4 
 apps/settings_list.c               |    4 
 apps/filetree.c                    |   30 ++-
 apps/SOURCES                       |    1 
 apps/plugin.h                      |    2 
 firmware/export/font.h             |   35 +++
 firmware/font.c                    |  369 ++++++++++++++++++++++---------------
 firmware/include/font_cache.h      |    5 
 firmware/powermgmt.c               |    2 
 18 files changed, 576 insertions(+), 173 deletions(-)

Comment by Alexander Levin (fml2) - Saturday, 13 February 2010, 22:36 GMT+2
Some notes and questions:

1. The plugin API version(s) should be bumped

2. What is the "buffer" field in the struct skin_font for?

3. I see the buffer allocation for a font (from the skin) but no release. Is it OK?

4. What is "first_init" in skin_font_init for?

5. The skin functions need at least a brief comment

6. I'd rename the variable "mbuf" to "main_buf" or similar in font.c, to be more in line with "remote_buf"
Comment by Jonathan Gordon (jdgordon) - Sunday, 14 February 2010, 03:44 GMT+2
1) yep.
2) that is to allow things to replace the loaded font without takeing more buffer. mostly goign to be used by the plugins once the api for them is done (I'm not too worried about not doing that in the first version though. it can come after)
3) you cant release the buffers from the skin buffer block, this goes with point 2 a bit. When a new theme is loaded all the fonts are blown away anyway so the ram is freed then.
4) should be remoavable now. it was there to unload fonts and save their cache files, but seen as only the main lcd's ui font is cached this is not needed.
5) yeah yeah :)
6) ok

Loading...