Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12293 - Global default glyph size setting for gui/skins

Attached to Project: Rockbox
Opened by Fred Bauer (freddyb) - Sunday, 25 September 2011, 20:32 GMT+2
Last edited by Fred Bauer (freddyb) - Sunday, 20 November 2011, 00:40 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.9
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

We should allocate font memory according to the number of glyphs needed because the font coverage can vary quite a bit between a small mono font and a large anti-aliased font for a given buffer size. It's easy to do this now that font memory is reserved via buflib. This patch adds a global setting for the default number of glyphs for fonts rather than a fixed buffer size for each font. The setting is under Theme Settings>Glyphs to cache, which sets global_settings.glyphs (default 250).

Adds the function font_load_glyphs( char *path, int glyphs );
Adds a #define font_load_whole( char *path ); that tries to load the entire font into memory.
font_load_glyphs now checks if the specified number of glyphs would make the font_cache structure larger than the font file and if so, just loads the whole font file in memory instead.
   font_load_glyphs.patch (7.6 KiB) 
 apps/lang/english.lang                      |   14 ++++++++++
 apps/settings.c                             |    2 -
 apps/gui/skin_engine/skin_parser.c          |   14 +++-------
 apps/settings.h                             |    1 
 apps/menus/theme_menu.c                     |    3 +-
 apps/settings_list.c                        |    7 ++++-
 firmware/export/font.h                      |    2 +
 firmware/font.c                             |   37 ++++++++++++++++------------
 manual/configure_rockbox/theme_settings.tex |    2 +
 9 files changed, 54 insertions(+), 28 deletions(-)

This task depends upon

Closed by  Fred Bauer (freddyb)
Sunday, 20 November 2011, 00:40 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  In r31031.
Comment by Jonathan Gordon (jdgordon) - Monday, 26 September 2011, 00:54 GMT+2
No, this is very much overkill, most times we load a font we want to be able to cache as much of it as possible, but we have to make sure not to waste too much ram doing so. So if you want to add a setting to speficy the maximum actual buffer size that would be fine (and not require an extra few disk accesses on load), but just adding a setting to say how many glpyhs to load is not a good idea.
Comment by Fred Bauer (freddyb) - Monday, 26 September 2011, 01:54 GMT+2
I disagree. The only way to allocate font memory efficiently is in terms of glyphs. Anything else is just a number out of the air.
Comment by Fred Bauer (freddyb) - Monday, 26 September 2011, 17:37 GMT+2
This is an example of why font memory should use a smarter allocation system. The following shows how the current behavior is completely inconsistent with needs. Glyph coverage varies by orders of magnitude. The default theme CabbieV2 allocates 1250 glyphs, the iLike theme allocates 8818, while BigInfoAA only allocates 82 for its big font. The argument against reading the font header before loading the font doesn't make sense to me because the extra read should very inexpensive. I simply do not understand the logic that this is overkill after all the work done to create buflib and move fonts over to it. If the glyphcache file bug gets fixed this would also reduce the chances of someone who uses many glyphs wiping out most of their glyphcache info by loading a large point font.

