D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  I'm not sure if this is the right way to do it or it might cause any glitch,  
but here we go. Upon line rendering, update the maximum height we've ever seen.
  
  Though line height won't shrink during the edit phase, it will back to the 
actual value upon save.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 82170.
xuetianweng added a comment.


  Use actual line height instead of representitive character.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80607=82170

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp
  src/render/katerenderer.h
  src/view/kateview.h

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Frederick Yin]

fakefred added a comment.


  In D25339#665351 , @xuetianweng 
wrote:
  
  > ... or you could give me some hint on that?
  
  
  Too bad I can't; it's been no more than a month since I began my study into 
KDE applications. I'd love to test out if someone provides a genius solution, 
though.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#663915 , @fakefred wrote:
  
  > I second the as-an-option proposal. Hey, why not automatically increase the 
line height when CJK characters are detected?
  
  
  So the thing is we need to maintain a such height depending on the document, 
not so sure if that is even possible (or whether it could be costly) or you 
could give me some hint on that?
  Actually, I'd rather to calculate the height based on actual character in the 
document because during my test, mixing Arabic together to CJK and latin could 
still have problem (likely same for Cyrillic).

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Sven Brauch]

brauch added a comment.


  In D25339#663915 , @fakefred wrote:
  
  > I second the as-an-option proposal. Hey, why not automatically increase the 
line height when CJK characters are detected?
  
  
  This issue has been around for years and actually yeah, I think that is the 
only viable solution unless somebody dives in and reworks the renderer to 
support variable line heights.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: brauch, sars, pshinjo, rjvbb, fakefred, anthonyfieroni, 
kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Frederick Yin]

fakefred added a comment.


  I second the as-an-option proposal. Hey, why not automatically increase the 
line height when CJK characters are detected?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: sars, pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Shinjo Park]

pshinjo added a comment.


  In D25339#663833 , @rjvbb wrote:
  
  > doesn't it give you US-ASCII canonical representations of every possible 
glyph?
  
  
  Whether it is possible or not, no CJK users will write TeX document using 
US-ASCII representation for all glyphs instead of their native alphabet, 
especially when the document is in their mother tongue. Welcome to the world of 
transliteration problems.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: sars, pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[René J . V . Bertin]

rjvbb added a comment.


  >   It's "KTextEditor", not "KCodeEditor". 
  
  Yes, but look at the traditional meaning of a text editor, which typically 
means "plain text" editor. KTextEditor's design decision to use a single 
lineheight puts it squarely in that domain - to reply in style: `It's 
"TKextEditor", not "KRichTextEditor" (and even less "KWordProcessor")` ...
  
  > write CJK LaTeX articles in Kile
  
  Like it or not, TeX is code (or a universal language, depending on how you 
look at it). That's intentional (WYSIWIG vs. WYSIAYG and all that). And 
frankly, TeX code is the last place where I'd expect this kind of rendering 
problem: doesn't it give you US-ASCII canonical representations of every 
possible glyph? (I know that doesn't help manuscript readability, but hey, 
that's the choice you make :) ) One could also make the argument that maybe 
Kile should look for another rendering engine if they want to support something 
more advanced than KTextEditor can offer. There are Qt  classes that have "rich 
text" support which might be more appropriate (if they offer editing support), 
and on the other end of the scale there's the Calligra project which probably 
has the required libraries.
  
  Anyway, serving a worldwide userbase isn't a new goal, but also shouldn't 
IMHO drag down usability to some lowest common denominator IMHO. Hence my firm 
request to make any chance that does have that effect optional, one way or 
another - I do agree with @sars .

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: sars, pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Kåre Särs]

sars added a comment.


  I'm starting to think that we need an option for enabling/disabling this 
change/feature. I would not want to have the extra space between the lines, but 
at the same time I can see that actually not seeing the whole character is not 
an acceptable situation...

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: sars, pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Frederick Yin]

fakefred added a comment.


  In D25339#663322 , @rjvbb wrote:
  
  > but that's hardly the domain of application for a text editing framework.
  
  
  It's "KTextEditor", not "KCodeEditor". This rendering issue, though not from 
