Theme Editor Feedback

[Bieber]

I'm the GSOC student working on the Theme Editor this Summer, and I'm interested in any feedback, feature requests, etc. the community has about it.  There's a wiki page about it with some more info.  If you have the Rockbox source tree checked out, you can just "qmake" and "make" the project in /utils/themeeditor, or else get precompiled binaries for Windows/OSX from the link on the wiki page.

[AsusFreak]

Hi Bieber,

I've been testing your theme editor already for a while and I'm quite happy with your work til now. Thanks for being so busy with it.

I'm currently working with your latest windows build r27405 and you've done a wonderful job so far.

It's great to open a project and have all important files listed automatically.
Selecting a file leads to the colored code so orientation is made easy.

The code is parsed in the background and errors are shown immediately - great help while coding new lines.

You've done a preview of the skins in the right upper window so the success of adding new code is shown visually - super.

In the right lower window I find the parse tree so I'm able to get more deeper into the parsing functionality.

Really a great job so far. And you've been really fast with your planned implementation !



Now that you're asking for some feedback I would like to give some suggestions as well.

1. You've added touchscreen areas, that's great but if I put them in a viewport like

Code: [Select]

%V(161,192,35,47,0)%Vf(CCCCCC)%Vb(000000)
%T(0,0,35,35,play)

If I take a look at the preview window then the touchscreen area is shown in a viewport %V(0,0,35,35,..) instead of %V(161,192,35,35,...). I think that's no big problem for you and it's the only bug I've seen so far.

2. You've placed the included files in the left window. This is the first orientation which files are important for this project. Then you've placed the source window in the middle and on the right side the preview and the parsing windows. I would like to have a bigger preview window and a smaller file window. I can resize them that's right, but maybe you could swap them (in spite you're right looking at the process: if you go from left to right you see the right order of process steps from opening a file to the parsing results) But for me a bigger preview window for the D2 skins would be great.

3. The parsing window is no big help for me trying to realize a new skin. Especially having errors I currently see no additional use of this output. I write the code with your theme editor and try it with the simulator and with the debugwps option there's the same output. And these information aren't helpful as well in most cases for me. But as a fair comment: I'm able to close the parsing error then it's ok for me (if the saved space could be used for the preview window (see 2.).


So no big issues at the moment. I'm really happy with the theme editor since now. Great job so far - stick with it, I'm really looking forward to your next steps. I saw on the wiki page you will try to implement graphical editing of WPS documents. So I'm really curious about this.

Keep it up, Bieber, you're doing a great job.

[Bieber]

Thanks for the feedback.  I'll definitely fix that touch area issue shortly.

As for the layout, I do think that in later releases I'll leave the parse tree hidden by default, and probably make the preview window a little bigger.  In the meantime, you can completely rearrange the panels if you want: you can close them with the little 'x' buttons in the corners, and if you want you can grab them by the title window and drag them around, or even tear them off into separate windows.  I've also added a timer module recently (I believe this was present in the last Windows build) that you can access from the view menu and dock along the edges of the main window, while we're on the topic of the layout.  You can use that to step the preview forward through time, if you like.

[AsusFreak]

Thanks for these hints, Bieber.

Dragging the windows is a nice feature but after closing the theme editor and restarting my window configuration does not exist anymore. Shouldn't it be stored in a config file?

EDIT: Sorry, I got two instances of your theme editor opened. Window positions were stored correctly - my fault.

A nice feature could be that the actual viewport regarding the actual line in the code window would be highlighted in another colour in the preview window. This would make it easier to change the viewport limits.

And yes the timer is a nice feature to test the clock depending output of the skin. On the other hand you'll get the question what's in scope of your theme editor and what's the goal of the simulator. My personal view is that a theme editor should be able enter the code, eventually to edit the code on a graphically way and to show parsing errors. The realtime behavior of the skin can be tested with the simulator, I think. So in my opinion it could be sufficient to spend the time on editing and simple previewing features otherwise the simulator will not be needed anymore

So no offence, your hard work gives great results - thank you.

Edit, 15.07.2010:
Thanks for providing r27436, Bieber.
Now the touchscreen areas are working, thank you.
Would it be possible to implement a feature to set conditional viewports?
Maybe you might take a look at my AF_Remote.sbs:
http://themes.rockbox.org/index.php?themeid=884&target=cowond2
The structure of a more complex theme like this depends on conditional viewports just to show the right information at the right time. With your editor we're able to see only the static viewports - do you see any possibility here to see them all depending on manually setting the conditional?

Opening and closing the project:
The last build didn't have the close project functionality. Is it right that closing the project only closes the regarding filenames in the project window? I would have expected that closing a project means closing all open files as well but they stay open at the moment and I have to close them manually.

View of the config file:
You changed the view of the config file. The structured view in the past has been dropped for a normal text based view. When I saw your structured view at the first time I was a bit confused but it reminded me that I'm in the config file and had to change parameters and not the skin code so after a while I loved this view. Why did you drop it?

Loading a file lasts a while
The loading of a wps or sbs file lasts a bit in the recent releases. Do you check now the files before loading them? I felt loading was faster in the past. Maybe an hour glass or a progressbar would be nice...

Quit does not work
Could you please check the quit program function - nothing happens when clicking at it.

Rendering sbs
If I click on render sbs and show viewports the ui viewport is shown at the wrong place. In my AF_Satin skin I defined

Code: [Select]

# Declaration of UI Viewport
%Vi(-,135,38,170,130,-)

And this viewport is shown as

Code: [Select]

%V(0,0,170,130,-)

in the preview window.

Album art
If I define album art in my WPS I get a red/white checked rectangle in the preview window but with smaller dimensions than they are in reality. Do I interpret this the wrong way?

[Bieber]

Edit, 15.07.2010:
Thanks for providing r27436, Bieber.
Now the touchscreen areas are working, thank you.
Would it be possible to implement a feature to set conditional viewports?
Maybe you might take a look at my AF_Remote.sbs:
http://themes.rockbox.org/index.php?themeid=884&target=cowond2
The structure of a more complex theme like this depends on conditional viewports just to show the right information at the right time. With your editor we're able to see only the static viewports - do you see any possibility here to see them all depending on manually setting the conditional?

Viewports can be displayed conditionally, you just need to set the appropriate tag values in the device config panel.  Is there any specific tag that's giving you problems in conditionals?

Opening and closing the project:
The last build didn't have the close project functionality. Is it right that closing the project only closes the regarding filenames in the project window? I would have expected that closing a project means closing all open files as well but they stay open at the moment and I have to close them manually.

At the moment, no, it doesn't close any open files.  I'll change this in the near future.

View of the config file:
You changed the view of the config file. The structured view in the past has been dropped for a normal text based view. When I saw your structured view at the first time I was a bit confused but it reminded me that I'm in the config file and had to change parameters and not the skin code so after a while I loved this view. Why did you drop it?

You can now switch back and forth between the two.  If you look at the bottom left of the config file viewer, you'll see two icons.  Clicking the icon with the bars will switch you to the old view, and it will stay there as long as you want.

Loading a file lasts a while
The loading of a wps or sbs file lasts a bit in the recent releases. Do you check now the files before loading them? I felt loading was faster in the past. Maybe an hour glass or a progressbar would be nice...

Could you give me your hardware/OS specs?  I've never heard of the program hanging on file load, and it shouldn't take long at all.  How long of a delay are you experiencing?

Quit does not work
Could you please check the quit program function - nothing happens when clicking at it.

This may be an OS-specific bug: the quit action seems to be working fine for me.

Rendering sbs
If I click on render sbs and show viewports the ui viewport is shown at the wrong place. In my AF_Satin skin I defined

Code: [Select]

# Declaration of UI Viewport
%Vi(-,135,38,170,130,-)

And this viewport is shown as

Code: [Select]

%V(0,0,170,130,-)

in the preview window.

I'll have to look into this, I thought I had the UI viewport displaying correctly.

Album art
If I define album art in my WPS I get a red/white checked rectangle in the preview window but with smaller dimensions than they are in reality. Do I interpret this the wrong way?

There's an option in the device config panel that allows you to change the album art size, so that you can observe how it will scale at different sizes.  If you want it to fill the entire area you have allocated for the album art, just set it to a size >= that area.


I would also like to add that if you're compiling from source, you can rearrange the options in the device control panel by editing the file resources/deviceoptions (which is just a text file that defines all the options) and recompiling.  If anyone comes up with a configuration that's more convenient than the default layout, please send it along to me and I may replace the default with it.

[yzflcyq]

Found a bug:

Code: [Select]

%V(4,242,232,20,2)%Vf(a4c0e4)
%s%ac%?id<%id%?iy<(%iy)>|%?iy<Year:%iy|--->>

I wrote this in order to show like:

Air OST(Year:2002)

And it does works on my DAP. But when I wrote this in the Theme Editor it doesn't work.

[Bieber]

Found a bug:

Code: [Select]

%V(4,242,232,20,2)%Vf(a4c0e4)
%s%ac%?id<%id%?iy<(%iy)>|%?iy<Year:%iy|--->>

I wrote this in order to show like:

Air OST(Year:2002)

And it does works on my DAP. But when I wrote this in the Theme Editor it doesn't work.

When the new parser is fully integrated in Rockbox this won't parse.  The problem is that you put parentheses around %iy without escaping them.  The correct way is like this

Code: [Select]

%V(4,242,232,20,2)%Vf(a4c0e4)
%s%ac%?id<%id%?iy<%(%iy%)>|%?iy<Year:%iy|--->>

[DavidW]

I hope this is the right place for this...

How about adding a function that will convert an old theme to the new format?

Thanks for your hard work.

[[Saint]]

DavidW

This is already possible, a pre-compiled executable is here:

skinupdater

The syntax is: skinupdater wps_name.wps wps_new_name.wps

Update your .wps .fms and .sbs files seperately



[St.]

[mc2739]

Great work so far on the theme editor.  I have already used it to clean up a couple of my themes that had issues with viewport alignment.  I am looking forward to the finished project.

Some comments after testing themeeditor-r27559 (windows version)

1. FMS preview does not render SBS info

2. An SBS viewport that is conditional on current screen (%cs) and playmode (%mp) does not render properly.  For example, I have an SBS viewport set to display song info while the song is playing or paused and the screen is the menu screen.  It is set to display an empty viewport when the song is stopped or the screen switches away from the menu screen.  The current behavior of the theme editor displays this viewport properly when displaying the SBS preview, but when displaying the WPS preview it merges the contents displayed in the SBS with the contents of the WPS viewport that occupies the same screen coordinates.

3. The Export Project function creates a zip file with a top level directory with no name.  It also includes all font files which are used in the theme.  The standard practice for theme submission is to only include fonts which are not already included in the font pack. 

4. Export Project messages could be a little more descriptive:

• Warning: configfile not found (what configfile)
• Warning: themebase not found (what themebase)

[Bieber]

Great work so far on the theme editor.  I have already used it to clean up a couple of my themes that had issues with viewport alignment.  I am looking forward to the finished project.

Some comments after testing themeeditor-r27559 (windows version)

1. FMS preview does not render SBS info

2. An SBS viewport that is conditional on current screen (%cs) and playmode (%mp) does not render properly.  For example, I have an SBS viewport set to display song info while the song is playing or paused and the screen is the menu screen.  It is set to display an empty viewport when the song is stopped or the screen switches away from the menu screen.  The current behavior of the theme editor displays this viewport properly when displaying the SBS preview, but when displaying the WPS preview it merges the contents displayed in the SBS with the contents of the WPS viewport that occupies the same screen coordinates.

Could you start a FlySpray bug page or otherwise contact me with the theme files that are causing these bugs for you?  I have some suspicions as to what might be going wrong, but I'll need to be able to reproduce the issue the same way you're experiencing it to know for sure.

3. The Export Project function creates a zip file with a top level directory with no name.  It also includes all font files which are used in the theme.  The standard practice for theme submission is to only include fonts which are not already included in the font pack. 

Is it not standard practice to make the first directory in the zip file .rockbox?  This is how I see all the themes on the themesite.  As for the fonts, the only fonts you want to put in your project's fonts/ directory are ones that you want bundled with the theme.  If you take a look at the "Render" tab of the preferences dialog, you'll see that there's an option for a directory to store fonts in and a button to download the font pack.  Just pick somewhere you want to keep the font pack (mine is ~/rbthemes/.fontpack, just pick somewhere that won't get in your way), click the download button, and it will dl/extract the font pack in that directory (or you can just copy them over if you already have it).  You'll be able to use those fonts in your themes without having to put them in the fonts/ directory, so they won't be included in the exported project

4. Export Project messages could be a little more descriptive:

• Warning: configfile not found (what configfile)
• Warning: themebase not found (what themebase)

configfile and themebase are both internal settings that shouldn't be showing up there   Could you show me the config file that gave you those warnings?

[mc2739]

Could you start a FlySpray bug page or otherwise contact me with the theme files that are causing these bugs for you?

I have a copy of the theme uploaded on jdgordan's BIG display code cleanup task here: http://www.rockbox.org/tracker/task/11470?getfile=22284

Is it not standard practice to make the first directory in the zip file .rockbox?

The zip file that i get when exporting the project has a folder with no name at the top level and then the .rockbox folder within that one.  I see this when opening the file with 7-zip.  Extracting the zip file contents does give the expected results.  See the attached screen shot.

As for the fonts, the only fonts you want to put in your project's fonts/ directory are ones that you want bundled with the theme.

You are exactly correct on this one.  I had set up the font pack location with an earlier version that I was looking at, but had not done so this time.

configfile and themebase are both internal settings that shouldn't be showing up there   Could you show me the config file that gave you those warnings?

See the theme linked above.

[Bieber]

The zip file that i get when exporting the project has a folder with no name at the top level and then the .rockbox folder within that one.  I see this when opening the file with 7-zip.  Extracting the zip file contents does give the expected results.  See the attached screen shot.