15-Adobe-Helvetica.fnt loaded (cache). file size 231946 - 60000 bytes - 1250 glyphs
20-Ubuntu-R.fnt loaded (cache). file size 431297 - 60000 bytes - 280 glyphs
38-Ubuntu-R.fnt loaded (cache). file size 656925 - 60000 bytes - 82 glyphs
15-Adobe-Helvetica.fnt loaded (cache). file size 231946 - 60000 bytes - 1250 glyphs
24 iLike.fnt loaded (mem). file_size 48510 - 8818 glyphs
18 iLike.fnt loaded (mem). file_size 48510 - 8818 glyphs
18 iLike.fnt loaded (mem). file_size 48510 - 8818 glyphs
TimesNewRomanPSMT-16.fnt loaded (cache). file size 401709 - 60000 bytes - 256 glyphs
Georgia-19.fnt loaded (cache). file size 253293 - 60000 bytes - 272 glyphs
Army-23.fnt loaded (cache). file size 225679 - 60000 bytes - 180 glyphs
Army-15.fnt loaded (mem). file_size 6305 - 95 glyphs
Army-18.fnt loaded (cache). file size 212355 - 60000 bytes - 300 glyphs
15-Adobe-Helvetica.fnt loaded (cache). file size 231946 - 60000 bytes - 1250 glyphs
DejaVuSans_10.fnt loaded (mem). file_size 57759 - 11351 glyphs
DejaVuSans_15.fnt loaded (cache). file size 79401 - 60000 bytes - 410 glyphs
DejaVuSans_20.fnt loaded (cache). file size 130751 - 60000 bytes - 250 glyphs
FreeSerif-10.fnt loaded (cache). file size 464502 - 60000 bytes - 517 glyphs
FreeSerif-20.fnt loaded (cache). file size 851638 - 60000 bytes - 142 glyphs
FreeSerif-30.fnt loaded (cache). file size 1489538 - 60000 bytes - 64 glyphs
Comment by Fred Bauer (freddyb) - Saturday, 01 October 2011, 21:36 GMT+2
This change adds a #define LIMIT_GLYPH_LOADS for targets with less than 4MB which limits font buffers to MAX_FONT_SIZE even if more glyphs are requested. Hopefully, this addresses the objections.
   font_load_glyphs.2.patch (7.7 KiB) 
 apps/lang/english.lang                      |   14 +++++++++
 apps/settings.c                             |    2 -
 apps/gui/skin_engine/skin_parser.c          |   14 ++-------
 apps/settings.h                             |    1 
 apps/menus/theme_menu.c                     |    3 +
 apps/settings_list.c                        |    7 +++-
 firmware/export/font.h                      |    2 +
 firmware/font.c                             |   43 ++++++++++++++++++++--------
 manual/configure_rockbox/theme_settings.tex |    2 +
 9 files changed, 64 insertions(+), 24 deletions(-)

Comment by Fred Bauer (freddyb) - Friday, 11 November 2011, 20:05 GMT+2
I'm reopening this to try again. This version includes some refactoring of font loading. The functional changes are the following:

-Creates a theme menu entry for the default number of glyphs to load
-Font_load_ex() is now font_load_ex( name, buffer size, glyphs ).
-Buffer size and glyphs may be zero for defaults.
-Default font_load() allocates the lesser of 256 glyphs or MAX_FONT_SIZE, which can be a significant memory savings (~46000 bytes for 15-Adobe-Helvetica)
-Font buffer will be limited to MAX_FONT_SIZE for targets with less than 4MB, overriding the glyph parameter
-Font buffer can exceed MAX_FONT_SIZE if glyphs are specified and buffer size is not and target mem >= 4MB.
-Font buffer will be limited to buffer size if both parameters are given.
-(The original patch limited the size of the font buffer to the size of the font file but that change was committed to SVN separately.)
-Skin fonts will use the global glyph parameter if not specified in the skin.
   font_load_glyphs.3.patch (28.5 KiB) 
 tools/convbdf.c                             |    1 
 apps/settings.c                             |    2 
 apps/lang/english.lang                      |   14 
 apps/gui/skin_engine/skin_parser.c          |   10 
 apps/settings.h                             |    1 
 apps/menus/theme_menu.c                     |    2 
 apps/settings_list.c                        |    6 
 firmware/export/font.h                      |    6 
 firmware/font.c                             |  517 ++++++++++++----------------
 manual/configure_rockbox/theme_settings.tex |    3 
 10 files changed, 276 insertions(+), 286 deletions(-)

Comment by Fred Bauer (freddyb) - Sunday, 20 November 2011, 00:28 GMT+2
Updated version.
   font_load_glyphs.6.patch (24.6 KiB) 
 tools/convbdf.c                             |    1 
 apps/lang/english.lang                      |   14 
 apps/settings.c                             |    2 
 apps/gui/skin_engine/skin_parser.c          |    6 
 apps/settings.h                             |    1 
 apps/menus/settings_menu.c                  |    9 
 apps/settings_list.c                        |    6 
 firmware/export/font.h                      |    4 
 firmware/font.c                             |  455 ++++++++++++----------------
 manual/advanced_topics/main.tex             |    3 
 manual/configure_rockbox/system_options.tex |   10 
 11 files changed, 247 insertions(+), 264 deletions(-)

Loading...