Bug 53713 - double spaced on some of the font
Summary: double spaced on some of the font
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: general (show other bugs)
Version: 1.2
Platform: RedHat Enterprise Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-01-31 05:26 UTC by llch
Modified: 2003-03-11 18:07 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
proposed patch (1.78 KB, patch)
2003-02-05 07:40 UTC, llch
Details
proposed patch - 2 (2.68 KB, patch)
2003-02-05 14:04 UTC, llch
Details
proposed patch - 3 (5.32 KB, patch)
2003-02-07 01:28 UTC, llch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description llch 2003-01-31 05:26:12 UTC
Version:           1.2 (using KDE KDE 3.0.99)
Installed from:    RedHat RPMs
OS:          Linux

Description of problem: 
Popular CJK Font like gulim (KR) and ZYSong18030 (CN) has doubled spaced for fonts only in konsole, not in other QT/KDE application. Other font like Mingtl (TW) and Mincho Gothic (JP) is okay.  
 
Version-Release number of selected component (if applicable): 
 
 
How reproducible: 
everytime 
 
Steps to Reproduce: 
1. run konsole in ko_KR.eucKR and zh_CN.GB18030 
 
Actual results: 
Double spaced or strange spaced 
 
Expected results: 
spaced 
 
Additional info:
Comment 1 llch 2003-02-05 07:39:48 UTC
What konsole trying to do is first it gets the maximum width from ASCII 
range and use that for every character. However beacuse konsole updates 
the string from QPaint.drawText() which uses different width for different 
glyphs, so that will create generate different spaces on the screen. 
 
What I did on the following patch is getting total width english glyphs and symbols 
and get the width by avg. it. Also did a "cell drawing" by drawing each character 
separately to a cell width. 
 
Following is the proposed patch. 
Comment 2 llch 2003-02-05 07:40:22 UTC
Created attachment 886 [details]
proposed patch
Comment 3 Waldo Bastian 2003-02-05 12:24:45 UTC
Subject: Re: [Konsole-devel]  double spaced on some of the font

On Wednesday 05 February 2003 07:39, llch@redhat.com wrote:
> What I did on the following patch is getting total width english glyphs and
> symbols and get the width by avg. it. 

Shouldn't it use the maximum? You want that the whole character fits your cell 
for all (single width) characters. If you average, you pretty much get that 
half of the chars fit and the other half doesn't.

> Also did a "cell drawing" by drawing each character separately to a cell
> width.

Thanks, something like this is indeed needed. How bad does this suck in terms 
of performance? 

Note that "use double width if not ascii" is not correct, we keep track 
internally of characters that are supposed to be double width and that 
information should be used instead.

E.g. setImage knows when a character is double width, because the next 
character will be 0. What might work is to pass fragments to drawAttrStr that 
consist of either only single width or only double width characters and add a 
bool isDoubleWidth to drawAttrStr. Then drawAttrStr can do "cell drawing" 
using the specified width.

It would be nice if you could make a patch based on the above, if not I will 
have a go at it myself in the second half of february.

Cheers,
Waldo
Comment 4 llch 2003-02-05 14:03:12 UTC
Subject: Re:  double spaced on some of the font         

On 5 Feb 2003, Waldo Bastian wrote:
> Shouldn't it use the maximum? You want that the whole character fits your cell 
> for all (single width) characters. If you average, you pretty much get that 
> half of the chars fit and the other half doesn't.

Popular portorional font like ZYSong18030 and Baekmuk Gulim has width from
7 to 12 even in english/symbol glyphs. So for example when using one of
the font with getting the max. width, the display will seen as double
spaced. I think it is between a trade off of getting max width to have
huge space on some portorional font, or getting avg width to have tight
space on some of portorional font. Of course it would be great if we could
think of other solutions.

> 
> Thanks, something like this is indeed needed. How bad does this suck in terms 
> of performance? 

I haven't notice any performance loses in my PIII machine. Any performance 
measurement recommend for me to do on konsole?

> 
> Note that "use double width if not ascii" is not correct, we keep track 
> internally of characters that are supposed to be double width and that 
> information should be used instead.
> 
> E.g. setImage knows when a character is double width, because the next 
> character will be 0. What might work is to pass fragments to drawAttrStr that 
> consist of either only single width or only double width characters and add a 
> bool isDoubleWidth to drawAttrStr. Then drawAttrStr can do "cell drawing" 
> using the specified width.
> 
> It would be nice if you could make a patch based on the above, if not I will 
> have a go at it myself in the second half of february.
> 

Right agreed. Please see the following patch. I made it to pass a cell at
a time so that it will know when to render a double width characters, as
well then we can omit the loop in drawAttrStr(). So far so good remotely,
however I will get the performance review done locally tomorrow.

Comment 5 llch 2003-02-05 14:04:57 UTC
Created attachment 890 [details]
proposed patch - 2
Comment 6 Waldo Bastian 2003-02-05 16:48:54 UTC
Subject:  double spaced on some of the font

> I think it is between a trade off of getting max width to have
> huge space on some portorional font, or getting avg width to have tight
> space on some of portorional font.

Well, that's the reason that we use fixed fonts for konsole. If you use a 
width that is too small for some of the glyphs the glyphs either get clipped 
or they get painted in adjoining cells, the latter results in garbage on the 
screen when scrolling or editing.

Hmm.. maybe can we prevent the garbage by drawing one cell more than strictly 
needed.

> I haven't notice any performance loses in my PIII machine. Any performance
> measurement recommend for me to do on konsole?

ls -lR | head -25000 > /tmp/largefile
time cat /tmp/largefile

You may want to disable the history so that it doesn't influence the 
measurement. 

Cheers,
Waldo
Comment 7 Waldo Bastian 2003-02-06 17:35:36 UTC
Subject: Re: [Konsole-devel]  double spaced on some of the font

On Wednesday 05 February 2003 14:03, llch@redhat.com wrote:
> I haven't notice any performance loses in my PIII machine. Any performance
> measurement recommend for me to do on konsole?

I have compared the time it takes to scroll 25000 lines.
Without patch: 3.1 sec
With patch: 6.3 sec

(Patch: http://bugs.kde.org/attachment.cgi?id=890&action=view )

Comment 8 llch 2003-02-07 01:26:31 UTC
I have done another patch based on two paths: 
- If we have same width across the representative characters, we will use the 
"default" drawing method 
- If we don't have same width across the representative characters, we will use 
"cell" drawing method 
 
I have also include some optimizations back so the new patch tends to go faster. 
 
Here are the benchmarks for scroll 25000 lines: 
zh_CN.GB18030 - using proportional font 
Orginal konsole: 7.878s 
Old Patched konsole: 9.784s 
New Patched konsole: 9.092s 
 
en_US.UTF-8 - using monospaced font 
Orginal konsole: 8.186s 
Old Patched konsole: 10.432s 
New Patched konsole: 8.582s 
 
Following attachment is the new patch. 
 
Comment 9 llch 2003-02-07 01:28:06 UTC
Created attachment 897 [details]
proposed patch - 3
Comment 10 Waldo Bastian 2003-03-11 18:07:38 UTC
Subject: kdebase/konsole/konsole

CVS commit by waba: 

Support for non-fixed fonts.
Based on a patch by llch@redhat.com
CCMAIL: 53713-done@bugs.kde.org


  M +103 -24   TEWidget.cpp   1.182
  M +1 -1      TEWidget.h   1.68