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:
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.
Created attachment 886 [details] proposed patch
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
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.
Created attachment 890 [details] proposed patch - 2
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
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 )
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.
Created attachment 897 [details] proposed patch - 3
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