Rockbox

Tasklist

FS#11435 - Use the new skin parser in rockbox!

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Thursday, 24 June 2010, 09:59 GMT
Last edited by Jonathan Gordon (jdgordon) - Sunday, 11 July 2010, 06:45 GMT
Task Type Patches
Category Battery/Charging
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

Details

This patch changes the core to use the new skin parser..

You should not see any difference... let me know if you do
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Sunday, 11 July 2010, 06:45 GMT
Reason for closing:  Out of Date
Comment by Thomas Martitz (kugel.) - Thursday, 24 June 2010, 10:55 GMT
You didn't change the displayer, right? Do you think it could be a good idea to rework the displayer too? I haven't looked at the patch, but I assume you need to convert the parse tree into the old array like system so legacy displayer can work with it?
Comment by Jonathan Gordon (jdgordon) - Thursday, 24 June 2010, 11:01 GMT
I made sure to not touch the displayer.

I originally wanted to replace the entire engine in one hit but that turned out to be way too huge of a task.

This patch converts te tree to the array, the next step is to replace all the parse_X functions with ones which directly use the preparsed args, then slowly replace the displayer to work with the tree.
Comment by Jonathan Gordon (jdgordon) - Thursday, 24 June 2010, 11:15 GMT
This patch goes onto of the above one o fix the playlist viewer in the wps to use the tree directly... seems to work :p please try both. (most bugs will probably be in the first one only thugh). BE CAREFUL to not put any tags in the Vp that arnt supported yet.. very minimal error checking so far...
Comment by Jonathan Gordon (jdgordon) - Thursday, 24 June 2010, 11:48 GMT
put it on my d2 and skin usage and boot time is both up (hardly surprising really seen as there is an extra step).. I might boost to fix the boot time issue
Comment by Jonathan Gordon (jdgordon) - Thursday, 24 June 2010, 12:00 GMT
new version so all the plugins build
Comment by Michael Chicoine (mc2739) - Thursday, 24 June 2010, 15:39 GMT
testing newparser.1.diff with r27106 on e200v2 and cabbiev2

Vertical spacing is not correct. All lines are one line higher than svn. It seems to be caused by the "#Now Playing" comment line. With the comment line removed, it displays correctly.

Comment by Jonathan Gordon (jdgordon) - Friday, 25 June 2010, 04:44 GMT
OK, seems I was handling comments wrongly.. try this one. newplaylistivewier.0.diff should apply cleanly to this still

edit: updated again to hopefully make %ax work
Comment by Michael Chicoine (mc2739) - Friday, 25 June 2010, 19:19 GMT
newparser.2.diff does fix the comment problem.

New Issues:
Using: Wide Cabbie theme on e200v2 (http://themes.rockbox.org/download.php?themeid=1)

1. Some viewports do not display. These vps do not have any %Vf() or %Vb(). Adding, at a minimum, %Vf(CCCCCC) to the vp definition corrects the problem. It looks like vps with images only do not have a problem, only vps that display text info.
2. When album art is available, this theme has a display line which consists of two viewports. The first vp has fixed text "Next:" and the second vp scrolls "Artist - Title". The second vp definition uses "-" for the width. This causes the second the second vp to be 2 pixels higher than defined and overwrite the first vp. Replacing the "-" with the remaining screen width corrects this problem.


I am attaching screendumps of the incorrect and correct wps displays and a diff of the original wps file and the updated wps file.
Comment by Michael Chicoine (mc2739) - Friday, 25 June 2010, 19:36 GMT
I missed two vp definitons that did not display and needed the %Vf() added. They are line 69 and line 100, and were also text fields.
Comment by Michael Chicoine (mc2739) - Friday, 25 June 2010, 19:52 GMT
New issue:

e200v2 - r27106 with newparser.2.diff and newplaylistviewer.0.diff and using Wide Cabbie theme

When playback starts the last song in a playlist, either through normal playback or by skipping or fast forwarding to the last song, I get panics or lockups:

Undefined instruction
at 3082673C (or 30826760, 30826F30)
Comment by Jonathan Gordon (jdgordon) - Saturday, 26 June 2010, 08:55 GMT
OK thanks,
comment and viewport handling is a bit wierd so I'm going to try this from another angle....

Loading...