This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#10566 - Customizable statusbar (using skin engine)
Attached to Project:
Rockbox
Opened by Thomas Martitz (kugel.) - Friday, 28 August 2009, 02:52 GMT+2
Last edited by Thomas Martitz (kugel.) - Monday, 19 October 2009, 17:29 GMT+2
Opened by Thomas Martitz (kugel.) - Friday, 28 August 2009, 02:52 GMT+2
Last edited by Thomas Martitz (kugel.) - Monday, 19 October 2009, 17:29 GMT+2
|
DetailsSo, this is my work on a custom statusbar. It uses the full skin engine, and accepts all WPS tags.
It replaces the classic statusbar, but you can still use the old one. It integrates as 1) an extra setting (statusbar: custom) and a skin file (<theme>.[r]sb) which is specified with sb: <path/to/file>.sb (like wps files). If it's placed next to a corresponding wps file, it uses the same image folder. It mostly uses the existing statusbar framework w.r.t. to updating and stuff, but the sb file loads regardless of the setting (unless you specify statusbar: -, i.e. an invalid filename), so that one can toggle between classic and custom statusbar seamlessly. It works on remotes independently of the main screen. However, there are 2 rules. 1) You must use viewports. Statusbars without viewports are not supported. The default viewport is deactivated (but you can still define images within it). A fullscreen default viewport as the wps has totally messes up list and wps drawing. 2) It is highly recommended that you specifiy a UI viewport the statusbar is supposed to work with (using the new %Vi tag). This makes it possible to use any theme with any statusbar, since the vp behind %Vi will be copied into the UI viewport. The WPS will also use this as default viewport. Not using %Vi is likely to cause nasty drawing problems if no custom ui viewport is used (and with the wps also). I changed wpsbuild.pl to allow for pre-packaging custom statusbar enabled themes, but I guess we need to discuss if we really want to do that. The cabbiev2.176x220x16.sb I added is for testing and is probably not going to be committed. Added 2 test themes. The first is for e200. The second is for fuze/h300/ipod color. It uses slick_letitblue for the main screen, and a cabbiev2.rsb and .rwps for the remote. EDIT: You can always get the latest changes from my git repo: http://repo.or.cz/w/kugel-rb.git?a=shortlog;h=refs/heads/statusbar 
custom-statusbat-v1.diff
(63.9 KiB)
b/apps/SOURCES | 1 b/apps/filetree.c | 21 +++- b/apps/filetypes.c | 6 + b/apps/filetypes.h | 2 b/apps/gui/list.c | 1 b/apps/gui/skin_engine/skin_display.c | 167 ++++++++++++++++++++++----------- b/apps/gui/skin_engine/skin_engine.h | 2 b/apps/gui/skin_engine/skin_parser.c | 57 ++++------- b/apps/gui/skin_engine/skin_tokens.c | 116 +++++++++++++++------- b/apps/gui/skin_engine/wps_internals.h | 14 +- b/apps/gui/statusbar-skinned.c | 149 +++++++++++++++++++++++++++++ b/apps/gui/statusbar-skinned.h | 43 ++++++++ b/apps/gui/statusbar.c | 3 b/apps/gui/statusbar.h | 3 b/apps/gui/viewport.c | 105 ++++++++++++-------- b/apps/gui/viewport.h | 16 +-- b/apps/gui/wps.c | 13 +- b/apps/lang/english.lang | 14 ++ b/apps/main.c | 3 b/apps/menus/theme_menu.c | 9 + b/apps/plugin.c | 7 - b/apps/settings.c | 27 +++++ b/apps/settings.h | 3 b/apps/settings_list.c | 14 +- b/wps/WPSLIST | 26 +++++ b/wps/cabbiev2.176x220x16.sb | 20 +++ b/wps/wpsbuild.pl | 86 ++++++++++++++-- 27 files changed, 713 insertions(+), 215 deletions(-) 
Azure_Ultimate_Ultra.176x220x16.zip
(97.5 KiB)
slick+cabbie.220x176x16.zip
(166.1 KiB)
|
This task depends upon
Closed by Thomas Martitz (kugel.)
Monday, 19 October 2009, 17:29 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed in r23258.
Monday, 19 October 2009, 17:29 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed in r23258.
fix voice for filename extension also.
Apply both patches, they're incremental (the token one first).
great work by the way. what i don't understand is, you mentioning the %Vi tag. what is it?
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).
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 :)
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).
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").
>> 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.
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...
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.
- 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...
%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.
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.
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.
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.
Everything else patched properly.
What do I do to fix this?
Thanks
Will
This one includes the patches I posted on sept 11th (i.e. the two above).
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.
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).
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.
>> 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 (r)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.
> Do you think they should all return NULL?
I'm not sure.
The files contain the commit message (i.e. the description).