Universal-Team / Universal-Updater

An easy to use app for installing and updating 3DS homebrew
https://universal-team.net/projects/universal-updater.html
GNU General Public License v3.0
892 stars 32 forks source link

[Bug Report] font size too big in CHT system #78

Closed roytam1 closed 3 years ago

roytam1 commented 3 years ago

Describe the bug the font scaling algorithm in v3.2.1 doesn't work well with CHT systems, which making font too big to see.

To Reproduce Steps to reproduce the behavior:

Expected behavior the font should be in a suitable size in CHT system.

Screenshots 2021-03-26_23-40-48 499

Console/Version (please complete the following information):

Console :

Luma 3DS version [v10.2.1-d0a44fd2-3gxldr] System version & Region [v11.14.0-38T]

Universal-Updater (please complete the following information):

If Release -> version [v3.2.1]

If Nightly -> Commit hash [example: a10b9da]

How do you launch it:

Additional context Add any other context about the problem here.

Epicpkmn11 commented 3 years ago

Huh... That's really weird, the font is based on the language, not the region, and the scaling is based on the loaded font so it should never scale the standard font...

Couple questions that might help narrow down why this would happen:

roytam1 commented 3 years ago

it should never scale the standard font...

but CHT and CHS has different font size than KOR/JPN.

  • Do you have a custom font? (Settings -> GUI Settings -> Use Custom Font)

this setting is off and I didn't ever turn it on here

  • Is your console set to Chinese or English?

the console has single language (i.e. Chinese) can be selected

  • Is Universal-Updater set to Chinese or English?

setting it to Chinese or English doesn't affect font scaling here

FYI: other application has similar problem, but normally they're showing smaller font size. for example, https://github.com/mgba-emu/mgba/issues/961

Epicpkmn11 commented 3 years ago

Huh, yeah it should use the correct font for Chinese and scale it appropriately, but the standard (English, Japanese, etc) font shouldn't be messed with...

On my American 3DS the fonts are all loaded and scaled correctly, so not sure why it's wrong here... Maybe C2D_FontLoadSystem() is somehow broken on Taiwan systems? Looking at other apps nothing else appears to explicitly load the system fonts with that...

Can you try this build? It forces the font to use the Taiwan version so if it still loads the standard one I'm going to assume that C2D_FontLoadSystem() is broken on Taiwan systems and I'll try work around that...

Universal-Updater.zip

Edit: Can you try this build too? It has C2D_FontLoadSystem() commented so if it does load the Taiwan font correctly then there's definitely a problem there. Universal-Updater.zip

roytam1 commented 3 years ago

Can you try this build? It forces the font to use the Taiwan version so if it still loads the standard one I'm going to assume that C2D_FontLoadSystem() is broken on Taiwan systems and I'll try work around that...

Universal-Updater.zip

this is a bit better with system font, but font size is still not scaled correctly 2021-03-27_09-01-25 116

Edit: Can you try this build too? It has C2D_FontLoadSystem() commented so if it does load the Taiwan font correctly then there's definitely a problem there. Universal-Updater.zip

same as screenshot above

EDIT: I think the problem is in https://github.com/Universal-Team/Universal-Core/commit/be0f07807c5323876bd5f4c3d106a0463b2bfee6#diff-4b0ede31b7a39085dcf44821151f9b41dfe9922d59fa0467ca79364906d77b91R224 since default system font size is 24px instead of 30px in CHT systems, size *= 1.3f is too big. and I did wonder why you need to manipulate y here (I haven't read through all logics)

Epicpkmn11 commented 3 years ago

... alright I have no idea why it's doing that so I'm just going to try doing this like the other apps with a scale based on the loaded font's size

size *= 30.0f / C2D_FontGetInfo(fnt ? fnt : Font)->tglp->cellHeight;

instead of trying to be fancy

if(!fnt) {
    switch(loadedSystemFont) {
        case CFG_REGION_CHN:
            y += 3.0f * size;
            break;
        case CFG_REGION_TWN:
            size *= 1.3f;
            y += 2.5f * size;
            break;
        case CFG_REGION_KOR:
            y += 3.0f * size;
            break;
        default:
            break;
    }
}

This makes them look slightly too big and not have enough top padding, but they're at least close...

Does this at least work for you? Universal-Updater.zip

Epicpkmn11 commented 3 years ago

EDIT: I think the problem is in https://github.com/Universal-Team/Universal-Core/commit/be0f07807c5323876bd5f4c3d106a0463b2bfee6#diff-4b0ede31b7a39085dcf44821151f9b41dfe9922d59fa0467ca79364906d77b91R224 since default system font size is 24px instead of 30px in CHT systems, size *= 1.3f is too big. and I did wonder why you need to manipulate y here (I haven't read through all logics)

I have it edit the Y because the Chinese and Korean fonts (at least the ones on my American 3DS) have less padding on the top so to keep vertical centering looking the same I have it increase the Y position a bit.

