search

kvirc / KVIrc

The KVIrc IRC Client
http://www.kvirc.net/
GNU General Public License v2.0
240 stars 75 forks source link

Font width issue #2566

Closed DarthGandalf closed 10 months ago

DarthGandalf commented 10 months ago

Gentoo Linux amd64, version 4a51c0e82d066349c0450a70d010c4e4e1d6ec00 Qt 5.15.11

When the last time I accidentally deleted my config by trying test version of kvirc (yes, I need better backups) something happened with calculation of font width. See the video: https://youtu.be/tvh8nV_K2vM

This actually makes it hard to select a substring, because when I click it the string visually moves, and I end up selecting a different substring than I intended.

ctrlaltca commented 10 months ago

It seems in #2567 that this font can trick QFontDialog into thinking it's bold while it's not, and this could be also the cause of a miscalculation of the glyph width. Does it happen with another font, too?

DarthGandalf commented 10 months ago

I'll check.

Before the accident I was using the same font, and I have another kvirc setup with this font (different version though), which also works fine.

ctrlaltca commented 10 months ago

It would be nice to check if the value of fontIrcView is the same in the different configs, and / or how the new version handles the old config

DarthGandalf commented 10 months ago

Changing font didn't fix it, same for changing style

DarthGandalf commented 10 months ago

This happens even on a brand new config, when running kvirc -f -n /tmp/kvircrc, without changing any fonts.

DarthGandalf commented 10 months ago

I ran git bisect, and the regression happened in 28292170d8c02743cabfabba3a1623b8d71b9aa2

Looks like @pragmaware tried to fix this in 148d8d9e1b6530676093f660ec64f6a2ee673235 but for me it made the effect even stronger

ctrlaltca commented 10 months ago

I guess https://github.com/kvirc/KVIrc/commit/28292170d8c02743cabfabba3a1623b8d71b9aa2#diff-457dd4d023c2f347f0f23f4d9413c1d71ff648494e2d8cefdfd01bd007c88692L412 it what is causing the issue (similar changes apply to input and topic widgets aswell.

We used some tricks before to ensure correct font width calculation, disabling kerning and the QFont::ForceIntegerMetrics style, that doesn't exist anymore in Qt6. Qt's advice is to use QFontMetrics to restore the old behavior (see https://doc.qt.io/qt-5/qfont.html#StyleStrategy-enum ). So, in https://github.com/kvirc/KVIrc/commit/28292170d8c02743cabfabba3a1623b8d71b9aa2 i switched from QFontMetricsF (float) to QFontMetrics (int). This probably caused some other problems (overlapping texts), so @pragmaware reverted this last change in https://github.com/kvirc/KVIrc/commit/148d8d9e1b6530676093f660ec64f6a2ee673235

Right now I'm not able to reproduce this, but I'll try harder.

DarthGandalf commented 10 months ago

I tried to replace horizontalAdvance with width, but it's still broken. Would need to debug further

ctrlaltca commented 10 months ago

A possible fix is here: https://github.com/kvirc/KVIrc/compare/master...ctrlaltca:KVIrc:fix_surrogate At least, it fixes emojis as reported by @gizmokid2005 yesterday on IRC Since the inputeditor needs a similar fix, i'll open a PR when that's ready as well

ctrlaltca commented 10 months ago

Can you please test these two different attempts at fixing the issue?

Attempt1: Attempt at fixing font width size by enable full font hinting, aka forcing glyphs to be aligned to pixels in order to disable subpixel positioning (source: https://www.mail-archive.com/development@qt-project.org/msg43330.html) This mimics the old "ForceIntegerMetrics" style that got removed in Qt6 https://github.com/ctrlaltca/KVIrc/commit/c9a84af3d76ad821fe0583af2510a301c6c8496f

Attempt2: Attempt at fixing font width problem by making the selection code use floats, too https://github.com/ctrlaltca/KVIrc/commit/7c569449d4e2b3f32b72cf053f0e16e2f1f6e7ee

DarthGandalf commented 10 months ago

Attempt1 fixes it, Attempt2 does not.