Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11567 - Smarter memory allocation for skin fonts

Attached to Project: Rockbox
Opened by Fred Bauer (freddyb) - Tuesday, 24 August 2010, 00:25 GMT+2
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 25 August 2010, 16:12 GMT+2
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

These patches improve the allocation of font memory in the skin engine. Allocating memory in proportion to the glyph size is more efficient and if done right can reduce the disk reads while loading skin_fonts. There are two variations:

"Skin_fonts_extra_tag" adds a field to the load user font tag that explicitly specifies how many glyphs to allocate memory for. This allows paring down fonts that are just used for number display and allows forcing more allocation for international users who use more glyphs.
%Fl(3,16-GNU-Unifont.fnt) would become %Fl(3,16-GNU-Unifont.fnt,100). Supplying a dash allocates 256.

"Skin_fonts_smart_alloc" is a less ambitious patch that allocates space for 256 glyphs without changing the tag specification.

   skin_fonts_extra_tag.diff (5.4 KiB) 
 apps/gui/skin_engine/skin_parser.c |    6 +++++-
 apps/gui/skin_engine/skin_fonts.h  |    6 ++++--
 apps/gui/skin_engine/skin_fonts.c  |   14 +++++++++++---
 lib/skin_parser/tag_table.c        |    2 +-
 firmware/export/font.h             |    1 +
 firmware/font.c                    |   29 +++++++++++++++++++++++++++++
 6 files changed, 51 insertions(+), 7 deletions(-)

   skin_fonts_smart_alloc.diff (2.8 KiB) 
 apps/gui/skin_engine/skin_fonts.h |    1 +
 apps/gui/skin_engine/skin_fonts.c |   10 ++++++++--
 firmware/export/font.h            |    1 +
 firmware/font.c                   |   29 +++++++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 2 deletions(-)

This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Wednesday, 25 August 2010, 16:12 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  in r27882, thanks
Comment by Jonathan Gordon (jdgordon) - Tuesday, 24 August 2010, 01:14 GMT+2
you should make the 3rd param optional and check it was given in skin_parser.c (and it not use a sensible default otherwise). do that by putting a | before the i in the tag params list and verify the params_count
Comment by Fred Bauer (freddyb) - Tuesday, 24 August 2010, 01:54 GMT+2
Thanks, I didn't know that was possible.
   skin_fonts_extra_tag_v2.diff (5.6 KiB) 
 apps/gui/skin_engine/skin_parser.c |   10 +++++++++-
 apps/gui/skin_engine/skin_fonts.h  |    6 ++++--
 apps/gui/skin_engine/skin_fonts.c  |   14 +++++++++++---
 lib/skin_parser/tag_table.c        |    2 +-
 firmware/export/font.h             |    1 +
 firmware/font.c                    |   29 +++++++++++++++++++++++++++++
 6 files changed, 55 insertions(+), 7 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Tuesday, 24 August 2010, 02:44 GMT+2
much better :) I would suggest "if(element->params_count > 2)" instead of "if(element->params_count == 3)" though so it wont break if the tag gets an extra param later (easily overlooked), and of course the manual could do with an update (manual/appendix/wps_tags.tex iirc).

I'll commit this tonight if I remember and noone beats me to it
Comment by Fred Bauer (freddyb) - Tuesday, 24 August 2010, 03:20 GMT+2
Feel free to reword my description if you don't think it's right.
   skin_fonts_extra_tag_v3.diff (6.5 KiB) 
 apps/gui/skin_engine/skin_parser.c |   10 +++++++++-
 apps/gui/skin_engine/skin_fonts.h  |    6 ++++--
 apps/gui/skin_engine/skin_fonts.c  |   14 +++++++++++---
 lib/skin_parser/tag_table.c        |    2 +-
 firmware/export/font.h             |    1 +
 firmware/font.c                    |   29 +++++++++++++++++++++++++++++
 manual/advanced_topics/main.tex    |    6 ++++--
 7 files changed, 59 insertions(+), 9 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Tuesday, 24 August 2010, 14:08 GMT+2
patch looks good but svn is refuing to work for me so I cant commit it :/

is SKIN_FONT_SIZE still used anywhere? if not please remove it with tthis patch :)
Comment by Fred Bauer (freddyb) - Tuesday, 24 August 2010, 14:54 GMT+2
It's used in the the calculation of SKIN_BUFFER_SIZE in skin_engine.h.

Loading...