And huh, does C2D_FontGetInfo(Font)->tglp->cellHeight give 24 for you? It's 20 on the Taiwan font for me... If the fonts on actual Taiwan consoles are larger that'd explain it I suppose

Epicpkmn11 commented 3 years ago

This is what the fonts look like for me (American font dumps in Citra, though it looks basically the same on console)

If anything the old code has it rendering a bit too small for me, I based the scale on the size so that the Hànzì matched the size of the Kanji in Japanese, but that makes the Latin text a bit on the small side. The new code's latin text matches up decently but Hànzì are much bigger than Japanese and it's all a bit too high...

Old code: old

The last build I sent: new

roytam1 commented 3 years ago

and this is how it(latest auto-updated by itself) shows in my CHT device (with CHT firmware) 2021-03-27_09-27-07 243 2021-03-27_09-27-21 370

Epicpkmn11 commented 3 years ago

Hmm, did you try this latest version I sent? https://github.com/Universal-Team/Universal-Updater/files/6214923/Universal-Updater.zip (Sorry kinda buried it ;P)

If that works the same as it does for me I guess I'll just switch to that, it's the same as what new-hbmenu does so it should work

roytam1 commented 3 years ago

Hmm, did you try this latest version I sent? https://github.com/Universal-Team/Universal-Updater/files/6214923/Universal-Updater.zip (Sorry kinda buried it ;P)

it is worse than older build. 2021-03-27_10-48-29 682

Epicpkmn11 commented 3 years ago

Ah, I see the problem. Citro2D has this built in already, however it seems that for some reason it only works on actual Chinese consoles

https://github.com/devkitPro/citro2d/blob/e4196339707e87d6e362206b49ae26e6c0c72011/source/text.c#L74

s_textScale = 30.0f / glyphInfo->cellHeight;

https://github.com/devkitPro/citro2d/blob/e4196339707e87d6e362206b49ae26e6c0c72011/source/text.c#L324-L325

scaleX = s_textScale; scaleY = s_textScale;

So it should work if I just don't do the scale if the region matches the font... Can you test this build? (I enabled Korean temporarily for this too so if this works can you make sure that all (standard, china, taiwan, and korea) fonts are correct?)

Universal-Updater.zip

roytam1 commented 3 years ago

Ah, I see the problem. Citro2D has this built in already, however it seems that for some reason it only works on actual Chinese consoles

https://github.com/devkitPro/citro2d/blob/e4196339707e87d6e362206b49ae26e6c0c72011/source/text.c#L74

s_textScale = 30.0f / glyphInfo->cellHeight;

https://github.com/devkitPro/citro2d/blob/e4196339707e87d6e362206b49ae26e6c0c72011/source/text.c#L324-L325

scaleX = s_textScale; scaleY = s_textScale;

So it should work if I just don't do the scale if the region matches the font... Can you test this build? (I enabled Korean temporarily for this too so if this works can you make sure that all (standard, china, taiwan, and korea) fonts are correct?)

Universal-Updater.zip

OK it works well in Chinese language now 2021-03-27_12-09-35 841

but if it is set with English on CHT firmware, the problem is not fixed: 2021-03-27_12-10-12 848

Epicpkmn11 commented 3 years ago

Ahhhhhhhhhhhh, assuming I'm reading it right, Citro2D calculates the size to scale the fonts using the default font, but then applies that to all fonts, so on Chinese / Taiwanese / Korean consoles it breaks any font that isn't the default.

Can you test this build? This should be the last fingers crossed Universal-Updater.zip

(thanks for testing this all for me btw 😄)

Edit: I went back to my original scaling as I think that looks the best, but do you think it's a bit too small still? I could maybe do 1.4x or 1.5x or so instead of 1.3x (assuming this even works at all)

roytam1 commented 3 years ago

Can you test this build? This should be the last fingers crossed Universal-Updater.zip

yeah it works, but instead, font size is too small in Chinese language now: 2021-03-27_12-53-05 653 2021-03-27_12-54-03 167 2021-03-27_12-54-41 643

Epicpkmn11 commented 3 years ago

Alright, nice! Committed it (4c792a1ea0a78f97159ecad8e68d0bd0a8925430) with a slightly larger font size

ss

Epicpkmn11 commented 2 years ago

Are you able to test these two builds and see how the font looks in each? Something, I think devkitPro/citro2d#37, broke my previous workarounds and made the Chinese font too big for me on my USA consoles.

In these I've reverted the font region specific size adjustments and that fixes them all on my USA console, however I'm not sure if the console region specific adjustments are needed. nofontfix just prints with no console region specific adjustments while fontfix still has the console region specific adjustments.

nofontfix.zip fontfix.zip

roytam1 commented 2 years ago

nofontfix.zip

nofontfix

fontfix.zip

fontfix

so fontfix version is too small in CHT devices.

Epicpkmn11 commented 2 years ago

Thanks!

And nice, it seems to be fully fixed in Citro2D, no more weird workarounds required.