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().

Best regards.

Reply via email to