Notice: A non well formed numeric value encountered in /sites/ on line 96 Notice: A non well formed numeric value encountered in /sites/ on line 96 Notice: A non well formed numeric value encountered in /sites/ on line 96 Deprecated: Function create_function() is deprecated in /sites/ on line 104 Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /sites/ on line 845 Deprecated: Function create_function() is deprecated in /sites/ on line 111 FS#11064 : Resizable skin buffer



FS#11064 - Resizable skin buffer

Attached to Project: Rockbox
Opened by Thomas Martitz (kugel.) - Saturday, 27 February 2010, 18:16 GMT
Last edited by Thomas Martitz (kugel.) - Thursday, 22 December 2011, 15:49 GMT
Task Type Patches
Category Configuration
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


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.
This task depends upon

Closed by  Thomas Martitz (kugel.)
Thursday, 22 December 2011, 15:49 GMT
Reason for closing:  Out of Date
Additional comments about closing:  The skin buffer is dynamic via buflib now.
Comment by Thomas Martitz (kugel.) - Saturday, 27 February 2010, 19:35 GMT
Close opened files and dirs. Backup buffer is 20k in this one.
Comment by Jonathan Gordon (jdgordon) - Sunday, 28 February 2010, 04:52 GMT
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 requires.
Also 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)
Comment by Thomas Martitz (kugel.) - Monday, 01 March 2010, 00:30 GMT
Use nvram.bin/global_status instead of an extra file
Comment by Jonathan Gordon (jdgordon) - Wednesday, 03 March 2010, 06:31 GMT
needs a sync (easy one...)
why 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
Comment by Jonathan Gordon (jdgordon) - Wednesday, 03 March 2010, 08:04 GMT
a few more comments because this should get commited soon...
remove the commented out code and extra unneeded comments.
the 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 second.
skin_buffer_dump_size() should be named something else, and the comments attached to it are incorrect.
edit: 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
Comment by Thomas Martitz (kugel.) - Wednesday, 03 March 2010, 15:14 GMT
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.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 03 March 2010, 17:16 GMT
yes the slow down is only during run, because like I said it opens every file twice
Comment by Thomas Martitz (kugel.) - Thursday, 04 March 2010, 02:17 GMT
handle themes that are too big on boot better. still no resetting to built in when the buffer is to small.
Comment by Andy (cbs_ghost) - Sunday, 07 March 2010, 09:17 GMT
Works great, but seems it need to be update again.