Bug 404907 - ktexteditor: lineheight regression
Summary: ktexteditor: lineheight regression
Status: RESOLVED FIXED
Alias: None
Product: frameworks-ktexteditor
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources All
: NOR major
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
: 385798 387428 390665 404713 406432 412095 414408 415036 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-02-28 12:42 UTC by RJVB
Modified: 2020-07-02 23:29 UTC (History)
11 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
sample text containing Unicode emoji (578 bytes, text/plain)
2019-02-28 12:42 UTC, RJVB
Details
incorrect lineheight due to #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b (45.20 KB, image/png)
2019-02-28 12:46 UTC, RJVB
Details
correct rendering, after reverting #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b (45.20 KB, image/png)
2019-02-28 12:46 UTC, RJVB
Details
118436: correct rendering, after reverting #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b (46.97 KB, image/png)
2019-02-28 22:24 UTC, RJVB
Details
both, with shared line background (37.33 KB, image/png)
2019-02-28 22:36 UTC, RJVB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2019-02-28 12:42:45 UTC
Created attachment 118434 [details]
sample text containing Unicode emoji

SUMMARY
Commit #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b introduced a regression when text contains certain Unicode glyphs that cause Qt to use a fallback (= different) font. This is platform independent.

STEPS TO REPRODUCE
1. open the attached example document which contains emoji

OBSERVED RESULT
Incorrect lineheight for lines that contain the emoji

EXPECTED RESULT
Correct lineheight.

SOFTWARE/OS VERSIONS
KDE Frameworks Version: KTextEditor master
Qt Version: 5.9.7

ADDITIONAL INFORMATION
Comment 1 RJVB 2019-02-28 12:46:06 UTC
Created attachment 118435 [details]
incorrect lineheight due to #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b

The fallback font used here is Segoe UI Symbol
Comment 2 RJVB 2019-02-28 12:46:43 UTC
Created attachment 118436 [details]
correct rendering, after reverting #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b

same fallback font in case it wasn't obvious ;)
Comment 3 Dominik Haumann 2019-02-28 20:49:52 UTC
Btw, I don't think this has anything to do with a fallback-font. It's simply that some glyphs force other line heights, and in that case we always lose the game.
Comment 4 Christoph Feck 2019-02-28 20:59:19 UTC
The issue is indeed with different fonts getting substituted based on script coverage. Qt only uses the same point size for the fallback fonts, but each font itself is free to use any (pixel) size for the requested point size, mostly depending on the amount of leading the designer felt required.
Comment 5 Thomas Schöps 2019-02-28 21:07:27 UTC
It seems that both uploaded images are the same and both show the incorrect state.

I couldn't reproduce this here since the emojis fit into the line with the font that is used here.

I am wondering whether the emojis already extended beyond the line height before, but that was hidden by the workaround with the second drawing pass that was applied there. To determine this, it might perhaps be helpful to have a screenshot of the correct state in which the cursor is in one of the lines with an emoji (since this line's background should be of a different color then, making the line height visible).
Comment 6 RJVB 2019-02-28 22:24:49 UTC
Created attachment 118449 [details]
118436: correct rendering, after reverting #ce4194b5bc2e13a6f02d5d03e7b003fa0eba650b
Comment 7 Thomas Schöps 2019-02-28 22:35:07 UTC
Thanks for adding the image. Comparing the correct and the incorrect version in GIMP, it seems that the patch causes no difference in line height for the font used here. So, the problem must come from the other part of the change: the removal of the second drawing pass workaround.
Comment 8 RJVB 2019-02-28 22:36:28 UTC
Created attachment 118450 [details]
both, with shared line background

Good idea the check the shaded line background mode. I don't know if this more really paints the entre lineheight (or rather, the text height) but it does show a little bit better what's going on. In particular pasting one of the emojis on a line that didn't have one yet:
- shows the line being pushed down (with the reverted commit) as if the emoji is higher
- shows the shaded background doesn't change (much) in height.

An additional test would be to use Segoe UI Symbol as the editor font, but that's not a monospaced font and I don't know to what extend Kate can handle variable-width fonts.
Comment 9 Christoph Cullmann 2019-03-02 15:21:26 UTC
See latest comments in

https://phabricator.kde.org/D19283

If you use proper fonts, that works.
Comment 10 Christoph Cullmann 2019-07-13 19:08:37 UTC
*** Bug 404713 has been marked as a duplicate of this bug. ***
Comment 11 Christoph Cullmann 2019-07-14 08:28:53 UTC
*** Bug 406432 has been marked as a duplicate of this bug. ***
Comment 12 Christoph Cullmann 2019-07-14 08:32:51 UTC
*** Bug 390665 has been marked as a duplicate of this bug. ***
Comment 13 Christoph Cullmann 2019-07-14 11:09:40 UTC
*** Bug 385798 has been marked as a duplicate of this bug. ***
Comment 14 Christoph Cullmann 2019-08-24 15:42:08 UTC
*** Bug 387428 has been marked as a duplicate of this bug. ***
Comment 15 Christoph Cullmann 2019-09-20 19:33:36 UTC
*** Bug 412095 has been marked as a duplicate of this bug. ***
Comment 16 Christoph Cullmann 2019-09-29 17:51:37 UTC
For me this works, if the font is chosen properly.

If you choose a font that will lead to some fallback for some characters and these have different heights, this fails.

If somebody has a proper fix for that case that doesn't lead to rendering artifacts between lines, this can be re-opened. Otherwise I consider this fixed.
Comment 17 postix 2020-05-04 09:33:56 UTC
*** Bug 415036 has been marked as a duplicate of this bug. ***
Comment 18 postix 2020-05-04 09:34:19 UTC
*** Bug 414408 has been marked as a duplicate of this bug. ***
Comment 19 postix 2020-05-04 09:38:56 UTC
There's another patch proposed: https://phabricator.kde.org/D25339