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

2013-12-10 Thread Lennart Poettering
On Thu, 28.11.13 09:32, David Herrmann (dh.herrm...@gmail.com) wrote:

  +sd_gfx_font *font;
  +int r;
  +
  +ppi = CLAMP(ppi, 10U, 1000U);
  +
  +font = calloc(1, sizeof(*font));
 
  font = new0(sd_gfx_font, 1);
 
 I actually dislike new0() to require the type. I'd prefer:
   font = new0(font);
 But this is horrible to read, so I went with calloc(1, sizeof(*xyz));
 But the systemd code-base seems to use new0() consistently so I can
 adjust all my calloc()s if you want? I'm fine with keeping consistency
 here.

I'd really prefer that. Not that new() is a glibc thing, where they have
g_new(). Of all the type-safe macros for allocation I certainly like it
the best...

  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).
 
 I wanna avoid these in fast-paths (the blending/blitting functions)
 but for all the other ones, I can add assert()s.

Yes, please do not add assert()s in inner loops!

I mean, the whole assert() story is not that definitive either. I add
them to a lot of functions, but I can totally understand when people
think I am adding to many of them

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-10 Thread Lennart Poettering
On Sun, 01.12.13 06:28, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 
 On Wed, Nov 27, 2013 at 07:48:40PM +0100, David Herrmann wrote:
  As a first step, we add the required header+build-chain and add the
  font-handling. To avoid heavy font-pipelines in systemd, we only provide
  a statically-sized fallback-font based on GNU-Unifont.
 Hi David,
 I don't think that GNU-Unifont is licensed in a way that allows it to
 be embedded in systemd. Systemd is LGPLv2+, while Unifont is GPLv2+ +
 FontException.

I don't think this is really a problem, is it? LGPL2+ is after all
compatible wit GPL2+. When you link stuff that is LGPL2+ with stuff that
is GPL2+ you effectively just downgrade LGPL2+ to GPL2+, and get a
result that effectively should be treated as GPL2+ altogether.

Now, what matters for us is that all our APIs and our sources stay
LGPL2+, so that we can freely copy/paste code around and library users
can use the stuff with little restrictions. However, if systemd-consoled
at runtime is downgraded to GPL2+, then that should not really affect
anybody... I mean, if somebody wanted to link non-free code into
consoled, then we'd have a problem, but nobody does that and consoled is
not a program where that's even something to think off.

I'd be fine to do with unifont what we do with the pci/usb-db, and ship
it in the sources, and provide an easy way to update it from upstream
(via a make target).

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-10 Thread Lennart Poettering
On Sun, 01.12.13 10:05, David Herrmann (dh.herrm...@gmail.com) wrote:

 I'm fine with installing the file into the system, but I doubt we win
 much. It's meant as fallback for early-boot, initrd and so on. If we
 keep it separate, we must make sure to include it in any systems we
 build (initrd, containers, vms, ..). So if there's no reason beside
 license issues, I'd like to keep it built-in.
 
  So if it is acceptable for systemd-gfx *binary* to be GPLv2+ licensed,
  we could use the system unifont.hex file at build time, and actually
  link it into the binary. I propose that we try to go this way.
 
 That's what I currently do.
 
  Or we could have the package also contain the converted font in appropriate
  format, and mmap it at runtime. But this is more complex, and doesn't 
  actually
  avoid the licensing issue, since the font would still be GPLv2+.
 
 Where is the difference between build-time linking and mmap()?
 (regarding licensing)
 Also, where's the point of keeping libsystemd-gfx.so LGPL just to have
 a *mandatory* dependency which is GPL?

I think embedding the thing into our binary in question is the best
choice here. I don't see any license problems, and it's certainly the
fastest and most robust thing to do...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-10 Thread Kay Sievers
On Wed, Dec 11, 2013 at 1:56 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Sun, 01.12.13 10:05, David Herrmann (dh.herrm...@gmail.com) wrote:

 I'm fine with installing the file into the system, but I doubt we win
 much. It's meant as fallback for early-boot, initrd and so on. If we
 keep it separate, we must make sure to include it in any systems we
 build (initrd, containers, vms, ..). So if there's no reason beside
 license issues, I'd like to keep it built-in.

  So if it is acceptable for systemd-gfx *binary* to be GPLv2+ licensed,
  we could use the system unifont.hex file at build time, and actually
  link it into the binary. I propose that we try to go this way.

 That's what I currently do.

  Or we could have the package also contain the converted font in appropriate
  format, and mmap it at runtime. But this is more complex, and doesn't 
  actually
  avoid the licensing issue, since the font would still be GPLv2+.

 Where is the difference between build-time linking and mmap()?
 (regarding licensing)
 Also, where's the point of keeping libsystemd-gfx.so LGPL just to have
 a *mandatory* dependency which is GPL?

 I think embedding the thing into our binary in question is the best
 choice here. I don't see any license problems, and it's certainly the
 fastest and most robust thing to do...

It is huge, ~2MB. We need to be sure that we will never have 2
different processes carrying it, it would be a waste of memory.

Storing it on disk and mmap()ing it from multiple different binaries
would be a fine alternative then, I think.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-10 Thread Lennart Poettering
On Wed, 11.12.13 02:14, Kay Sievers (k...@vrfy.org) wrote:

 
 On Wed, Dec 11, 2013 at 1:56 AM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Sun, 01.12.13 10:05, David Herrmann (dh.herrm...@gmail.com) wrote:
 
  I'm fine with installing the file into the system, but I doubt we win
  much. It's meant as fallback for early-boot, initrd and so on. If we
  keep it separate, we must make sure to include it in any systems we
  build (initrd, containers, vms, ..). So if there's no reason beside
  license issues, I'd like to keep it built-in.
 
   So if it is acceptable for systemd-gfx *binary* to be GPLv2+ licensed,
   we could use the system unifont.hex file at build time, and actually
   link it into the binary. I propose that we try to go this way.
 
  That's what I currently do.
 
   Or we could have the package also contain the converted font in 
   appropriate
   format, and mmap it at runtime. But this is more complex, and doesn't 
   actually
   avoid the licensing issue, since the font would still be GPLv2+.
 
  Where is the difference between build-time linking and mmap()?
  (regarding licensing)
  Also, where's the point of keeping libsystemd-gfx.so LGPL just to have
  a *mandatory* dependency which is GPL?
 
  I think embedding the thing into our binary in question is the best
  choice here. I don't see any license problems, and it's certainly the
  fastest and most robust thing to do...
 
 It is huge, ~2MB. We need to be sure that we will never have 2
 different processes carrying it, it would be a waste of memory.

That's a very good point. Given that we link everything in systemd
statically so far, then this is a strong point to store in an external
mmapable binary file if this is used by more than one process...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-02 Thread David Herrmann
Hi

 The binary file contains all glyphs in a compressed 1-bit-per-pixel format
 for all 8x16 and 16x16 glyphs. It is linked directly into libsystemd-gfx
 via the *.bin makefile target. Binary size is 2.1MB, but thanks to paging,
 the kernel only loads required pages into memory. A ASCII-only screen thus
 only needs 40k VIRT mem.
 Do we have quad-glyphs (16x32) for the ASCII part of this for hi-res screens?

 (also, if we use NFD we could reuse these for some of unicode[1], but
 that is less important)

 [1]http://blog.golang.org/normalization

See here:

+static void gfx_glyph_blend(gfx_glyph *g, unsigned int ppi, const
struct unifont_data *u) {
+unsigned int i, j;
+const uint8_t *src;
+uint8_t *dst;
+
+/*
+ * TODO: scale glyph according to @ppi
+ */

Idea is to simply scale the glyph by an integer (sth like max(1, ppi /
72)). This will result in a readable font-size on all screens. Note
that we don't care for hi-res fonts, we're not EFI-firmware..

Regarding NFD: I don't care for normalization. I don't do any
string-comparisons. We support combining characters in the
font-renderer just fine, so I don't think we ever need unicode
normalization.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-12-01 Thread David Herrmann
Hi

On Sun, Dec 1, 2013 at 6:28 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, Nov 27, 2013 at 07:48:40PM +0100, David Herrmann wrote:
 As a first step, we add the required header+build-chain and add the
 font-handling. To avoid heavy font-pipelines in systemd, we only provide
 a statically-sized fallback-font based on GNU-Unifont.
 Hi David,
 I don't think that GNU-Unifont is licensed in a way that allows it to
 be embedded in systemd. Systemd is LGPLv2+, while Unifont is GPLv2+ + 
 FontException.
 FontException allows embedding in documents, so it doesn't apply.

I disagree. I'm allowed to embed GNU-Unifont in a pdf/postscript file,
right? However, postscript is as turing-complete as x86-assembler, so
I don't see the difference between an ELF-document and a
postscript-document.

