Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12337 - r30773 breaks all skin fonts

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

Details

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
   Screenshot-Sansa e200.png (41.2 KiB)
   cabbie.diff (0.6 KiB) 
 cabbiev2-broken.wps |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

This task depends upon

Closed by  Thomas Martitz (kugel.)
Saturday, 29 October 2011, 17:16 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  r30850
Comment by Jonathan Gordon (jdgordon) - Tuesday, 18 October 2011, 01:42 GMT+2
this is f course what it is supposed to look like
   Screenshot-Sansa e200-1.png (26.2 KiB)
Comment by Andree Buschmann (Buschel) - Tuesday, 18 October 2011, 08:08 GMT+2
If I comment two lines in firmware/drivers/lcd_bitmap_common.c my WPS seems to be drawn as intended.
   draw_text_v01.patch (0.7 KiB) 
 firmware/drivers/lcd-bitmap-common.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Tuesday, 18 October 2011, 08:33 GMT+2
yeah, but that is equivilant to reverting the commit...
Comment by Michael Chicoine (mc2739) - Wednesday, 19 October 2011, 04:14 GMT+2
This still seems to be happening with r30800. Attached are screendumps showing good and bad wps.
   r30772-good.bmp (75.7 KiB)
   r30800-bad.bmp (75.7 KiB)
Comment by Michael Chicoine (mc2739) - Saturday, 22 October 2011, 18:39 GMT+2
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.
   12-to-19.bmp (75.7 KiB)
   19-to-12.bmp (75.7 KiB)
Comment by Michael Chicoine (mc2739) - Saturday, 22 October 2011, 19:33 GMT+2
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.
   dump 111022-123010.bmp (75.7 KiB)
Comment by Fred Bauer (freddyb) - Sunday, 23 October 2011, 09:11 GMT+2
Michael: Try this.
   font_wps_touch.patch (2 KiB) 
 apps/gui/skin_engine/skin_parser.c |    4 ++--
 apps/gui/skin_engine/skin_render.c |    9 ++++++++-
 apps/gui/list.c                    |    4 ++++
 3 files changed, 14 insertions(+), 3 deletions(-)

Comment by Thomas Martitz (kugel.) - Sunday, 23 October 2011, 13:06 GMT+2
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, 15:34 GMT+2
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()
   font_wps_touch.2.patch (1.4 KiB) 
 apps/gui/skin_engine/skin_parser.c |    4 ++--
 apps/gui/skin_engine/skin_render.c |    8 +++++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comment by Michael Chicoine (mc2739) - Sunday, 23 October 2011, 18:48 GMT+2
Fred,

The latest patch corrects the wps/sbs issue I was having
Comment by Fred Bauer (freddyb) - Sunday, 23 October 2011, 19:01 GMT+2
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, 06:16 GMT+2
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, 10:23 GMT+2
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, 19:12 GMT+2
This patch should fix the remaining wps/sbs issue. Try it, I'll play with another idea in the meantime.
   skin_parser.c.vc.diff (0.4 KiB) 
 b/apps/gui/skin_engine/skin_parser.c |    1 +
 1 file changed, 1 insertion(+)

Comment by Michael Chicoine (mc2739) - Saturday, 29 October 2011, 04:36 GMT+2
kugel.,

Thanks, wps/sbs look good with this patch

Loading...