Rockbox

Tasklist

FS#9931 - As for some fonts, ascent, descent is not correct.

Attached to Project: Rockbox
Opened by Yoshihisa Uchida (Uchida) - Friday, 20 February 2009, 11:24 GMT
Last edited by Alexander Levin (fml2) - Friday, 20 March 2009, 19:06 GMT
Task Type Patches
Category Font/charset
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

There is some fonts with not correct value of pf->ascent and pf->decent.

The value of pf->ascent (pf->descent) is equal to the value of FONT_ASCENT (FONT_DESCENT) in *.bdf.

The problem that the shape of the character is not correctly displayed occurs when a character that is bigger than the value of FONT_ASCENT (or FONT_DESCENT) exists.
(see orig_1.png, orig_2.png (using 35-Adobe-Helvetica-Bold))

I corrected to make pf->ascent and pf->descent by using BBX of an actual character when *.fnt files create.

new_1.png, new_2.png are result of using the font made after the patch is applied.

The font_ascent_descent_changes.txt is the list of the font to which ascent or descent changed.





This task depends upon

Closed by  Alexander Levin (fml2)
Friday, 20 March 2009, 19:06 GMT
Reason for closing:  Fixed
Additional comments about closing:  There were no further comments/issues, so close it (the fix was committed in r20321)
Comment by Alexander Levin (fml2) - Saturday, 21 February 2009, 16:28 GMT
Yoshihisa, could you please provide the information about what characters are affected? I think in this case the fonts should be corrected rather than try to straighten it up with the program. If a character goes beyond the ascent/descent specified in the font then it's an error in the font design IMO.

Note that the situation in  FS#9907  is different. There the program can "correct" the values specified in the header. I think it's OK since the values specified in the header are not required to be the MAX for the font (at least I couldn't find a place in a spec explicitly stating that). I'd rather interpret that values as the default bounding box values, i.e. the values to use if a character doesn't have its own BBX clause.

Thank you for your work!
Comment by Alexander Levin (fml2) - Saturday, 21 February 2009, 19:29 GMT
One more comment: you read the fields from the BBX clause in the file header. But, according to the BDF spec, this clause may only occur at the char level, not in the header. Hence the "if (isprefix(buf, "BBX "))" will never get executed.
Comment by Yoshihisa Uchida (Uchida) - Sunday, 22 February 2009, 07:13 GMT
Alexander, thank you for a valuable comment.

I upload the list of influenced characters.
- font_bbx.txt is the list of characters for which the change in BBX or the change in the style of type is necessary.
- font_bbx.txt is the list of characters for which the change in FONTBOUNDINGBOX, BBX or the change in the style of type is necessary.

Remark:
Because Sazanami-Mincho fonts submits the patch of the font change (FS#9206), these have been excluded from the list.

PS.
For  FS#9907 , when the confirmation of operation ends about the your patch file, I will write the comment.
Comment by Alexander Levin (fml2) - Sunday, 22 February 2009, 11:01 GMT
Thank you for the information! I think, the font ascent and descent are correct (hence the task title is somewhat inaccurate), but rather the BBX clauses of some characters are not correct. Attached is the patch for the convbdf tool that prints warnings if a character goes beyond the font's ascent or descent. (Or should the tool print an error and stop?) Font patches will follow.
Comment by Alexander Levin (fml2) - Sunday, 22 February 2009, 11:17 GMT
Here are the patches for some fonts (other, i.e. Helvetica, require too many changes). With my version of the convbdf tool, I haven't got any warnings for 14-Nimbus and 19-Nimbus. The Helvetica fonts should probably be adjusted with a script, it's not doable by a human being! :-) Anyone?
Comment by Alexander Levin (fml2) - Sunday, 22 February 2009, 20:20 GMT
Attached is a script to correct the BBX clauses. It reads the BDF file from STDIN and writes the corrected file to STDOUT. Information about the corrections made is written to STDERR.
Comment by Alexander Levin (fml2) - Sunday, 22 February 2009, 21:26 GMT
PS. I think one should correct the characters' BBX based on the font's ascent and descent rather than the opposite because font size, ascent and descent (ascent+descent=size) are the base properties of the font. If we correct them then we should rename the font as well. For example, 35-Adobe-Helvetica-Bold.bdf has the size 35, ascent is 28, descent is 7. That's OK. If a character breaks that bound it should be corrected. Or, if it can't, ie. if the char's shape doesn't easily allow to correct it we might consider changing the font size to 36 or 37 or whatever.
Comment by Yoshihisa Uchida (Uchida) - Tuesday, 24 February 2009, 11:25 GMT
For conv_check.patch:

> Attached is the patch for the convbdf tool that prints warnings if a character goes beyond the font's ascent or descent. (Or should the tool print an error and stop?) Font patches will follow.

I think that the convbdf should stop if pf->ascent and pf->descent don't change according to the value of BBX.
Because when FONT_ASCENT < bbh + bby or FONT_DESCNET < -bby for some characters, the fnt file is not valid.
(see orig_1.png. "Á" is invalid glyph.)

I think that it is impossible to change the shape of fonts of *-Adobe-Helvetica or *-Adobe-Helvetica-Bold.
Because if problem characters correct, it is necessary to correct other characters.
I think that I should change the value of ascent and descent like my patch according to the value of BBX of actual characters.

I think that we should change the font name according to the size of the font.



Comment by Alexander Levin (fml2) - Tuesday, 24 February 2009, 19:34 GMT
Yoshihisa, I tend to agree with you that convbdf should stop if a "too big" character is encountered. I also agree with you that we should change the ASCENT/DESCENT characteristics of some fonts. That would imply the change of the font size and hence also a change of the font name.

But I don't completely agree that the ascent and descent should be computed as the MAX(bbh+bby) and MAX(-bby) respectively. Because these values may have been chosen by design.

For example, in the font 14-Nimbus, the max used ascent is 10 and not 11 (as "allowed" by the font's ascent setting). But that doesn't mean that we should change the font's ascent to 10, because the intended font size is 14 and not 13.

Attached is the patch that prints an error message and stops if a too big character is found.
Comment by Yoshihisa Uchida (Uchida) - Saturday, 28 February 2009, 07:54 GMT
After I sended the comment, I thought about the name of the font again.
In my previous opinion, I insisted that I had to change the name of the font.
However, I came not to think to change the name of the font.
1) When the font name is changed, it will give rise to a misunderstanding because the size of the character doesn't change in this correction.
2) Because the size of the character becomes small if the same font as former size is made when the name of the font is changed, the displayed character is not beautifully displayed.

Defining the name of the font as follows, the font name can be done as well as the name used now.
(FONT_ASCENT + FONT_DESCENT)-font name


I think that I should correctly make the Rockbox font file by using the value of BBX.
I change the font structure (struct font dfined in firmware/font.h) like the value of ascent and descent not changing, but adding parameter maxascent, maxdescent newly.
maxascent = The maximum value of ascent (= bbh + bby) of all characters.
maxdescent = The maximum value of descent (= -bby) of all characters.

Also, I change the height of the font to maxascent + maxdescent instead of FONT_ASCENT + FONT_DESCENT.

When the value of maxascent - FONT_ASCENT > FONT_ASCENT/2 or maxdescent > 3 * FONT_DESCENT,
I judged that the value of BBX was not correct, and convbdf output the error message.

When the header is checked, BBX is checked still. This is from a purpose that the efficiency of processing is not deteriorated.

Comment by Alexander Levin (fml2) - Saturday, 28 February 2009, 10:43 GMT
I'm sorry but I can't agree with you now. If I'm not mistaken, all rockbox fonts are named like (ASCENT+DESCENT)-fontname. The "ascent+descent" part indicates the "full" height of the font, i.e. the number of pixels between two lines of text rendered with the font. It doesn't tell much about the actual size of glyphs in the font. There are already fonts having identical glyphs but different font size -- due to the different values of ascent/descent. I.e. they only have different line spacing.

