Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Save previous font and Unicode map broken #94

Closed
Tecol87 opened this issue Jun 16, 2023 · 20 comments
Closed

Save previous font and Unicode map broken #94

Tecol87 opened this issue Jun 16, 2023 · 20 comments
Labels
bug

Comments

@Tecol87
Copy link

Tecol87 commented Jun 16, 2023

Saving the previous font is broken since 2.6.0. The tty just shows garbage.
It works to save a font with 2.5.1 and load it with 2.6.0.

Steps to reproduce:

# setfont default8x16
# # save font
# setfont -O /tmp/myfont
# # load font
# setfont /tmp/myfont
setfont: ERROR setfont.c:141 try_loadfont: font position 32 is nonblank
setfont: ERROR setfont.c:153 try_loadfont: background will look funny
@Tecol87 Tecol87 mentioned this issue Jun 16, 2023
Closed
@legionus
Copy link
Owner

Hm. I can't reproduce it.

#!/bin/sh -e
kbd-2.16.0/src/setfont /lib/kbd/consolefonts/default8x16.psfu.gz
kbd-2.16.0/src/setfont -O /tmp/default8x16.psfu
kbd-2.16.0/src/setfont /tmp/default8x16.psfu

The script runs without errors.

$ kbd-2.15.1/src/setfont -O /tmp/UniCyr_8x16.psf.v2.15.1
$ kbd-2.16.0/src/setfont -O /tmp/UniCyr_8x16.psf.v2.16.0

$ file /tmp/UniCyr_8x16.psf.v2.15.1 /tmp/UniCyr_8x16.psf.v2.16.0
/tmp/UniCyr_8x16.psf.v2.15.1: Linux/i386 PC Screen Font v1 data, 256 characters, Unicode directory, 8x16
/tmp/UniCyr_8x16.psf.v2.16.0: Linux/i386 PC Screen Font v1 data, 256 characters, Unicode directory, 8x16

$ cmp /tmp/UniCyr_8x16.psf.v2.15.1 /tmp/UniCyr_8x16.psf.v2.16.0; echo $?
0

$ kbd-2.16.0/src/setfont /tmp/UniCyr_8x16.psf.v2.16.0; echo $?
0

I don't see any difference in font dumps between old and current version.

@legionus
Copy link
Owner

@Tecol87 Please provide more information.

@Tecol87
Copy link
Author

Tecol87 commented Jun 17, 2023

@legionus
Thank you very much.
This happens on my uptodate arch linux system with ter-u32b and default8x16. If I load the resulting font, it shows diamonds where no character is and it shows mostly spaces and some uncommon special characters where a character is.

cmp /tmp/myfont-1*
/tmp/myfont-15.1.psfu /tmp/myfont-16.0.psfu differ: byte 109, line 1

Here are the two different fonts files. I gzipped them, because github does not accept .psfu files.
myfont-15.1.psfu.gz
myfont-16.0.psfu.gz

I don't know what additional data I can provide, but I am happy to provide any data or info you request, also I am happy to try patches.

@legionus
Copy link
Owner

What kernel version are you using?

@Tecol87
Copy link
Author

Tecol87 commented Jun 18, 2023

@legionus 6.3.8-arch1-1

@Tecol87
Copy link
Author

Tecol87 commented Jun 18, 2023

@legionus
Good catch, this does not happen with 6.1.34-1 (and not with 6.3.4).
Do you know how to debug this or where to send it? Otherwise I'll have a look at what kernel mailing list this fits. (and try to bisect it tomorrow).

@legionus
Copy link
Owner

@Tecol87 It seems to be related to tall font support.
Could you try the for-master branch fix ?

https://github.com/legionus/kbd/archive/1e15af4d8b272ca50e9ee1d0c584c5859102c848.zip

@Tecol87
Copy link
Author

Tecol87 commented Jun 20, 2023

@legionus
Thank you, sadly the behavior is the same.

@legionus
Copy link
Owner

@Tecol87 Interesting. Then could you please attach the output of the command:

strace -o /tmp/setfont.log setfont /tmp/myfont

@legionus
Copy link
Owner

Ops. Sorry. Not output, but /tmp/setfont.log itself.

@Tecol87
Copy link
Author

Tecol87 commented Jun 20, 2023

@legionus
Here is the strace log.
setfont.log.gz

@Tecol87
Copy link
Author

Tecol87 commented Jun 20, 2023

@legionus
Sorry, the log is from 6.3.5 (while bisecting).
I attach a log from 6.3.8 (save and load).

setfont-load.log.gz
setfont-save.log.gz

@Tecol87
Copy link
Author

Tecol87 commented Jun 20, 2023

Is there a reason you want the strace from loading the font? I thought the problem is saving the font (-O).
The saved font from 2.6.0 and 1e15af4d8b272ca50e9ee1d0c584c5859102c848 do not differ.
If I save a font under 6.1.34 and load it under 6.3.8, the font loads correct.

@legionus
Copy link
Owner

The thing is that kernel 6.2 introduced support for tall fonts. They need to be fetched and loaded via a new operation. Then this font will be written to the file. Then you try to read this file again and load it into the kernel.

You have a normal font in the linux kernel so there is no difference. The old setfont retrieves the font according to the old scheme and write it. The result is a correct .psfu. The new setfont on a new kernel should use the new mechanism (should be visible in strace). I think that at the time of writing the font to the file, the psfu header is written incorrectly and therefore an error occurs at the time of loading the font.

@Tecol87
Copy link
Author

Tecol87 commented Jun 20, 2023

Thank you very much for the detailed explanation.
I had a look at the files and there are differences in the first 16kb (17kb total).
Very often content in 6.1 is zero in the 6.3 file, besides that, I was not able to recognize a pattern. (Also I don't have knowledge about psfu files).

@legionus
Copy link
Owner

Unfortunately I have an older kernel and can't reproduce this problem. But I tried to fix the font reading/writing in for-master. Please try this version.

https://github.com/legionus/kbd/archive/refs/heads/for-master.zip

@Tecol87
Copy link
Author

Tecol87 commented Jun 21, 2023

@legionus
Thank you very much for your time and work!
The font changed a bit, now the diamonds are squares and the random characters are different. I attach the strace logs and the font file.

setfont-load.log.gz
setfont-save.log.gz
my_font.gz

@legionus
Copy link
Owner

@Tecol87 Thank you. Now the situation has become perfectly clear. Please try the for-master version again. I believe the problem is fixed there.

@legionus legionus added the bug label Jun 22, 2023
@Tecol87
Copy link
Author

Tecol87 commented Jun 22, 2023

@legionus
It works like a charm, thank you very much!
Respect for fixing this without the new kernel!

legionus added a commit that referenced this issue Jun 23, 2023
@legionus
Get a font with a charsize of at least 32
7e58ec6 
The new KD_FONT_OP_GET_TALL uses the height as the size for each
character (the old KD_FONT_OP_GET always uses 32). This works for fonts
larger than 32x32. But if we get the old 8x16 font using the
KD_FONT_OP_GET_TALL for it, then vpitch becomes 16. When saving such a
font to a file in psf format, the vpitch information will be lost and
the font will turn into garbage.

psf1 has no height information at all. psf2 preserves the height, but if
it's less than 32 we can't tell the vpitch for that font from a normal
font that has 32.

In the future we may add a flag to psf2 to distinguish fonts with vpitch
< 32, but this would require carrying this information until the font is
loaded into the linux kernel. There is currently no place to store this
information.

For now, we will use the KD_FONT_OP_GET for fonts, and only if the
font has a height greater than 32 will we use KD_FONT_OP_GET_TALL. In
this case, we can understand that we need to use psf2 to save such a
font and then, when reading, we can understand which interface should be
used to load into the kernel.

Fixes: 287a3ba ("font: Leverage KD_FONT_OP_GET/SET_TALL font operations")
Link: #94
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
@legionus
Copy link
Owner

Fixed in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@legionus @Tecol87