Bug 205707 - Improve parsing of BDFs by vtfontcvt
Summary: Improve parsing of BDFs by vtfontcvt
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.2-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Ed Maste
URL:
Keywords: patch, vt
: 228288 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-12-30 00:12 UTC by Mikhail Teterin
Modified: 2019-06-19 13:33 UTC (History)
4 users (show)

See Also:


Attachments
Allow BDF-characters to have less than 16 rows (1.63 KB, patch)
2015-12-30 00:12 UTC, Mikhail Teterin
no flags Details | Diff
Allow BDF-characters to appear out of order (855 bytes, patch)
2015-12-30 00:17 UTC, Mikhail Teterin
no flags Details | Diff
read character size from BBX entry (830 bytes, patch)
2016-03-28 14:13 UTC, Ed Maste
no flags Details | Diff
read FONTBOUNDINGBOX and BBX, handle other DWIDTH (2.38 KB, patch)
2017-03-18 20:35 UTC, Henry Hu
no flags Details | Diff
fix x-position handling (2.39 KB, patch)
2017-03-18 22:25 UTC, Henry Hu
no flags Details | Diff
More complex patch (16.98 KB, patch)
2018-06-27 09:02 UTC, Dmitry Wagin
no flags Details | Diff
More complex patch (16.29 KB, patch)
2018-12-02 20:36 UTC, Dmitry Wagin
no flags Details | Diff
cava.bdf (32.55 KB, text/plain)
2018-12-02 21:09 UTC, Tobias Kortkamp
no flags Details
vtfontcvt.c.diff (391 bytes, patch)
2019-06-16 13:04 UTC, Dmitry Wagin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Teterin freebsd_committer 2015-12-30 00:12:56 UTC
Created attachment 164832 [details]
Allow BDF-characters to have less than 16 rows

The current implementation is somewhat naive in assuming, the bitmaps will always contain exactly 16 rows. The assumption is not verified resulting in cryptic failure messages trying to parse a 8x13 font, for example.

The attached patch fixes this -- it parses the hex rows until it encounters the ENDCHAR-keyword or reads the 16 rows (the hard-coded constant currently in place).

This allows me to convert the KOI8-U font (koi8x13.bdf) from the x11-fonts/geminifonts port. Not that the result is very useful -- presumably, because non-UTF locales aren't supported yet.
Comment 1 Mikhail Teterin freebsd_committer 2015-12-30 00:17:35 UTC
Created attachment 164833 [details]
Allow BDF-characters to appear out of order

I made these changes locally, because the utility kept rejecting my font with the "Bad ordering" error. Although it turned out, the error was bogus and due simply to the misparsing addressed by the other patch, these hunks may still be useful in case a BDF with misordered characters is encountered by someone else.
Comment 2 Ed Maste freebsd_committer 2015-12-30 02:25:11 UTC
> The current implementation is somewhat naive in assuming, the bitmaps will always contain exactly 16 rows. The assumption is not verified resulting in cryptic failure messages trying to parse a 8x13 font, for example.

Note that you can specify the height and width with -h and -w, but I agree that it's a bit silly to require it to be specified, and we should provide a useful error message if parsing fails.
Comment 3 Mikhail Teterin freebsd_committer 2015-12-30 06:33:53 UTC
(In reply to Ed Maste from comment #2)
> we should provide a useful error message if parsing fails.

Well, my changes make the function accept arbitrary number of rows UP TO the height. I don't know, if BDF-format allows different characters in the same file to have different heights, but with my patch the utility will be better prepared to handle such cases...
Comment 4 Ed Maste freebsd_committer 2016-03-28 14:13:07 UTC
Created attachment 168728 [details]
read character size from BBX entry
Comment 5 Ed Maste freebsd_committer 2016-03-28 14:18:39 UTC
(In reply to Mikhail Teterin from comment #3)

> I don't know, if BDF-format allows different characters in the same file to
> have different heights, but with my patch the utility will be better prepared
> to handle such cases...

I agree that a cryptic error message is unacceptable.

However, in the case of just stopping upon ENDCHAR the result with a 13 row .bdf will still be a 16x8 .fnt font that has the final 3 rows empty. This is probably not what is desired.

I've attached a patch that might be part of the solution - reading the height and width from the BBX entry in the font.

It doesn't fully work with koi8x13.bdf because most glyphs are 13x8 but there is one 9x6 and one 13x9.
Comment 6 Henry Hu 2017-03-18 20:35:05 UTC
Created attachment 180939 [details]
read FONTBOUNDINGBOX and BBX, handle other DWIDTH

I tried to convert fonts in x11-fonts/wqy (an open source Chinese bitmap font) for use with vt. The original fonts are in pcf, I used https://github.com/ganaware/pcf2bdf to convert them to bdf.

I found that the original assumptions in the code have several problems:
* Assuming DWIDTH = width or width * 2
Actually there can be varying widths. For wqy, there are DWIDTH between 2-13 for the 10pt font, and 2-12 for the 9pt font.
* Assuming number of lines in the bitmap == height
Actually it usually equals to the height in the BBX line.

This patch fixes the problems by:
* Read the FONTBOUNDINGBOX line.
This gives us the baseline position. If offset_y in this line is -3, we need to reserve 3 lines at the bottom.
* Read the BBX line.
The offset_x and offset_y information in this line tells us where to position the glyph. For example, if offset_y == -2, the font should be positioned 2 lines below the baseline.
* Position the glyph at the correct place.
The starting line is calculated by:
    line_y = height + font_y - bb_h - bb_y + i;
The font's baseline (bottom) is height + font_y, the glyph's baseline is height + font_y - bb_y.

The font generated by this patch is used successfully with the wqy font on the vt terminal to display chinese characters. There seems to be problems for alphabet characters which are double-width (the 'W' in wqy font), so the working configuration requires a width of at least 10 pixels.
Comment 7 Henry Hu 2017-03-18 22:25:06 UTC
Created attachment 180943 [details]
fix x-position handling

The previous patch has some problems in deciding the x-position of the character.

The input line seems to have howmany(bb_w, 8) bytes. The bitmap is aligned to the left, and occupies the left bb_w bits.
The first shift moves the bitmap to the right, now it's right aligned to bit 0.
The parse_bitmap_line() function seems to assume that the glyph occupies the top dwidth bits. To put the character at bb_x position, the number of bits at right is howmany(dwidth, 8) * 8 - bb_x - bb_w.
Comment 8 Dmitry Wagin 2018-06-27 09:02:16 UTC
Created attachment 194668 [details]
More complex patch

Added support all font width, improve parsing of BDFs (FONT, FONTBOUNDINGBOX, BBX etc)
Comment 9 Dmitry Wagin 2018-06-27 09:02:53 UTC
*** Bug 228288 has been marked as a duplicate of this bug. ***
Comment 10 Dmitry Wagin 2018-11-17 06:37:21 UTC
(In reply to Dmitry Wagin from comment #8)
also, the patch added support for HiDPI displays
Comment 11 Tobias Kortkamp freebsd_committer 2018-12-02 10:58:09 UTC
In the x11-fonts/spleen port I'm converting all but one of the fonts
to vt(4) fonts.  This works pretty well, but converting the
spleen-32x64 [1] font fails with:

$ vtfontcvt -w 32 -h 64 spleen-32x64.bdf spleen-32x64.fnt
vtfontcvt: Unsupported wbytes 4!

While Dmitry's patch fixes the error message, all generated fonts are
now broken and do not display correctly.  Characters appear as if some
part of them are cut off.  Something is not quite right.

[1] https://github.com/fcambus/spleen/blob/1.0.2/spleen-32x64.bdf
Comment 12 Dmitry Wagin 2018-12-02 17:46:37 UTC
(In reply to Tobias Kortkamp from comment #11)

old patch version, I'll find the corrected
Comment 13 Dmitry Wagin 2018-12-02 20:36:05 UTC
Created attachment 199764 [details]
More complex patch

A corrected version of the patch, please test.
Comment 14 Tobias Kortkamp freebsd_committer 2018-12-02 20:48:56 UTC
(In reply to Dmitry Wagin from comment #13)
Thank you, Dmitry.  This seems to work nicely now!
Comment 15 Tobias Kortkamp freebsd_committer 2018-12-02 21:09:45 UTC
Created attachment 199765 [details]
cava.bdf

There seems to be a new problem when building audio/cava and trying to
convert cava.bdf which previously worked fine:

vtfontcvt: Font spacing "C" (Character cell) required

I'm attaching cava.bdf in case you want to take a look.
Comment 16 commit-hook freebsd_committer 2018-12-02 21:26:34 UTC
A commit references this bug:

Author: tobik
Date: Sun Dec  2 21:26:00 UTC 2018
New revision: 486487
URL: https://svnweb.freebsd.org/changeset/ports/486487

Log:
  x11-fonts/spleen: Update to 1.0.3

  - Provide spleen-32x64 for vt(4).  The vtfontcvt in supported FreeBSD
    versions cannot deal with it at the moment, so provide a pregenerated
    version of it for now.  This was made possible by the vtfontcvt
    feature patch from Dmitry Wagin (thanks!) [1].

  Changes:	https://github.com/fcambus/spleen/releases/tag/1.0.3
  PR:		205707 [1]

Changes:
  head/x11-fonts/spleen/Makefile
  head/x11-fonts/spleen/distinfo
  head/x11-fonts/spleen/pkg-plist
Comment 17 Dmitry Wagin 2018-12-02 21:41:40 UTC
(In reply to Tobias Kortkamp from comment #15)

incorrect font header, it just wasn't checked before.

https://en.wikipedia.org/wiki/Glyph_Bitmap_Distribution_Format
"FONT -gnu-unifont-medium-r-normal--16-160-75-75-c-80-iso10646-1" defines the font family and face names as an X logical font description. 

in cava.bdf:
FONT psf
Comment 18 commit-hook freebsd_committer 2018-12-03 11:14:08 UTC
A commit references this bug:

Author: tobik
Date: Mon Dec  3 11:13:41 UTC 2018
New revision: 486512
URL: https://svnweb.freebsd.org/changeset/ports/486512

Log:
  audio/cava: Add a dummy fontname to cava.bdf

  Without it, the BDF is malformed with FONT set to just "psf" which
  is not a proper XLFD.  This confuses newer vtfontcvt versions which
  check for character cell spacing.

  PR:		205707

Changes:
  head/audio/cava/Makefile
Comment 19 commit-hook freebsd_committer 2019-01-19 02:06:15 UTC
A commit references this bug:

Author: tobik
Date: Sat Jan 19 02:05:39 UTC 2019
New revision: 490695
URL: https://svnweb.freebsd.org/changeset/ports/490695

Log:
  New port: x11-fonts/vtfontcvt-ng

  An improved version of vtfontcvt(8) with support for all font widths
  and improved parsing of BDFs (FONT, FONTBOUNDINGBOX, BBX, etc.)
  based on patches from https://bugs.freebsd.org/205707.

  WWW: https://github.com/t6/vtfontcvt-ng

  PR:		205707

Changes:
  head/x11-fonts/Makefile
  head/x11-fonts/vtfontcvt-ng/
  head/x11-fonts/vtfontcvt-ng/Makefile
  head/x11-fonts/vtfontcvt-ng/distinfo
  head/x11-fonts/vtfontcvt-ng/pkg-descr
Comment 20 commit-hook freebsd_committer 2019-02-06 18:51:59 UTC
A commit references this bug:

Author: emaste
Date: Wed Feb  6 18:50:48 UTC 2019
New revision: 343842
URL: https://svnweb.freebsd.org/changeset/base/343842

Log:
  vtfontcvt: whitespace cleanup

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 21 commit-hook freebsd_committer 2019-06-04 18:27:03 UTC
A commit references this bug:

Author: emaste
Date: Tue Jun  4 18:26:42 UTC 2019
New revision: 348653
URL: https://svnweb.freebsd.org/changeset/base/348653

Log:
  vtfontcvt: use VFNT_MAP_{NORMAL|BOL}_RH symbolic constants

  PR:		205707
  Reported by:	Dmitry Wagin
  MFC after:	1 month

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 22 commit-hook freebsd_committer 2019-06-04 18:32:31 UTC
A commit references this bug:

Author: emaste
Date: Tue Jun  4 18:31:52 UTC 2019
New revision: 348656
URL: https://svnweb.freebsd.org/changeset/base/348656

Log:
  vtfontcvt: unwrap a line per style(9)

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 23 commit-hook freebsd_committer 2019-06-04 18:50:50 UTC
A commit references this bug:

Author: emaste
Date: Tue Jun  4 18:49:49 UTC 2019
New revision: 348661
URL: https://svnweb.freebsd.org/changeset/base/348661

Log:
  vtfontcvt: rework height/width setting

  Introduce VFNT_MAXDIMENSION to replace bare 128, add set_height, and
  consistently use set_height and set_width.

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 24 commit-hook freebsd_committer 2019-06-04 18:55:57 UTC
A commit references this bug:

Author: emaste
Date: Tue Jun  4 18:55:03 UTC 2019
New revision: 348662
URL: https://svnweb.freebsd.org/changeset/base/348662

Log:
  vtfontcvt: include width and height in verbose info

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 25 commit-hook freebsd_committer 2019-06-04 19:39:33 UTC
A commit references this bug:

Author: emaste
Date: Tue Jun  4 19:39:07 UTC 2019
New revision: 348668
URL: https://svnweb.freebsd.org/changeset/base/348668

Log:
  vtfontcvt: zero memory allocated by xmalloc

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 26 commit-hook freebsd_committer 2019-06-05 15:24:05 UTC
A commit references this bug:

Author: emaste
Date: Wed Jun  5 15:23:49 UTC 2019
New revision: 348692
URL: https://svnweb.freebsd.org/changeset/base/348692

Log:
  vtfontcvt: exit on error if the input font has too many glyphs

  The kernel has a limit of 131072 glyphs in a font; add the same check to
  vtfontcvt so that we won't create a font file that the kernel will not
  load.

  PR:		205707
  Submitted by:	Dmitry Wagin
  MFC after:	2 weeks

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 27 commit-hook freebsd_committer 2019-06-08 08:26:16 UTC
A commit references this bug:

Author: emaste
Date: Sat Jun  8 08:25:44 UTC 2019
New revision: 348796
URL: https://svnweb.freebsd.org/changeset/base/348796

Log:
  vtfontcvt: allow out-of-order glyphs

  PR:		205707
  Reported by:	mi
  MFC after:	2 weeks
  Event:		Vienna Hackathon 2019
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 28 commit-hook freebsd_committer 2019-06-16 09:17:58 UTC
A commit references this bug:

Author: emaste
Date: Sun Jun 16 09:17:26 UTC 2019
New revision: 349100
URL: https://svnweb.freebsd.org/changeset/base/349100

Log:
  vtfontcvt: improve BDF and hex font parsing

  Support larger font sizes.

  PR:		205707
  Submitted by:	Dmitry Wagin (original version)
  MFC after:	2 weeks
  Event:		Berlin Devsummit 2019
  Differential Revision:	https://reviews.freebsd.org/D20650

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 29 commit-hook freebsd_committer 2019-06-16 10:44:19 UTC
A commit references this bug:

Author: emaste
Date: Sun Jun 16 10:43:19 UTC 2019
New revision: 349101
URL: https://svnweb.freebsd.org/changeset/base/349101

Log:
  vtfontcvt: initialize bbwbytes to avoid GCC 4.2.1 uninitialized warning

  PR:		205707
  MFC with:	349100
  Event:		Berlin Devsummit 2019
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 30 commit-hook freebsd_committer 2019-06-16 12:27:45 UTC
A commit references this bug:

Author: emaste
Date: Sun Jun 16 12:26:46 UTC 2019
New revision: 349105
URL: https://svnweb.freebsd.org/changeset/base/349105

Log:
  vtfontcvt: initialize another variable to quiet GCC warning

  I believe this case could be triggered by a broken .bdf font.

  PR:		205707
  Reported by:	ci.freebsd.org
  MFC with:	349100
  Event:		Berlin Devsummit 2019
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 31 Dmitry Wagin 2019-06-16 12:34:42 UTC
(In reply to commit-hook from comment #30)

Cool, I was late to write about it.
Comment 32 Dmitry Wagin 2019-06-16 13:04:43 UTC
Created attachment 205123 [details]
vtfontcvt.c.diff

also i think there is a mistake here or i don't understand the hex font  format.
Comment 33 commit-hook freebsd_committer 2019-06-16 15:15:17 UTC
A commit references this bug:

Author: emaste
Date: Sun Jun 16 15:14:50 UTC 2019
New revision: 349111
URL: https://svnweb.freebsd.org/changeset/base/349111

Log:
  vtfontcvt: correct typo in hex parsing update

  PR:		205707
  Submitted by:	Dmitry Wagin
  MFC with:	349100
  Event:		Berlin Devsummit 2019

Changes:
  head/usr.bin/vtfontcvt/vtfontcvt.c
Comment 34 commit-hook freebsd_committer 2019-06-19 13:19:45 UTC
A commit references this bug:

Author: emaste
Date: Wed Jun 19 13:19:37 UTC 2019
New revision: 349189
URL: https://svnweb.freebsd.org/changeset/base/349189

Log:
  vtfontcvt: whitespace and other cleanup

  MFC r343842: vtfontcvt: whitespace cleanup

  MFC r348653: vtfontcvt: use VFNT_MAP_{NORMAL|BOL}_RH symbolic constants

  MFC r348656: vtfontcvt: unwrap a line per style(9)

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
_U  stable/12/
  stable/12/usr.bin/vtfontcvt/vtfontcvt.c
Comment 35 commit-hook freebsd_committer 2019-06-19 13:33:58 UTC
A commit references this bug:

Author: emaste
Date: Wed Jun 19 13:33:35 UTC 2019
New revision: 349191
URL: https://svnweb.freebsd.org/changeset/base/349191

Log:
  vtfontcvt: whitespace and other cleanup

  MFC r343842: vtfontcvt: whitespace cleanup

  MFC r348653: vtfontcvt: use VFNT_MAP_{NORMAL|BOL}_RH symbolic constants

  MFC r348656: vtfontcvt: unwrap a line per style(9)

  PR:		205707
  Submitted by:	Dmitry Wagin

Changes:
_U  stable/11/
  stable/11/usr.bin/vtfontcvt/vtfontcvt.c