an "intended purpose", makes it painful to write CJK LaTeX articles in Kile. 
Now that KDE declares to be serving a worldwide userbase, the foundation of 
numerous applications has to see some change somehow.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Shinjo Park]

pshinjo added a comment.


  In D25339#663462 , @rjvbb wrote:
  
  > This patch is only needed when mixing a main Latin1 (like) alphanumeric 
font with occasional glyphs from a font that have a different, taller height?
  
  
  Yes, though the "occasion" will highly depend on the user locale. For someone 
this will likely never happen, for someone this will happen everyday.
  
  > Am I right that any text that uses only a single font will see some form of 
significant loss of the number of lines that fit within a given vertical space?
  
  Yes, given that the font contains all required glyphs to represent the string 
"AHIygあ你말" is limited. That's also visible in my test 2. Previous tests were 
done in artificially high font size (32pt for example) and less dense texts so 
the original authors might overlooked this problem.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[René J . V . Bertin]

rjvbb added a comment.


  This patch is only needed when mixing a main Latin1 (like) alphanumeric font 
with occasional glyphs from a font that have a different, taller height?
  
  Am I right that any text that uses only a single font will see some form of 
significant loss of the number of lines that fit within a given vertical space?
  
  >   Test 3 - Ubuntu Mono + Noto Sans Mono CJK KR 9pt. For me the right hand 
side is a regression, since neither Cyrillic nor Korean scripts are clearly 
visible when both of them are in the same line. Also in this setup most of 
Korean texts are cut by half even when there are only Korean texts. Compare it 
with the other lines where only Cyrillic is visible:...
  
  
  
  > But this part is the raison d'etre for this patch: provide enough font 
height for non-latin-1 text. Also not considered is glyphs from other scripts, 
such as Cyrillic, Arabic, ... What could be another solution?
  
  If local lineheight adaptation isn't possible with the current implementation 
(not even by inserting 1 or more virtual linebreaks?) then the behaviour 
introduced with this patch c/should become optional. Selected automatically if 
possible (with an off toggle in the settings dialog) but otherwise via a menu 
action that's not too well hidden so people who need to turn it on and off on 
any kind of regular basis can do so without having to hunt the command down too 
much.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Shinjo Park]

pshinjo added a comment.


  In D25339#563370 , @xuetianweng 
wrote:
  
  > And what I'd like to point is, for CJK users, it is uncommon for them to 
select a single font to cover all the characters, because such fonts are really 
rare. People usually select one latin only font and just let system 
(fontconfig) select the fallback for them.
  
  
  This statement is valid even for non-CJK users who only occasionally see the 
said characters.
  
  For all screenshots, left hand side is git master+this patch, right hand side 
is Kate 19.12.3/Frameworks 5.68.0 on Ubuntu 20.04.
  
  Test 1 - Hack + Noto Sans Mono CJK KR 9pt (Hack does not contain CJK glyphs, 
so for missing glyphs the latter will be used). Notice that only in the left 
hand side the underline in my e-mail address is visible:
  
  F8284453: Screenshot_20200504_210144.png 

  
  Test 2 - Nanum Square 9pt (contains both Latin-1 and Korean glyphs):
  
  F8284455: Screenshot_20200504_210313.png 

  
  Test 3 - Ubuntu Mono + Noto Sans Mono CJK KR 9pt. For me the right hand side 
is a regression, since neither Cyrillic nor Korean scripts are clearly visible 
when both of them are in the same line. Also in this setup most of Korean texts 
are cut by half even when there are only Korean texts. Compare it with the 
other lines where only Cyrillic is visible:
  
  F8284485: Screenshot_20200504_213506.png 

  
  I can also confirm about 6-7 lines of loss, but for me the right hand side 
has issues. Need to find some compromise.

INLINE COMMENTS