The fields maxascent/descent are only needed during the font generation, but not at runtime. Hence I wouldn't introduce them into the generated file format.

And the "tolerance" rules are a bit of black magic for me. Why have you chosen exactly these values? In any case, if a character goes beyond the font's ascent/descent then the font will be displayed not correctly. Hence I wouldn't introduce any tolerance but would instead just correct the fonts.

I think, in this discussion we need opinion from other people.
Comment by Alexander Levin (fml2) - Sunday, 01 March 2009, 17:28 GMT
OK, I've thought about it again and now seem to understand everything. In the BDF format, glyphs can go beyond the font's ascent or descent. Ascent and descent are just use to align the glyphs on the base line and to define the font height (=distance between two subsequent base lines). It is the responsibilty of the rendering engine to correctly display glyphs going beyond that. For example, a character with a large ascent could overlap a character on the previous line.

But the Rockbox rendering engine can't render overlapping lines (this would for example cause problems with scrolling lines). (Rockbox also doesn't render overlapping characters -- negative bbdx in BDF lingo). Hence the best we can do is to vertically clip the glyphs so that they stay within the font's ascent and descent.

Attached is the patch that implements vertical clipping and also two examples of how text (multiple lines of "ÊÝĚČ" with the font 35-Adobe-Helvetica) looks like without (unclipped.bmp) and with (clipped.bmp) clipping. This way, the font height is preserved as it was provided by the font designers but there's also no weird leftovers from other characters. If a character is clipped, a warning is printed out on STDERR but the font is processed further.

So once again: the problem stems from the limitations of how Rockbox renders text.
Comment by Yoshihisa Uchida (Uchida) - Wednesday, 04 March 2009, 13:55 GMT
I am sorry for the comment's slowing.

I tried to make the font by using the patch file for which you made.

When I use new font, it is not normally displayed according to the text.
See after_clipped_patch.png
View text
 1st line U+0122 U+0122 U+0067 U+0067
 2nd line U+01D7 U+0041 U+01D7 U+0041
Font: 35-Adobe-Helvetica-Bold.

In your method, I think that it doesn't go well.
I think that I should correct the format of the Rockbox font file, and introduce the mechanism that the line interval is changed into the Rockbox.

By changing font's height to maxascent + maxdescent, I also noticed the problem of the extension of the interval of the line more than the current version.
It causes because of being coded in the Rockbox on the assumption that the interval of the line is equal to font's height.

I create the patch file that added the mechanism that the interval of the line was able to be changed dynamically.
The text can be displayed by this mechanism at same line intervals as before.

[Setting]
Settings -> General Settings -> Display -> Line Space

1) When you select "Wide".
- line space sets font's height (= maxascent + maxdescent).

In this case, all caharcters are normally displayed.(See linespace_wide.png)
But the interval between "g" and "A" is too wide.

2) When you select "Middle".
- line space sets max (maxascent + descent, ascent + maxdescent).

In this case, it is normally displayed excluding the first row(U+122, U+1D7). (see linespace_middle.png)

3) When you select "Narrow".
- line space sets ascent + descent.
In this case, only characters such that the ascent <= FONT_ASCENT and the descent <= FONT_DESCENT are normally displayed.
(See linespace_narrow.png)
In this example, only "g" and "A" are normally displayed.
Comment by Yoshihisa Uchida (Uchida) - Wednesday, 04 March 2009, 14:26 GMT
Additional comment.

Please apply the patch of line_space.patch after appling the patch of convbdf_height_2.patch.
Comment by Alexander Levin (fml2) - Wednesday, 04 March 2009, 20:10 GMT
> In your method, I think that it doesn't go well.

It doesn't look nice, I agree, but it looks exactly as intended with the patch. But it's because the way Rockbox renders fonts.

While I can see the point in having the feature "line spacing" it shouldn't be considered as a cure to the problem we're discussing here since with a "bad" font, text is still rendered "ugly" with one or the other setting. I Think the two features are independent.

Please join the discussion on the mailing list.
Comment by Alexander Levin (fml2) - Wednesday, 04 March 2009, 21:46 GMT
Attached is the output for all fonts in Rockbox telling how many glyphs (and which ones) go beyond ascent/descent. The file has been produced with the attached script. The script should be started from the build directory.
Comment by Yoshihisa Uchida (Uchida) - Thursday, 05 March 2009, 04:53 GMT
I give other examples.

Example 1. See clipped.png
It is displayed in the type as which any character is the same. But, it is all different characters.
U+0170, U+01D5, U+01D7, U+01D9 and U+01DB is displayed from the left.

Even if a part of the character is lost by "clipped", the problem might be few if the character used for the text is an alphabet.
There is a possibility to be displayed in a quite different character in the Kanji (that is CJK Ideoglyph).

Example 2. See japanese.png.
In the left figure (Fig. 1), two different characters are displayed. And the right figure (Fig.2) is a display of same characters by the clipped font.
(I use the modified 14-Sazanami-Mincho)
In the right figure, it is displayed in the character as which two characters are the same.

These are characters with a quite different meaning.
下 (It is a meaning "Under" in Japanese.) 卜 (It is a meaning " Fortune-telling" in Japanese.)

Also, these characters are used for person's name.
In the clipped character, Mr. 下部 (pronounced "Simobe") is displayed as Mr.卜部 (pronounced "Urabe").

In the Rockbox, everyone can freely add various fonts.
There is a possibility that strange phenomenons are generated more according to the font.

I think that it is a wrong selection to clip the font.
If it is necessary to clip the glyphs by all means, it is necessary to change the screen display processing of the Rockbox.
(I do not think that I want to add such a function at all.)
Comment by Alexander Levin (fml2) - Thursday, 05 March 2009, 21:45 GMT
Your examples clearly demonstrate that if a font has "wrong" metrics then the glyphs will be clipped and hence look weird (or will even lose information) in Rockbox. But the solution to this should be rather in correcting font files -- which is done once -- rather than in making RB's rendering more complex. Which (a) increases code complexity and bin size and (b) still does not completely solve the problem that glyphs are displayed correctly (as your example linespace_narrow.png shows).

I plan to introduce some more command line parameters to the convbdf utility so that it will be possible to change the font's ascent/descent to desired values or to make it stretchable, i.e. to let them increase if that's necessary to fully display the glyphs. RB's default build process would not allow this feature because we want that the resulting .fnt file has the height declared in the font name. But you, for example, could produce adjusted fonts for your personal use by increasing ascent/descent. Thus you'd code the appropriate line spacing into the font.

Later we might also adjust the BDF files.

Here's what I think the new command line parameters could look like (a quote from my post to the mail list):

So my plan is as follows: I'll change the convbdf utility so that it
will have command line parameter for ascent/descent "stretchability" or
"tolerance". Before the font is generated, all glyphs will be analyzed
and the max values for glyph ascent/descent will be found. If they don't
go beyond the font's values then everything is fine. Otherwise the
font's ascent/descent values will be increased but no more than
specified by the new parameters. Parts of the glyphs that go beyond the
newly computed ascent/descent will be clipped, i.e. those parts won't be
generated in the .fnt file, and a warning message issued.

It will be possible to specify the amount for e.g. ascent to grow in
absolute pixels (nn), in % of the original value (nn%) and as a "must!"
value (nn!). The latter directly sets the ascent to the given value.
There will be separate parameters for ascent and descent.
Comment by Alexander Levin (fml2) - Sunday, 08 March 2009, 16:27 GMT
With r20239 I introduced the new options, -a and -d, that specify how the ascent and descent are allowed to grow. If you specify -a100 and -d100 when generating a font, ascent and descent will be changed as needed (I assume that no glyph has ascent/descent greater than 100 :-)

Please test and tell me whether it fits your needs. If yes, I'll close the task, if not, then we'll see what to do :-)
Comment by Yoshihisa Uchida (Uchida) - Friday, 13 March 2009, 13:07 GMT
I report on an point by using new convbdf.

I think that you should change the error message though the following error message are output when the value of -a(-d) option is specified for a too large value.
" ERROR: /for/bar/tools/convbdf.c:1110 width x height overflow xxx bytes buffer, needs xxxx"

Points that you should change:
1) You should delete the file path and the line number from the error message.
(It is because where the error message was output is understood even if these information doesn't exist. )

2) I think that I need not output the size of buffer(=sizeof bitmap_t).
I think that you should output the maximum value of the value of ascent (or descent) than it.
(It is because the maximum value of ascent (or descent) is not understood even if the size of buffer is understood.)

I will report when something is noticed again.
Comment by Alexander Levin (fml2) - Friday, 13 March 2009, 19:05 GMT
With r20321 I hopefully implemented what you mean. Please tell me whether there are issues left.
Comment by Yoshihisa Uchida (Uchida) - Sunday, 15 March 2009, 13:36 GMT
I complete to confirm about new convbdf.

1) If it is possible, I am glad when the option that assumes the value of ascent (descent) to be maxascent (maxdescent) is prepared.
e.g., convbdf -ma -md -f 35-Adobe-Helvetica.bdf

Because of I think the maxascent (maxdescent) to take time to obtain all fonts by hand power.

2) It is necessary to correct tools/buildzip.pl to correspond to the automatic construction of the font.
I create an easy correction file about this.
Comment by Alexander Levin (fml2) - Sunday, 15 March 2009, 18:47 GMT
To 1). I don't quite understand what you mean. Do you mean that there should be two more options to specify the max. value of the ascent/descent? Additionally to or instead of the existing options that specify the max. DELTA? Or do you mean, that the options should allow the ascent/descent to grow to the max values that occur in the font? The latter is possible now as well. Just specify the max. value to grow as 100 (which should be more than enough). But without an exclamation. E.g.

convbdf -a100 -d100 -f my-font.bdf

Without an exclamation, 100 is just the max. allowed value. The font will actually grow not by 100 but to the max. value of a char's ascent/descent. (WITH an exclamation, e.g. -a100!), the ascent would be directly increased by 100, regardless of whether the glyphs "demand" it.

I wouldn't like to introduce the options "ma" and "md" just because of the way the option parsing is implemented: It assumes that all options a single letters.


To 2). The new options were introduced as a way for users to create their "personal" versions of fonts with the other values of asc/desc than specified in the font (bdf). They are NOT assumed to be used in the official Rockbox builds. To officially produce differently ascented/descented fonts (.fnt files), the BDF files must be edited (ASCENT/DESCENT lines) and renamed to reflect the new height of the font. Hence I think, there's no need to modify the buildzip.pl script (you can use it privately of course to always produce the fonts resized as you like it).

BTW: I'd include in the file fonts.lst (if I'd use it) only the fonts for which a non-default value for ascent/descent growth is needed. With your current version, you always have to not forget to include a newly added BDF file into the file.

I suggest that we close this task (its focus has became blurred IMO) and open new tasks (one for each font) that modify the ASCENT/DESCENT metrics of the fonts and rename them. I'd discuss the proposed changes in IRC or mailing list first. I definitely agree that e.g. the ASCENT of 35-Adobe-Helvetica should be increased. May be we should contact the author(s) of the file to ask them why they chose the values as they are.


But do I understand you correctly that now there are no more issues with utility itself but only with the way it's used?

Loading...