Rockbox mail archiveSubject: Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export
From: Thomas Martitz <kugel_at_rockbox.org>
Date: Tue, 25 Oct 2011 10:49:24 +0200
Am 25.10.2011 09:36, schrieb Jonathan Gordon:
>> 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()).
Okay, I understand the difference. But why does there need to be one? In
your patch you call them both most of the time.
I understand setfont() sets the current (i.e. default vp) font, and
setuifont() sets the id which was previously constant (FONT_UI). FONT_UI
is the alias for the screen's default font which is in fact simply the
font of the default vp.
So, from my understanding you want to do both calls at the same time.
Unless there is a reason to separate these steps which I'm missing, I
suggest the behavior to be merged in setfont().
Received on 2011-10-25