search

mobius3 / kiwi

KiWi: Killer Widgets
zlib License
186 stars 19 forks source link

Font inclusion into init #19

Closed wasamasa closed 8 years ago

wasamasa commented 8 years ago

I've noticed now that it's crucial to set the GUI font with KW_SetFont after KW_Init as the application will otherwise segfault. I'm not sure whether this is a deliberate decision (to support the alternative usecase of setting fonts for each widget involving them or compositions not involving text) or an API mistake. If it's the latter, why not change KW_Init to require a KW_Font argument and remove all KW_SetFont calls from the examples?

mobius3 commented 8 years ago

It crashes because every text rendering widget needs a font - they should not crash but warn instead, or fail silently (probably the best course, imo). I do not want to make the font part of KW_Init because it is not required for all widgets, whereas the render driver is. I'll make them not fail and print errors if NDEBUG compiler flag is not set. Thoughts?

wasamasa commented 8 years ago

Hm. I agree that it's not mandatory to always use a font, what I'm suggesting here is that the number of cases that don't need one at all is rather small, for instance none of the examples work without at least one around.

I disagree with changing the strategy. It's better to crash and burn as early as possible. As for logging, I'm still of the opinion that a library shouldn't print stuff unless I ask it to, a NDEBUG flag is pretty much the opposite of that as it requires me to take extra action for getting there. It would be more C-like to change the widget creation functions instead to signal an error, then let users check for them and exit early. Or ignore them and crash early.

mobius3 commented 8 years ago

Crash and burn is exactly what it is doing now, but the way it is now its just hard to understand why - one would need to look into the library to understand whats wrong, eventually users would need to debug the library to see who's wrong - user code or library code. That is not a good behaviour in my opinion.

NDEBUG (or maybe another "do-not-debug" flag) is only set (or should be) if cmake is not executed with -DCMAKE_BUILD_TYPE=Debug, so release versions would be safe against that. Maybe we should assert this kind of stuff in the debug library.

Maybe the GUI should not have a font at all, as you said, maybe those widgets that render text should require a KW_Font argument. This, together with asserts() and NDEBUG, seems like good library behaviour for me.

mobius3 commented 8 years ago

What if KiWi had a default, embedded font, and tileset?

mobius3 commented 8 years ago

So, I've created a resource generator for KiWi and used it to embed the sourcesans font. If the user does not set a font, KiWi is going to return it as a default font in KW_GetFont(). I'll consider this issue fixed, please reopen if you have any more considerations regarding this.

wasamasa commented 8 years ago

Thanks, that works for me. The only real alternative to defaulting to a bundled font is detecting existing system fonts, then picking a suitable one, but involves significantly more code, so I'm fine with this.