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.

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

Fallback to 8x16 font in case 8x14 not present in VGA ROM #2137

Merged
merged 6 commits into from
Dec 16, 2024

Conversation

rafael2k
Copy link
Contributor

We do a test to check if the 8x14 is present based on a VGA BIOS behaviour found in:
https://github.com/rafael2k/fix8x14/blob/9d7dfd4d13d54e2d42f2bf0b9734632599c0e67b/SRC/DIAG8X14/DIAG8X14.C--#L124
and fall back to 8x16 is the font is not present.

ps: can you do a squash merge if all good?

@ghaerr
Copy link
Owner

ghaerr commented Dec 16, 2024

Really great work!

I want to say this PR is a super example of how nicely a problem can be and has been solved by multiple conversations and contributors to ELKS.

Starting with the discussion in #2117 (comment), it was initially deemed that RAM font code should be added to all nano-X executables. Then, because of the larger size requirement, it was thought that a new configuration option should be added, which would work but require users to recompile from source.

However, with @bocke's input and @rafael2k's coding and testing, there was a path found that required negligible extra size to all Nano-X executables with no (re)configuration requirement - and just work! Solved in the (new) ELKS way, small, beautiful and it just works. :)

Very nice @rafael2k and @bocke, I am very impressed with how this turned out. Thank you!

@ghaerr ghaerr merged commit 51ab9e2 into ghaerr:master Dec 16, 2024
@rafael2k
Copy link
Contributor Author

nxterm running on 8x16 fonts (my notebook has no 8x14):
5116393243653812027

nxterm with ram fonts:
5116393243653812028

@rafael2k
Copy link
Contributor Author

This was a nice one indeed. ELKS' team work.
: )

@ghaerr
Copy link
Owner

ghaerr commented Dec 16, 2024

@rafael2k,

It just occurred to me that on ancient BIOS's, there is a possibility that the GETROM8x16 function is not implemented, and happens to return the same address as its (only) 8x14 ROM set. So final testing of this enhancement will have to wait until it gets tested on an old, non-VGA machine with old ROMs.

Should it fail, I'm sure we can come up with a configure-less fix, like the checking CX and DL values int 10h return instead of just the address at ES:BP, or something like that.

Out of curiosity, what does QEMU do with this? Does Nano-X end up with 8x16 or 8x14 fonts?

@rafael2k
Copy link
Contributor Author

rafael2k commented Dec 17, 2024

In QEMU Nano-X ends up with 8x14 fonts.
Anyone with a XT for testing?

Btw, can't we just compare the shape of a couple of chars of the 8x14 table, then if they are wrong, we continue with the logic to compare 8x14 and 8x16 pointers?

ps: or how you propose I think, we could check int 10h return too, right?

@rafael2k
Copy link
Contributor Author

rafael2k commented Dec 18, 2024

So, a proposal here is to use the 8x14 as default (as before), and let the user choose the font via env variables, and we create a wiki page explaining nano-X font behavior and how to identify if your VGA bios, for example, does not have the 8x14 font.

related PR: #2141

@ghaerr
Copy link
Owner

ghaerr commented Dec 18, 2024

can't we just compare the shape of a couple of chars of the 8x14 table,

You don't want to get into comparing glyphs, but looking at font width/height may be available. I would say we're ok until someone has a problem - no need for complex code for ROM fonts.

let the user choose the font via env variables,

Most of the ROM fonts won't be usable, but if you want to play with it, may I suggest using env var ROMFONT=3 etc where it is the index used for the 1103 function call, instead of names. Saves lots of code that won't be used very much. You'll then be able to quickly see each font through enumeration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants