[systemd-devel] [RFC 05/12] gfx: add sd-gfx library with unifont section

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrmann at gmail.com) wrote:

> This patch introduces sd-gfx, a systemd-internal library dealing with all
> these things. Note that it is designed to be exported some day, but for
> now we keep it internal.
> While the library itself will be kept small and almost self-contained, it
> is designed to be extensible and can be used in rather complex graphics
> applications. For systemd, we plan to add a basic emergency-console, an
> initrd password-query, kernel-log-screen and a fallback login-screen.
> These allow booting without CONFIG_VT and provide a basic system for
> seats without VTs (either CONFIT_VT=n or seats != seat0).

BTW, one more use of this I'd like to see: if we boot-up and detect that
no AC and very low battery we should show a pretty screen and block the
boot until AC is plugged in or the user hits some magic key...

> +static int gfx_glyph_render(gfx_glyph *g, unsigned int ppi, const struct unifont_data *u) {
> +        g->buf.width = u->cells * unifont_width;
> +        g->buf.height = unifont_height;
> +        g->buf.stride = u->cells * unifont_stride;
> +        g->buf.format = SD_GFX_BUFFER_FORMAT_A1;
> +
> +        g->buf.data = calloc(unifont_height, unifont_max_cells * unifont_stride);
> +        if (!g->buf.data)
> +                return -ENOMEM;

Nice, this must be the first use of alloc() I have ever seen that
actually takes benefit of the weird parameters it takes ;-)

> +
> +        gfx_glyph_blend(g, ppi, u);
> +        return 0;
> +}
> +
> +int sd_gfx_font_new(sd_gfx_font **out, unsigned int ppi) {

Hmm, so far we followed the rough rule that parameters we pass out are
listed last on the function parameter list. 

int sd_gfx_font_new(unsigned ppi, sd_gfx_fond **out);



> +        sd_gfx_font *font;
> +        int r;
> +
> +        ppi = CLAMP(ppi, 10U, 1000U);
> +
> +        font = calloc(1, sizeof(*font));

font = new0(sd_gfx_font, 1);

> +        font->glyphs = hashmap_new(trivial_hash_func, trivial_compare_func);
> +        if (!font->glyphs) {
> +                free(font);
> +                return log_oom();

Hmm, "library" code should never log. Please just return -ENOMEM
here. log_oom() should be used in main programs only really...

> +void sd_gfx_font_free(sd_gfx_font *font) {
> +        gfx_glyph *g;

For public APIs we are pretty defensive and check all parameters we
take. Given you kinda made this a public library it might make sense to
follow that rule here, too. assert_return() is particularly useful for
this. (Well, not for the instance above, bug if you return a negative
errno it is super-useful).

Most destructors we have tend to accept NULL pointers too. It makes
clean-up code a lot easier (especially in combination with gcc cleanup
attributes). libc free() takes NULL pointers too, so this is a natural
extension.


> +void sd_gfx_font_render(sd_gfx_font *font, uint32_t id, const uint32_t *ucs4, size_t len, sd_gfx_buffer **out) {
> +        const struct unifont_data *u;
> +        gfx_glyph *g;
> +
> +        if (!len)
> +                goto error;
> +
> +        if (!id)
> +                id = *ucs4;
> +
> +        g = hashmap_get(font->glyphs, INT_TO_PTR(id));
> +        if (g)
> +                goto out;
> +
> +        g = calloc(1, sizeof(*g));

g = new(gfx_glyph, 1);

> +        if (!g) {
> +                log_oom();
> +                goto error;
> +        }
> +
> +        u = unifont_get(*ucs4);
> +        if (gfx_glyph_render(g, font->ppi, u) < 0) {
> +                log_oom();
> +                free(g);
> +                goto error;
> +        }
> +
> +        while (--len) {
> +                u = unifont_get(*++ucs4);
> +                gfx_glyph_blend(g, font->ppi, u);
> +        }
> +
> +        if (hashmap_put(font->glyphs, INT_TO_PTR(id), g) < 0) {
> +                log_oom();
> +                free(g->buf.data);
> +                free(g);
> +                goto error;
> +        }
> +
> +        goto out;
> +
> +error:
> +        g = &font->fallback;
> +out:
> +        *out = &g->buf;
> +}

Hmm, we so far followed to rule to not clobber return parameters on
failure. i.e. we try hard to fill any return values in until we know
that nothing can fail anymore. This makes the contract for the caller
much easier who can refrain from freeing/resetting any parameters if a
call failed...

Lennart

-- 
Lennart Poettering, Red Hat