Rockbox

Extracted the patch to make the skin engine id3==NULL safe, which is needed for the statusbar to function without music playing initially.
fix voice for filename extension also.

Apply both patches, they're incremental (the token one first).

   make_tokes_id3_save.diff (14.6 KiB)
   custom-statusbar-v1b.diff (50.3 KiB)

the sub/lines stuff is kicking my ass so I didnt get to look at this more…. :(

I don't get this to work. i downloaded the latest tar from your site and compiled a win32 sim with debian vmware image for the gigabeat. i fear it might have simething to do with filenames containing spaces, am i right? I get no errors by the wps parser though. does my code look ok to you?
great work by the way. what i don't understand is, you mentioning the %Vi tag. what is it?

   Blackglass-Boombox.zip (318.9 KiB)

Doesn't look incorrect to me at the first glance. Have you chosen did you go to theme settings → statusbar to select *custom*?

The %Vi tag describes the viewport which doesn't disturb statusbar, i.e. the viewport where the main menu would be drawn into (if you don't use the ui viewport, for example). That will act as default viewport for every part in rockbox (again, unless you specify a ui viewport).

Thanks for clearing that up. i forgot to choose custom indeed. this looks superdope! is there much keeping it from being committed?

   dump 090829-010613.bmp (150.1 KiB)
   dump 090829-010459.bmp (150.1 KiB)

The upcoming release for example.

/me is *really* looking forward to themes using this… I just hope everyone is reembering to do it in such a way that they dont look silly when no music is playing… and before anyone asks, yes there will be a tag so the skin can know if its in the wps, or browser, or setting, or menu :)

This is brilliant, if I remember correctly jBlackglass had a patch like this except it wasn't customizable.

sync

   custom-statusbar-v1c.diff (50.3 KiB)

It can't compile due to filetypes.o?

make clean should help.

Works indeed :) Its possible to merge WPS and menus/settings into one thing, but there are some bugs though. Some images fail to redraw after exiting plugins and some settings and so does text which contains song information (like song name, album name and etc.) But it works … can't wait to see it commited!

Please upload the theme so I that I can investigate it.

Here You go.

   scribble.zip (136 KiB)

the first thing I see is that you didn't define a %Vi (mentioned in the first post), it's not strictly required, but highly recommended to avoid some redraw issues. That could be one reason.

plugins dice, mazeam, pong, sliding puzzle, snake, sokoban and solitaire either need to be adjusted to use the custom ui viewport dimensions, or have to be made conform to other game menus (i.e. have the font being written on a black background).
all application plugins except clock, lamp, paint and text_editor need the same adjustments.
demos that have to be fixed: bounce, cube, logo, mosaique, snow and vu_meter.

It'll look funny on targets that have small screens, since then the plugins will only have a tiny space left in which they are displayed (depending on the dimensions of the custom ui vp of course).

i know this'll be an awful load of work, but i think to really commit this as clean as possible it has to be done. in the end, i think it'd be logical to have all plugins act the same (i.e. have the same background for the menu and also have the menu as the starting screen OR have them all start right away).

DONE! Still no effect…
Album art/ "art" images still disappear, and information about track (as mentioned before) disappears as wall, BUT if I replace disappearing lines with something different like volume or battery level information, then those lines wont disappear. What I'm trying to say is that this bug has nothing to do with my theme :)
I took your "slick" theme (the one which is uploaded here), modified .sb file by replacing clock with information about track ( replaced this "%ac%ck:%cM %cp" with this "%it") and after exiting plugin or using color changer (or whatever that thing is called) this line (songs title) disappears.
There are some more strange things:
Replacing backdrop (using "Set As Backdrop" setting) has some strange effects;
and
Changing themes SOMETIMES couse UI viewport not to reset. (try changin your "slick letitblue" to "slick bushfire").

plugins dice, mazeam, pong, sliding puzzle, snake, sokoban and solitaire either need to be adjusted to use the custom ui viewport dimensions, or have to be made conform to other game menus (i.e. have the font being written on a black background).
» all application plugins except clock, lamp, paint and text_editor need the same adjustments.
»demos that have to be fixed: bounce, cube, logo, mosaique, snow and vu_meter.

I rather fix the plugins that use a black background. It should eventually be done, but likely not before this is committed.

It'll look funny on targets that have small screens, since then the plugins will only have a tiny space left in which they are displayed (depending on the dimensions of the custom ui vp of course).
Entirely the themers fault. That applies to all menus, not just plugins.

Album art/ "art" images still disappear, and information about track (as mentioned before) disappears as wall, BUT if I replace disappearing lines with something different like volume or battery level information, then those lines wont disappear. What I'm trying to say is that this bug has nothing to do with my theme :)

Alright, static information is apparently not being redrawn when it should (originating from the WPS, static information is supposed to be updated on track change (or a similar event). I can see that they aren't being
redrawn. I'll have a look. Album art is a bit tricky anyway, it's not really supposed to work as I haven't done any changes.

"Replacing backdrop (using "Set As Backdrop" setting) has some strange effects;"
What do you mean here?

Changing themes SOMETIMES couse UI viewport not to reset. (try changin your "slick letitblue" to "slick bushfire").
Yea, the themes need updating to include "ui viewport: -" which resets it.

What do you mean here?
Its hard for me to explain this in English :D. OK, by using your DAP go to : Files>.rockbox>backdrops>select any backdrop file (except one which is currently being used) and enter Context Menu. Now hit "Set As Backdrop" item. And this is it- screen is completely messed :) I know, this is minor bug, but still…

Works fine here. Of course the statusbar draws over the backdrop which might look funny, but that's expected and cannot be fixed. The backdrop is part of a theme.

H300 with the custom-statusbar-v1c patch and r22570:

When holding REC during startup in order to reset the settings the device hangs at the rockbox logo. The disc continues spinning and a hard reset is required.

Take a look at this, I compile this on my 5G 60 gig. It's not on a clean build but I don't have a patches that affect filetypes.o

   Custom StatusBar.png (14.4 KiB)

comments from the v1c diff:

- lots of debug code which shouldnt be needed if its ready for commit
- dont use goto in find_image(), its just as easy to use a fail bool on the while loop, actually all that is debug so dont touch that function
- 1 code line per line… "vp_refresh_mode = 0; hidden_vp = true;" is unacceptable
- +#define SDEBUGF(varargs…) //DEBUGF(varargs) ← remove
- get rid of all the debug code
- skin_data_load() should be changed to accept its default vp
- %vi stands for *info* ?! I assumed invisible untill I saw VP_INFO_LABEL….
- +/* these are never drawn, nor cleared, i.e. just ignored */
+#define VP_NEVER_VISIBLE 0x8 ← then call it VP_IGNORED and remove the comment
- unsigned char sb_file[MAX_FILENAME+1]; /* last wps */ ← dont be lazy…. fix the comment… ditto on the rsb_file
- I think sbs and rsbs is better extensions (statusbar skin)

+ TEXT_SETTING(F_THEMESETTING,sb_file, "sb",
+ DEFAULT_WPSNAME, WPS_DIR "/", ".sb"), ← more copy+paste laziness

and in make_tokes_id3_save.diff :
- I'm not wild about the HANDLE_NULL_ID3* macros… but can live with them… the zero and na variables should be renamed to something better… zero_str or something…

other than all that, my only comment would be that it would be better if the old/current/svn statusbar was completly removed (expect in the rec screen), but I can live with that being a seperate patch for later…

heh, half of your comments on the debug stuff, which is going to be removed any way :)

%Vi stands indeed info. It's also invisible, but that also applies for the default viewport. As for that, I was also always curious what %Vd and %Vl stand for :)

+ TEXT_SETTING(F_THEMESETTING,sb_file, "sb",
» + DEFAULT_WPSNAME, WPS_DIR "/", ".sb"), ← more copy+paste laziness

That is not a typo, since DEFAULT_WPSNAME is just cabbiev2, without the ext. That would also be correct for the sb (i.e. default sb is cabbiev2.sb). But, we could also make it like "#define DEFAULT_SBNAME DEFAULT_WPSNAME" and use DEFAULT_SBNAME.

- 1 code line per line… "vp_refresh_mode = 0; hidden_vp = true;" is unacceptable
Since when is that unacceptable? I didn't know that, I couldn't find something in CONTRIBUTING regarding that either.

Resync. NOTE: I changed the extensions to .sbs and .rsbs as per JdGordons suggestion.

I also fixed the bug that exiting plugins don't redraw the statusbar properly (artist info was missing, for example).
Also addressed a few of JdGordon's remarks (comment typos, DEFAULT_SBS_NAME, unchange find_image()).

Full changelog (i.e. commit log) is on the git repo linked in the first post.

   custom-statusbar-v2.diff (62.3 KiB)

This still can't compile on my 5G, see the image.

   Skin error.png (5.4 KiB)

Delete the entire build/ directory, and make a new one, then try again.

Sync and fix a few redraw issues.

   custom-statusbar-v3.diff (62.2 KiB)

So dose this work with Custom patches or dose it not?

Concerrning the freeze when resetting settings during boot reported earlier:
The freeze seems to be related to the splash screen that is shown then. When commenting out the line 'splash(HZ*2, str(LANG_RESET_DONE_CLEAR));' in apps/main.c then there is no freeze anymore.
Maybe this helps finding the problem.

Alright, first go at supporting other screens with the skins. Apply these patches to
a) have %mp indicate state of recording and radio as well (recording, rec pause, radio, radio pause are new values for %mp [in that order])
b) have the %cs tag which indicates the current screen (lists or wps, rec, radio screens [in that order]). it can be used conditionally

both patches could be used with conditional viewports showing different information in the FM screen for example.

More that this shouldn't be really needed for the statusbar to work with other screens. However, displaying metadata tags within rec screen or radio still shows the tags for the last played music.

Patches are against SVN, I don't have them tested yet with the statusbar patch itself.

   0001-Change-pm-tag-to-use-cur... (3.2 KiB)
   0002-Add-cs-tag.patch (4.1 KiB)

When I try to patch this I get an error saying the file apps/gui/skin_engine/skin_tokens.c can't be found.
Everything else patched properly.
What do I do to fix this?
Thanks
Will

.kugel, i think you changed the %mp tag and not the %pm tag

ah yes, that was a type in the comment.

Any updates?

Is something not working?

Sync.
This one includes the patches I posted on sept 11th (i.e. the two above).

   custom-statusbar-v3b.diff (62.4 KiB)

Almost everything works except album art and the peak meter. Album art is displayed but not resized. Also, Rockbox (3.4) has crashed a few times after patching this ('PANIC - Event list full' or something like that.)

Hi,

did you really include the patches you posted on sept 11th?
With the v3b version and r22843 neither the %pm nor the %cs tags work correctly anymore:
The %pm tag shows in the rec- and radio screen always the first conditional (stop). So the new states (recording, rec pause, radio, radio pause) never get shown, the old states (play, pause, ff, rw) still work fine.
The %cs tag always shows the last conditional (radio screen), so you always see the radio specific information even when not in radio mode.

sorry, i meant %mp (not %pm).

Seems I made a little mistake. The branch on the git repo at repo.or.cz has the patches integrated.

The PANIC: event line full is easy to fix, I'll have a look (grep for "#define MAX_EVENTS" manually if you can't wait).

Album art is difficult since Rockbox doesn't support any way of multiple album art in memory right now. How exactly is the peakmeter not working? It may not update often enough (besides, peak meter needs to update really often, which is likely to noticeably slow down browsing experience, hence I don't think it's a good idea to use it - but yea, it should probably still work regardless).

The dots in the middle are displayed, but the peak doesn't seem to update. I doubt anybody would use it, so I would just remove it from the custom stausbar code. If you did get it to work, it would probably be too slow anyways.

This one supports albumart (apart from a few minor redraw issues).

   custom-statusbar-v4.diff (88 KiB)

great work, this has been worked well for me. and I have some questions.
I noticed that track informations on custom statusbar are not changed on track change while in WPS. Isn't it fixable?
it doesn't looks correct that HANDLE_NULL_ID3_NUM_ZERO and HANDLE_NULL_ID3_NUM_NULL are same in skin_tokens.c.
Also it doesn't looks correct for me that these macros depend on local variable. why not just use "0" and so on? And why do they return na but NULL?
What is SB_DIR in settings.h? it doesn't seem to be used. will it be used in the future?
DEFAULT_SBS_NAME and DEFAULT_WPSNAME seem to be mixed up in settings_list.c.

This isn't for 3.4 is it?

great work, this has been worked well for me. and I have some questions.
» I noticed that track informations on custom statusbar are not changed on track change while in WPS. Isn't it fixable?
Probably a resync bug slipped in.

it doesn't looks correct that HANDLE_NULL_ID3_NUM_ZERO and HANDLE_NULL_ID3_NUM_NULL are same in skin_tokens.c.
arg, typo. thanks for seeing that

Also it doesn't looks correct for me that these macros depend on local variable. why not just use "0" and so on? And why do they return na but NULL?
expanding "0"/"n/a" each time might mean a space waste. I don't know how gcc handles if the same literal constant is used several times. I wanted to make sure it's always using the same copy, that's why I allocate them explicitly.
Do you think they should all return NULL? That way the bar wouldn't show anything if id3 == NULL. With "n/a" it's clear that track info isn't available yet.

What is SB_DIR in settings.h? it doesn't seem to be used. will it be used in the future?
Should be defined the same as WPS_DIR, and used for ®sbs; if not it's a bug (not a too critical one though).

DEFAULT_SBS_NAME and DEFAULT_WPSNAME seem to be mixed up in settings_list.c.
Possible another resync bug.

Thanks for the reply.
> Do you think they should all return NULL?
I'm not sure.

This are the 4 patches I mentioned on the mailing list that I intend to commit in a few hours.
The files contain the commit message (i.e. the description).

   0001-Change-mp-tag-to-use-cur... (3.2 KiB)
   0002-Add-cs-tag.patch (5.1 KiB)
   0003-Make-the-skin-engine-beh... (16.9 KiB)
   0004-Rework-albumart-bufferin... (21.7 KiB)

This would be the statusbar patch after I committed the above ones.

   custom-statusbar-v5.diff (50.3 KiB)

Tiny update

   custom-statusbar-v5b.diff (49.2 KiB)