Rockbox

Tasklist

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

Attached to Project: Rockbox
Opened by Christopher Williams (christop) - Thursday, 03 April 2008, 18:56 GMT
Last edited by Jonathan Gordon (jdgordon) - Sunday, 06 June 2010, 11:42 GMT
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

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.
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Sunday, 06 June 2010, 11:42 GMT
Reason for closing:  Later
Additional comments about closing:  something like this will happen once we change to the new skin format....
Comment by Christopher Williams (christop) - Friday, 04 April 2008, 07:31 GMT
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.
Comment by Peter D'Hoye (petur) - Friday, 04 April 2008, 07:56 GMT
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)?
Comment by Christopher Williams (christop) - Friday, 04 April 2008, 08:18 GMT
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.
Comment by Christopher Williams (christop) - Friday, 04 April 2008, 09:31 GMT
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...