• Status Closed
  • Percent Complete
  • Task Type Patches
  • Category Configuration
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by kugel. - 2010-02-27
Last edited by kugel. - 2011-12-22

FS#11064 - Resizable skin buffer

This patch modifies skin loading and buffering so that it enables resizing of the skin buffer.

Skin parsing happens in 2 passes now.
- the first pass parses everything but fonts and images (i.e. everything that fits into a sane backup buffer)
- the second pass loads the bitmaps and fonts (the filenames are saved, so the second pass doesn’t mean reparsing)

Upon parsing, the first pass reports the size needed. if it exceeds the existing buffer it asks for a reboot. in any event, it will drop a .skin_engine_%d file where %d has the size needed. Upon reboot, the skin buffer will use this for its size. A 10k backup buffer is always added.

This is all complicated because of the skin buffer architecture (bidirectional) but seems to work.

2 issues:
- backdrop handling is a bit stupid right now, it will assume loading one even if the skin reuses an already buffered one. that’s not a big issue as it will be right after the next reboot (the parsing at boot dumps the real size, not the one reported by the first pass).
- everything is pretty messed up if the buffer wasn’t enough, like corrupted images etc. this needs better handling.

Closed by  kugel.
2011-12-22 15:49
Reason for closing:  Out of Date
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

The skin buffer is dynamic via buflib now.

Close opened files and dirs. Backup buffer is 20k in this one.

from a quick look, save the required size in the filename is absolutly the wrong way to do it. use a setting in the cfg file which is loaded before the themes are init anyway. and then its easy to know how much a random theme from the site
quit your belly aching about the buffer mechanism, it works quite well and complaining just annoys people. (and if you really care that much then change the tokens array to a linked list, use offsets instead of pointers everywhere, and make the bmp loader smarter so it knows how much it reauires t load a bmp)

Use nvram.bin/global_status instead of an extra file

needs a sync (easy
do you add 20K to the known size? that seems way too high, it should be screen depth dependant, 2K is probblay enough for non colour.

theme loading is noticably slower (even with a CF disk), but its worth it :) looking good

a few more comments because this should get commited
the commented out code and extra unneeded
reboot splash isnt good enough, if it fails it really should reload the inbuilt skins (otherwise rockbox will crash on entering the WPS), and the timeout on that splash should be more than 1
should be named something else, and the comments attached to it are
the SKIN_BUFFER_SIZE should be increased by at least 1xSKIN_FONT_SIZE, probably more so there is a very high chance that any theme loaded without nvram.bin will load first go (I tihnk though for this reason that the theme size should be in the .cfg and not nvram.bin

I wonder why it’s slower. But it is only slower when loading at run time, right? I haven’t noticed boot time slowdowns. It is possible that the extra call image loader with FORMAT_RETURN_SIZE isn’t as fast as I thought (I thought it was fast because it doesn’t load the image, but only inspects the header).

The function name was correct when I used the extra file, but it can be renamed of course.

The 20k buffer is to ensure that at least all tokens fit into it (i.e. ensure that the first pass succeeds). Imagine someone goes from a simple text only wps (only few kB big) to a more complex one. In this scenario the first pass could fail without a bigger backup buffer. And if the first pass fails there’s no way of knowing the size for the second pass. It doesn’t need to be display depth dependent since it’s not for images.

I’m disagreeing with the size in the .cfg idea. people shouldn’t be able to mess with it.

yes the slow down is only during run, because like I said it opens every file twice

handle themes that are too big on boot better. still no resetting to built in when the buffer is to small.

Works great, but seems it need to be update again.


Available keyboard shortcuts


Task Details

Task Editing