 It would be possible have some sources which are GPLv2+ only, but I
 think we want to avoid such complications.

It's not about sources. Assuming the font-exception doesn't apply,
this only means all binaries linking to libsystemd-gfx are GPLv2. The
sources stay LGPL as usual.

 Also, if the font was embedded in systemd, distributions would then
 remove it in order to replace is with the system version. So I think
 that including the font sources is pointless... Debian has it packaged [1],
 but an old version, I'm not sure if there have been recent updates, and
 possibly in the wrong format. Fedora doesn't seem to have it yet.
 But adding fonts is easy, I'd do the Fedora package myself, and other
 distributions could surely add/update it.

I'm fine with installing the file into the system, but I doubt we win
much. It's meant as fallback for early-boot, initrd and so on. If we
keep it separate, we must make sure to include it in any systems we
build (initrd, containers, vms, ..). So if there's no reason beside
license issues, I'd like to keep it built-in.

 So if it is acceptable for systemd-gfx *binary* to be GPLv2+ licensed,
 we could use the system unifont.hex file at build time, and actually
 link it into the binary. I propose that we try to go this way.

That's what I currently do.

 Or we could have the package also contain the converted font in appropriate
 format, and mmap it at runtime. But this is more complex, and doesn't actually
 avoid the licensing issue, since the font would still be GPLv2+.

Where is the difference between build-time linking and mmap()?
(regarding licensing)
Also, where's the point of keeping libsystemd-gfx.so LGPL just to have
a *mandatory* dependency which is GPL?

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-30 Thread Shawn Landden
On Sat, Nov 30, 2013 at 9:28 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, Nov 27, 2013 at 07:48:40PM +0100, David Herrmann wrote:
 As a first step, we add the required header+build-chain and add the
 font-handling. To avoid heavy font-pipelines in systemd, we only provide
 a statically-sized fallback-font based on GNU-Unifont.
 Hi David,
 I don't think that GNU-Unifont is licensed in a way that allows it to
 be embedded in systemd. Systemd is LGPLv2+, while Unifont is GPLv2+ + 
 FontException.
 FontException allows embedding in documents, so it doesn't apply.
 It would be possible have some sources which are GPLv2+ only, but I
 think we want to avoid such complications.

 Also, if the font was embedded in systemd, distributions would then
 remove it in order to replace is with the system version. So I think
 that including the font sources is pointless... Debian has it packaged [1],
 but an old version, I'm not sure if there have been recent updates, and
 possibly in the wrong format. Fedora doesn't seem to have it yet.
 But adding fonts is easy, I'd do the Fedora package myself, and other
 distributions could surely add/update it.

 So if it is acceptable for systemd-gfx *binary* to be GPLv2+ licensed,
 we could use the system unifont.hex file at build time, and actually
 link it into the binary. I propose that we try to go this way.

 Or we could have the package also contain the converted font in appropriate
 format, and mmap it at runtime. But this is more complex, and doesn't actually
 avoid the licensing issue, since the font would still be GPLv2+.
The font is needed in the initramfs

 Zbyszek

 [1] http://packages.debian.org/sid/all/unifont/filelist
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2013-11-28 Thread David Herrmann
Hi

On Wed, Nov 27, 2013 at 11:11 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@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...

Yepp, sounds good. Would be like 200 lines to write that, I guess.

 +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 ;-)

Do I get an award or something? :)

 +
 +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);

I actually dislike new0() to require the type. I'd prefer:
  font = new0(font);
But this is horrible to read, so I went with calloc(1, sizeof(*xyz));
But the systemd code-base seems to use new0() consistently so I can
adjust all my calloc()s if you want? I'm fine with keeping consistency
here.

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

I wanna avoid these in fast-paths (the blending/blitting functions)
but for all the other ones, I can add assert()s.

 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.

Oh, I know. This is presumably the only destructor where I somehow
forgot that. Fixed.

 +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...


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

2013-11-27 Thread David Herrmann
If we want to interact with a user in the initrd, during
emergency-situations, in single-user mode, or in any other rather limited
situation, we currently rely on the kernel to do input and graphics access
for us. More precisely, we rely on the VT layer to do keyboard parsing and
font rendering. Or in other words: The *kernel* provides our UI!

This is bad for the same reasons we don't put any other UI into the
kernel. However, there are a few reasons to keep a minimal UI in the
kernel:
 1) show panic-screen with kernel oops message
 2) show log-screen during early boot
 3) allow kernel-debugging via kdb
While people using kdb are encouraged to keep VTs, for 1) and 2) there is
a replacement via fblog/drmlog.

So we run out of reasons to keep a UI in the kernel. However, to allow
moving the UI handling to userspace, we need a bunch of helpers:
 - keyboard handling: convert keycodes into keysyms and modifiers/..
 - modesetting: program the gfx pipeline and provide a rendering
infrastructure
 - fonts: to render text, we need some basic fonts
 - hotplugging: device detection and assignment during runtime
(Note that all these are implemented (often quite rudimentary) in the
kernel to allow a basic UI.)

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

As a first step, we add the required header+build-chain and add the
font-handling. To avoid heavy font-pipelines in systemd, we only provide
a statically-sized fallback-font based on GNU-Unifont. It contains glyphs
for the *whole* Base-Multilingual-Plane of Unicode and thus allows
internationalized text.

The make-unifont.py script is used by the make update-unifont custom
target to regenerate the unifont files. As this can take quite some time,
we check the result into git so only maintainers need to run this.

The binary file contains all glyphs in a compressed 1-bit-per-pixel format
for all 8x16 and 16x16 glyphs. It is linked directly into libsystemd-gfx
via the *.bin makefile target. Binary size is 2.1MB, but thanks to paging,
the kernel only loads required pages into memory. A ASCII-only screen thus
only needs 40k VIRT mem.
---
Hi

I removed the unifont-data from this patch so this will not apply cleanly.
However, the ML would reject the huge patch otherwise. If anyone is interested
in the raw patch, the series is available on:
  http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=console

Thanks
David

 Makefile.am  |31 +
 configure.ac |10 +
 make-unifont.py  |   138 +
 src/libsystemd-gfx/.gitignore| 1 +
 src/libsystemd-gfx/Makefile  | 1 +
 src/libsystemd-gfx/gfx-unifont.c |   273 +
 src/libsystemd-gfx/unifont.bin   |   Bin 0 - 2162688 bytes
 src/libsystemd-gfx/unifont.hex   | 63488 +
 src/systemd/sd-gfx.h |65 +
 9 files changed, 64007 insertions(+)
 create mode 100755 make-unifont.py
 create mode 100644 src/libsystemd-gfx/.gitignore
 create mode 12 src/libsystemd-gfx/Makefile
 create mode 100644 src/libsystemd-gfx/gfx-unifont.c
 create mode 100644 src/libsystemd-gfx/unifont.bin
 create mode 100644 src/libsystemd-gfx/unifont.hex
 create mode 100644 src/systemd/sd-gfx.h

diff --git a/Makefile.am b/Makefile.am
index ce27e82..2edb091 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3853,6 +3853,37 @@ EXTRA_DIST += \
 endif
 
 # 
--
+if HAVE_GFX
+noinst_LTLIBRARIES += \
+   libsystemd-gfx.la
+
+libsystemd_gfx_la_SOURCES = \
+   src/libsystemd-gfx/sd-gfx.h \
+   src/libsystemd-gfx/gfx-unifont.c
+
+libsystemd_gfx_la_CFLAGS = \
+   $(AM_CFLAGS)
+
+libsystemd_gfx_la_LIBADD = \
+   libsystemd-shared.la \
+   src/libsystemd-gfx/unifont.bin.lo
+
+src/libsystemd-gfx/unifont.bin: make-unifont.py src/libsystemd-gfx/unifont.hex
+   $(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.hex | $(PYTHON) 
$ $@
+
+src/libsystemd-gfx/unifont.cmp: make-unifont.py src/libsystemd-gfx/unifont.bin
+   $(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.bin | $(PYTHON) 
$ verify $@
+
+update-unifont: src/libsystemd-gfx/unifont.bin src/libsystemd-gfx/unifont.cmp
+   @RET=`diff -u src/libsystemd-gfx/unifont.hex 
src/libsystemd-gfx/unifont.cmp | wc -l` ; \
+   if test x$$? != x0 -o x$$RET != x0 ; then \
+   echo Generated Unifont-file differs from original; 

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

2013-11-27 Thread Lennart Poettering
On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel