Use modern X fonts instead of X core fonts #38
Conversation
| | Some pc -> | ||
| let packages = [("x11","-lX11"); ("xft", "-lXft")] in | ||
| let cflags, libs = | ||
| packages | ||
| |> List.map (fun (package, fallback) -> | ||
| match C.Pkg_config.query pc ~package with | ||
| | None -> [], [fallback] | ||
| | Some { cflags; libs } -> cflags, libs) | ||
| |> List.split | ||
| in | ||
| List.concat cflags, List.concat libs |
There was a problem hiding this comment.
Ouch. pkg-config handles multiple package names just fine, e.g. pkg-config --libs xft x11 gives -lXft -lX11, no need for multiple invocations. But of cours Dune configurator doesn't support this.
The whole discover.ml file is an embarrassment. It would be 3 times shorter as a shell script.
Also: it would be prudent to link -lXft before -lX11, as the former certainly uses symbols from the latter.
| XftDrawString8 (d, &xftcol, caml_gr_font, | ||
| caml_gr_x, Bcvt(caml_gr_y) - caml_gr_font->descent + 1, | ||
| (const FcChar8 *) txt, len); | ||
| } |
There was a problem hiding this comment.
Missing XftDrawDestroy(d); here?
| visual, caml_gr_colormap); | ||
| XftDrawString8 (d, &xftcol, caml_gr_font, | ||
| caml_gr_x, Wcvt(caml_gr_y) - caml_gr_font->descent + 1, | ||
| (const FcChar8 *) txt, len); |
There was a problem hiding this comment.
Missing XftDrawDestroy(d); here?
| XFlush(caml_gr_display); | ||
| } | ||
| caml_gr_x += XTextWidth(caml_gr_font, txt, len); | ||
|
|
||
| if (d) XftDrawDestroy (d); |
There was a problem hiding this comment.
This destroys one XftDraw object but two may have been created. Also, d is not initialized, if I'm not mistaken, so the if (d) test is weird.
This is an actualization of the patch proposed in #3 (first issued at ocaml/ocaml#4917), so all credit goes to the original author of the patch (Richard Jones).
I've adapted the machinery to link against the xft library in
discover.ml, though I have no clue if this is the right way to do it (there may be some other things to tweak for the darwin target).