From: Jonathan Gordon <jdgordy_at_gmail.com>
Date: Mon, 7 Nov 2011 02:24:37 +1100

Hi all,
The only long term solution to the issue of the skin buffer size is to
completly redo how the skin engine uses its buffer, to that end I have
just started the rather large task of replacing every pointer in it to
an offset into the (currently) shared buffer which is then turned back
into a pointer when it is being used.

One annoyance with the current skin engine code is that all skins are
linked together with the single buffer, this means that if any skins
need reloading (user request to change theme perhaps) all of them need
to be reloaded. So one of the goals with this new work is to split
that up so each skin manages its own buflib handle, allowing skins to
be un/loaded on demand (there is no point loading the fm skin and
wasting buffer space if the user never actualy loads the fm screen for
example.)

The plan is to load the skin into the plugin buffer (and buflib
handles for images as done now), then do a memcpy() of the loaded skin
into a new buflib handle of the exact size needed for the skin. This
means people wanting boring themes don't limit people wanting Over The
Top themes (and no extra ram is wasted). Using offsets to do this
means there is also no need to manage pointers when the handle moves
around.

Another ultimate goal is to draw more of the UI using the skin engine,
specifically screens which are currently hard coded for different
screen sizes (the time&date screen is an obvious candidate), this is
very far down the track though so we'll see if that ever actually
happens.

The attached patch is the very start of this work. I'm really writing
this email because before I get too far in I want to make sure the
macros are going to be acceptable.

Three macros and a typedef have been added.
SKINOFFSETTOPTR() and PTRTOSKINOFFSET() convert between the offset and
the real pointer. I originally wanted to put the type in the first
macro and use that instead of void* but not sure if that is really
needed.
the 3rd macro OFFSETTYPE() is added because I realised the structs
used by the various tags will become very confusing if all the members
are skinoffset (or long) types instead of the pointer type which the
data actually contains. So I added this macro to hopefully help
document the code a bit.

Are these going to cause people to grimace? Is there a better way to
do what they are doing?
This is going to be a rather massive diff so if these macros are
unacceptable I really want to know now rather than once it is all
done.

Cheers
Jonathan


P.S This is on my skinengine_to_offsets github branch
https://github.com/jdgordon/rockbox/tree/skinengine_to_offsets and
will likely be pushed upstream as a single patch when it is done, but
would anyone like to help out? :D

P.P.S If you have a patch that touches apps/gui/skin_engine/* please
talk to me on IRC before committing so we dont cause extra work for
eachother.

Received on 2011-11-06