I can't reproduce this one, I'm going to have to take a look at with 7-zip on a Windows machine later.  I'm pretty sure that my last couple revisions fixed all the other bugs you reported: if you get a chance, it would be nice to know if it worked out for you

[mc2739]

SBS info does now render in the FMS preview and the configfile and themebase warnings on export are no longer displayed.

The SBS no longer merges the two lines on the WPS preview, but the line is no longer displayed on the SBS preview (%?cs on line 37 of the SBS file).  Also, two additional lines which should prevent the the shuffle and repeat icons from displaying on the radio screen are not rendered properly (%?cs on lines 55 and 59 of the SBS file).

[Bieber]

The SBS no longer merges the two lines on the WPS preview, but the line is no longer displayed on the SBS preview (%?cs on line 37 of the SBS file).  Also, two additional lines which should prevent the the shuffle and repeat icons from displaying on the radio screen are not rendered properly (%?cs on lines 55 and 59 of the SBS file).

That one's not really a bug, just a less-than-intuitive feature   To get those things to render, you had to change the "Current Screen" setting in the "Misc." tab of the device options panel.  I just committed an update that checks the file extension of documents when they're first loaded and changes the "Current Screen" setting to "WPS" for WPS documents, "FM Screen" for FMS documents, and "Menus" for SBS documents, so that should give you a better preview by default.