FS#12337 - r30773 breaks all skin fonts

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Monday, 17 October 2011, 23:25 GMT
Last edited by Thomas Martitz (kugel.) - Saturday, 29 October 2011, 15:16 GMT
Task Type Bugs
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity High
Priority Urgent
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Adding line height to viewports breaks all skin fonts because their viewports are initialised and the font changed later.

The attached screenshot shows the problem nicely. the only change to cabbiev2.wps there is attached as a diff
This task depends upon

Closed by  Thomas Martitz (kugel.)
Saturday, 29 October 2011, 15:16 GMT
Reason for closing:  Fixed
Additional comments about closing:  r30850
Comment by Jonathan Gordon (jdgordon) - Monday, 17 October 2011, 23:42 GMT
this is f course what it is supposed to look like
Comment by Andree Buschmann (Buschel) - Tuesday, 18 October 2011, 06:08 GMT
If I comment two lines in firmware/drivers/lcd_bitmap_common.c my WPS seems to be drawn as intended.
Comment by Jonathan Gordon (jdgordon) - Tuesday, 18 October 2011, 06:33 GMT
yeah, but that is equivilant to reverting the commit...
Comment by Michael Chicoine (mc2739) - Wednesday, 19 October 2011, 02:14 GMT
This still seems to be happening with r30800. Attached are screendumps showing good and bad wps.
Comment by Michael Chicoine (mc2739) - Saturday, 22 October 2011, 16:39 GMT
This is much worse than a wps problem.

The attached screendumps are from e200 sim. The procedure to reproduce is:

1. Start sim (fresh install)
2. Go to Settings->Theme Settings->Font and select a different font.

The screendumps show a switch from 12-Adobe-Helvetica.fnt to 19-Nimbus.fnt and, after restart, a switch from 19-Nimbus.fnt to 12-Adobe-Helvetica.fnt

The line height is not adjusted to match the font. Reloading the theme or restarting the device will correct the line height.
Comment by Michael Chicoine (mc2739) - Saturday, 22 October 2011, 17:33 GMT
r30826 fixes the problem with switching fonts, but the problem in my previous comment still exists.

If it makes a difference, the font used in that them is the sysfont (font 0). When I switch to the menu screen, The sbs displays the artist and song title, also in sysfont, and it also displays the same symptom.
Comment by Fred Bauer (freddyb) - Sunday, 23 October 2011, 07:11 GMT
Michael: Try this.
Comment by Thomas Martitz (kugel.) - Sunday, 23 October 2011, 11:06 GMT
Should be fixed in SVN.

EDIT: I mean the font switching issue, I havent looked at the other sbs one yet.
Comment by Fred Bauer (freddyb) - Sunday, 23 October 2011, 13:34 GMT
Here's an updated version to fix the WPS issue. It uses font_get_ui() to determine FONT_UI instead of global_status.font_id[] and updates the viewport height in skin_render_viewport()
Comment by Michael Chicoine (mc2739) - Sunday, 23 October 2011, 16:48 GMT

The latest patch corrects the wps/sbs issue I was having
Comment by Fred Bauer (freddyb) - Sunday, 23 October 2011, 17:01 GMT
The list logic depends on skinlist_is_configured() which is always false for every theme I throw at it. The above patch has a problem for viewports on touchscreens because there is no functioning logic to determine which lists to pad.
Comment by Jonathan Gordon (jdgordon) - Monday, 24 October 2011, 04:16 GMT
Fred: can you come on irc some time? (or did you see my email to the dev ml regarding your last commit?)
Comment by Thomas Martitz (kugel.) - Thursday, 27 October 2011, 08:23 GMT
For the sbs/wps issue:

I think a similar fix as r30786 should be applied to the SYSFONT case (skin_parser.c:1656 and following). I don't think rendering needs to be touched.
Comment by Thomas Martitz (kugel.) - Friday, 28 October 2011, 17:12 GMT
This patch should fix the remaining wps/sbs issue. Try it, I'll play with another idea in the meantime.
Comment by Michael Chicoine (mc2739) - Saturday, 29 October 2011, 02:36 GMT

Thanks, wps/sbs look good with this patch