Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category User Interface → Themes
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Christopher Williams - 2008-04-03
Last edited by Jonathan Gordon - 2010-06-06

FS#8844 - "Smart" expansion of sub-images in a conditional list

I’d like to see a “smart” expansion of sub-images in a conditional list so an image expands into a list of its sub-images. Currently, in a conditional tag like the current volume, we currently have to list each sub-image we want to display for each volume:

%xl|a|volume.bmp|16|0|9|
%?pv<%xdaa|%xdab|%xdac|%xdad|%xdae|%xdaf|%xdag|%xdah|%xdai>

Instead, I would like to be able to write it much shorter, like so:

%xl|a|volume.bmp|16|0|9|
%?pv<%xda>

so that the “%xda” tag expands not into the first sub-image (the current behavior) but into a list of all sub-images in the image. This would considerably shorten WPS files that have many sub-images. Also, this change would make writing WPS files slightly easier and less error-prone, and would allow for more than 52 sub-images per image.

Closed by  Jonathan Gordon
2010-06-06 11:42
Reason for closing:  Later
Additional comments about closing:  

something like this will happen once we change to the new skin format….

Christopher Williams commented on 2008-04-04 07:31

Here’s a patch that works with limited testing (I just figured out how the WPS parser worked today!). It expands sub-images only inside conditionals; essentially, for each sub-image after the first one, this code adds the “conditional option” token (the “|” character) and another image display token, taking care to keep the token count correct, etc.

I also modified the “iCatcher” WPS to demonstrate how this new tag behavior can be used and how much space it can save. The original iCatcher.wps file (for the Sansa e200, anyway) is 2095 bytes, and the new file is only 1406 bytes. That’s a savings of 689 bytes, or about a third of the file size! There’s a lot less redundant code in it now.

The only downside that I see with this new behavior is that it might break older WPS files that use sub-images inside a conditional list without always specifying the sub-image’s ID. On the other hand, the parser has become stricter over time anyway and has broken numerous WPS files already, and sub-images are a relatively new feature of the WPS parser anyway, so I believe this is not a big disadvantage.

Peter D'Hoye commented on 2008-04-04 07:56

Nice of you to mention the filesize savings, but they are irrelevant if they’re made in a wps file. Code size reduction would have been much better ;)
Not that I’m against this request/patch. Does it protect against the case where the strip contains less images than the conditional requires or do I fail to understand something (I’m not too familiar with the GUI code)?

Christopher Williams commented on 2008-04-04 08:18

File size savings (and the resulting simplicity granted by this patch) is somewhat relevant to the WPS author. Of course, looking at the code again I can see how I can reduce code size as well (by moving some bits of common code to a routine), but this patch adds relatively little code as it is.

I don’t know of any conditionals that require a certain number of options, so it’s not an issue of how many sub-images the image contains. In conditionals that have a fixed number of options (such as 5 for the %mp conditional tag), I believe any unspecified option is treated as empty, but I think this occurs at a higher level after the WPS is parsed (which is where my code runs). This patch inserts only as many sub-images as are specified in the WPS file for that image (in the last parameter of the %xl tag), so it should have the exact same effect as manually writing out each sub-image in increasing order inside the list.

Christopher Williams commented on 2008-04-04 09:31

It’s probably not my responsibility to document the user-visible changes of my code (I’m just a programmer!), but I updated the manual to reflect the different behavior this patch causes.

I’m not very familiar with TeX, so I made as few changes as needed. Basically, take this diff at your own risk. That’s not to say that it will make your computer blow up, at least mine didn’t combust when I made the PDF with the changes. :)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing