Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category User Interface
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.9
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Buschel - 2011-09-09
Last edited by jdgordon - 2011-09-24

FS#12268 - multiple WPS fonts not loading since r30478

Since r30478 my own WPS is not loading anymore, cabbiev2 works fine. This is with a simulator for iPod nano 2G. The output of the simulator says: <Unable to load font 3: ‘13-arialbd.fnt’>

The attached patch fixes the issue for me. It enlarges SKIN_FONT_SIZE.

Closed by  jdgordon
2011-09-24 14:01
Reason for closing:  Fixed

This patch, applied to r30562, unbreaks the FMS of my own ipodosha-v1.3 skin for Sansa e200 (uploaded to the themes site) on both sim and actual target (e280v1). Without this patch, the sim balked at "35-Adobe-Helvetica-Bold.fnt" (or other fonts of 16 points or larger) for the huge FM frequency display. It also unbreaks the skin mine was based on (ipodosha-v1.2 by Sergiu Rotaru) and presumably all his previous ipodosha and iPhosha skins, all of which use a similar large-font FMS. Any objections to committing?

yes, a proper fix is on the way

After discussions on IRC about the underlying memory issues–both the buflib "proper fix" and how to minimize glyphs in WPS–I figured out how to minimize the font memory used by Sergiu's WPS code without changing any features; it now runs in r30563 without this patch. (My only previous contribution was the 12-hour clock option; the rest of the code was Sergiu's.) I uploaded the revised version as ipodosha-v1.4. With that, I'm OK with waiting for the buflib fix.

Here is a quick and dirty patch to get skin fonts allocated via buflib until something better comes along. This does allocate according to the font and number of glyphs requested. I shamelessly cut and pasted from other's code (mostly Jonathan's). It may need a more selective locking mechanism because I assumed it's rarely needed to move a font while another is loading.

Freddy, your patch essentially jumps the gun on  FS#12273 , the official "use buflib for fonts" patch; I would *NOT* recommend its use. As Jonathan himself posted earlier, we need to wait on the official patch where possible. If you have any improvements to the official patch, post it in that FS.

As I posted just above you, I have since resolved my immediate problem without this patch by cutting back the glyphs (and unnecessary fonts) loaded in my skin. Anyone who can't fix their skin problems that way (and can compile) should apply Buschel's patch as a stopgap measure; everyone else should wait for  FS#12273 .

Richard, you don't have to use it but others might find it useful. (If you're that afraid of my code you should not use the number of glyphs parameter for your fonts either.)

Freddy, I think you missed my whole point: By posting a buflib-based "solution", you're essentially creating your own version of the other patch (or at least part of it). If you read that FS carefully, you'll see it has bugs that need working on before it can be committed to SVN. If your patch improves on the work being done in the other FS, it should be posted there, NOT here. Otherwise, I suspect it's too buggy to replace the patch here.

Buschel's patch simply enlarges an existing static buffer to accommodate the fonts; that "fixes" both his problem and mine without buflib (and in a stable manner), but at the cost of a larger total memory footprint for ALL users on ALL targets. That's why it's only a stopgap measure till the buflib fonts patch is ready. (I was arguing to go ahead and commit it, but that was deep-sixed in IRC before Jonathan even replied here.)

And what's that last comment about? To me that looks like a complete misunderstanding of what I did. The skin I was working with already had glyphs parameters; I got it to work patch-free by using SMALLER glyphs parameters, NOT by removing them altogether. For example, my biggest problem was a 35-point font for the FM frequency display that broke the skin with only 50 glyphs; I reduced it and some others to 20 (I only needed 14 but left some slack), and commented out some fonts that weren't being used at all. (IIRC, not using a glyphs parameter reserves 256 glyphs per font, which would have made things even worse.)

Richard, you don't have to use it but others might find it useful. Increasing SKIN_FONT_SIZE may not be a satisfactory fix for non-English users with large fonts. I'm not competing with Jonathan for an SVN commit. This is a temporary fix people can use until JD finishes his patch, which may be a while because he's working on several items. I posted it here under bugs because that is where people will look if something stops working, rather than patches which is for new features. If you think my code is buggy then do not use it. I just want to warn you that I also wrote the code that allows you to specify the number of glyphs to allocate for skin fonts, so you probably don't want to risk using that, either.

The issue wasn't necessarily "trusting" your coding ability; I'm sure your glyph code has been around awhile and is well-tested, but the same can't be said yet for your buflib-font code. Beyond that, I think I'll let it go; after being rebuffed (for good reason) on committing the original patch, I dug into the WPS code and fixed the problem on my end without patching, so I didn't wanna get into more patches. Obviously, we agree that JD's working on the final be-all, end-all fix to this problem.

This bug report will be closed as soon as  FS#12273  is submitted. I am just keeping this open to avoid new bug reports in this area.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing