From: Jonathan Gordon <jdgordy_at_gmail.com>
Date: Tue, 25 Oct 2011 18:36:05 +1100

On 25 October 2011 17:35, Thomas Martitz <kugel_at_rockbox.org> wrote:
> Plugins can use the screen_access api. Ideally those should be using a
> custom viewport if they want a custom font, right?
>
Any plugins which are not using the screen acess api are probably so
old that yes, they dont use a custom font anyway (and wont be modified
to any time soon)

> One question about the patch though: Why separate setfont() and setuifont()?
> I imagine one could be sufficient, but perhaps I'm missing something.
>
> Best regards.
>

There are a few issues here:
The biggest issue is that the lcd API is full of 10 years of
evolution. First the multiscreen api came in, and then viewports, both
adding different bits of cruft. (not necessarily bad cruft). lcd_*
functions *mostly* work on the current viewport (lcd_setfont()
included), and some work on a specific viewport, and others work on
the actual display as a whole.

lcd_setfont() changes the current viewports font number. Should this
be removed completly? The *only* time the caller doesnt have access to
the viewport struct is with the default viewport (which we are trying
to slowly get away from, and this should only happen in plugins now,
so no, we cant remove it just yet).

screen_access->setfont() is a wrapper for lcd_setfont() to make sure
FONT_UI gets set correctly. screen_access->setuifont() does not call
lcd_setfont() and only sets global_settings.font_ui[screen].
screen_acces->getuifont() just returns the
global_settings.font_ui[screen] value (and there is no lcd_getfont()).


> Might be worthwhile also to check if font_get(vp->font)->height calls can be
> replaced with screen->getcharheight(). I admit I forgot about that one
> before Björn mentioned it and used the former too.
>

getcharheight() is just a wrapper around
font_get(lcd_getfont())->height so it only works on the current
viewports current font, which isnt necessarily what the caller wants.
even if it is, we want get get rid of lcd_getfont() anyway so at a
minimum this wrapper needs replacing.
Received on 2011-10-25