> katerenderer.cpp:1040
>  m_fontHeight = qMax(1, qCeil(m_fontMetrics.ascent() + 
> m_fontMetrics.descent()));
> +// Try to use representitve character from English, Chinese, Japanese
> +// and Korean to calculate the line height.

I think this causes the regression mentioned in

In D25339#663322 , @rjvbb wrote:

> I can't speak for the special cases where this change would improve matters, 
> but for me it introduces a clear regression (waste of vertical space: 12 
> lines less) in a basic ascii code editing context. Font used is Ubuntu Mono 
> 10pt.


But this part is the raison d'etre for this patch: provide enough font height 
for non-latin-1 text. Also not considered is glyphs from other scripts, such as 
Cyrillic, Arabic, ... What could be another solution?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: pshinjo, rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, 
kwrite-devel, rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[René J . V . Bertin]

rjvbb requested changes to this revision.
rjvbb added a comment.
This revision now requires changes to proceed.


  I can't speak for the special cases where this change would improve matters, 
but for me it introduces a clear regression (waste of vertical space: 12 lines 
less) in a basic ascii code editing context. Font used is Ubuntu Mono 10pt.
  
  F8284297: image.png 
  
  Granted, the current display (left) is a tad crowded but I actually like it 
like that for code editing. For reading prose it might be too much, but that's 
hardly the domain of application for a text editing framework.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks, rjvbb
Cc: rjvbb, fakefred, anthonyfieroni, kde-frameworks-devel, kwrite-devel, 
rrosch, LeGast00n, cblack, domson, michaelh, ngraham, bruns, demsking, 
cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Frederick Yin]

fakefred added a comment.


  So I tested this, great improvements regarding line height can be seen: 
  F8283136: kate_1.png 
  
  在使用本身就是 CJK 的字体时，显示仍然正常。
  F8283141: kate_2.png 

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks
Cc: fakefred, anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, 
LeGast00n, cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, 
dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Christoph Cullmann]

cullmann added a reviewer: Frameworks.
cullmann added a comment.


  I tried the current version.
  For me this looks OK now.
  Thought I would like to have more people trying this out before we merge.
  Some volunteers?
  
  Beside this: Thanks for working on this issue.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann, #frameworks
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80607.
xuetianweng added a comment.


  Fix the blank text issue by alway setLineWidth.
  
  For QTextLine, the layout information is not filled until certain function is 
called.
  If dynamic wrap is not enabled, the line width is -1 so at the point we do 
the calculation,
  we will have no size information of the text. Use INT_MAX to set the 
lineWidth as workaround.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80606=80607

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80606.
xuetianweng added a comment.


  Clean up the code a little bit and adding comments. Also fix a small bug if 
m_fontHeight has 
  big difference with m_fontMetrics's height.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80444=80606

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#651701 , @cullmann wrote:
  
  > Hmm, after applying this patch, for me, no text is visible at all.
  >  By selecting a bit stuff, one at least sees an outline (CMakeLists.txt of 
ktexteditor toplevel dir).
  >
  > F8246692: kwrite_no_text.png 
  
  
  Are you sure it is the latest patch? I know some old version in this patch 
might have this effect.
  What's your color & font in kwrite setup BTW?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#651701 , @cullmann wrote:
  
  > Hmm, after applying this patch, for me, no text is visible at all.
  >  By selecting a bit stuff, one at least sees an outline (CMakeLists.txt of 
ktexteditor toplevel dir).
  >
  > F8246692: kwrite_no_text.png 
  
  
  let me test with some different colorscheme..

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Christoph Cullmann]

cullmann added a comment.


  Hmm, after applying this patch, for me, no text is visible at all.
  By selecting a bit stuff, one at least sees an outline (CMakeLists.txt of 
ktexteditor toplevel dir).
  
  F8246692: kwrite_no_text.png 

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Christoph Cullmann]

cullmann added a comment.


  I appreciate work on this issue.
  I am not sure about how well this "hack" will solve the issue, thought.
  I will give it a try here in any case.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Anthony Fieroni]

anthonyfieroni added a comment.


  +1, looks great.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80444.
xuetianweng added a comment.


  Fix the handling when layout formats has background.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80438=80444

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  F8244684: Screenshot_20200417_222110.png 

  
  Add a screenshot to demostrate the change.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  In D25339#563937 , @cullmann wrote:
  
  > Actually, I could live with:
  >
  > 1. All lines are a bit higher, for me that makes reading even easier. But 
the rendering shall have no glitches.
  > 2. Some lines have different heights. But I assume this is hard to 
implement at the moment.
  
  
  After some experiment and reading the Qt code, seems there is no easy way to 
extend the line height..
  E.g. QPlainTextEdit will show the text with different line height, so I 
believe I couldn't do it with in Qt.
  
  So I tried to use a small trick to fill the gap.
  If there is some gap need to be filled, then the code tried to draw some 
transparent text so the background will be extended to fill the gap. Then draw 
the real text.
  
  The solution is hacky though.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80438.
xuetianweng added a comment.


  handle the range with multiple lines.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=80433=80438

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 80433.
xuetianweng added a comment.


  Try to fill the gap if we increase the line height with representitive char.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=69848=80433

BRANCH
  arcpatch-D25339

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, rrosch, LeGast00n, 
cblack, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Christoph Cullmann]

cullmann added a comment.


  Actually, I could live with:
  
  1. All lines are a bit higher, for me that makes reading even easier. But the 
rendering shall have no glitches.
  2. Some lines have different heights. But I assume this is hard to implement 
at the moment.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D25339#563822 , @xuetianweng 
wrote:
  
  > Having different font height for every line?
  
  
  
  
  1. We don't want a bigger lines
  2. We don't want a lines that are bigger that other in a view.
  3. We don't want different font height in a view.
  
  I don't know which is best.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: anthonyfieroni, kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  Any idea about how konsole derive
  
  In D25339#563656 , @cullmann wrote:
  
  > The issue with this approach is, that you get "too high" lines for the case 
that none of this characters ever appear.
  >  Beside that, unfortunately, you get rendering artifacts in addition, as Qt 
will not paint e.g. selection high enough for lines without such chars.
  >
  > See e.g. attached screenshot.F7768081: rendering_issue.png 

  
  
  Then what is the ideal solution to you for this case?
  
  Having different font height for every line? Or keep this "slightly higher 
line" by do vertical center align  and fix the artifact?
  
  At least the current state doesn't look right to me and I think it need to be 
fixed.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Christoph Cullmann]

cullmann added a comment.


  The issue with this approach is, that you get "too high" lines for the case 
that none of this characters ever appear.
  Beside that, unfortunately, you get rendering artifacts in addition, as Qt 
will not paint e.g. selection high enough for lines without such chars.
  
  See e.g. attached screenshot.F7768081: rendering_issue.png 


REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added reviewers: cullmann, dhaumann.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor, cullmann, dhaumann
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng added a comment.


  F7766842: Screenshot_20191116_092819.png 

  
  Add screenshot to demonstrate the problem.
  
  And what I'd like to point is, for CJK users, it is uncommon for them to 
select a single font to cover all the characters, because such fonts are really 
rare. People usually select one latin only font and just let system 
(fontconfig) select the fallback for them.
  So this could be a common problem for them to see the "half character".

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D25339

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng updated this revision to Diff 69848.
xuetianweng added a comment.


  Add comment about the actual characters.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25339?vs=69847=69848

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

[Xuetian Weng]

xuetianweng created this revision.
xuetianweng added a reviewer: KTextEditor.
xuetianweng added projects: Kate, Frameworks.
xuetianweng requested review of this revision.

REVISION SUMMARY
  When a font has fallback, the fontHeight is broken can may cut the string
  in the middle in the line. Try to use some representive character from 
multiple
  languages to estimate the actual fontHeight.
  
  BUG: 404907

TEST PLAN
  Manually tested.

REPOSITORY
  R39 KTextEditor

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D25339

AFFECTED FILES
  src/render/katerenderer.cpp

To: xuetianweng, #ktexteditor
Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann