Rockbox

Tasklist

FS#11567 - Smarter memory allocation for skin fonts

Attached to Project: Rockbox
Opened by Fred Bauer (freddyb) - Monday, 23 August 2010, 22:25 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 25 August 2010, 14:12 GMT
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
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.

This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Wednesday, 25 August 2010, 14:12 GMT
Reason for closing:  Accepted
Additional comments about closing:  in r27882, thanks
Comment by Jonathan Gordon (jdgordon) - Monday, 23 August 2010, 23:14 GMT
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) - Monday, 23 August 2010, 23:54 GMT
Thanks, I didn't know that was possible.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 24 August 2010, 00:44 GMT
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, 01:20 GMT
Feel free to reword my description if you don't think it's right.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 24 August 2010, 12:08 GMT
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, 12:54 GMT
It's used in the the calculation of SKIN_BUFFER_SIZE in skin_engine.h.

Loading...