Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Battery/Charging
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by jdgordon - 2010-06-24
Last edited by jdgordon - 2010-07-11

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

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

You should not see any difference… let me know if you do

Closed by  jdgordon
2010-07-11 06:45
Reason for closing:  Out of Date

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?

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.

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…

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

new version so all the plugins build

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.

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

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.

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.

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)

OK thanks,
comment and viewport handling is a bit wierd so I'm going to try this from another angle….

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing