229270 - [xft] non-BMP Unicode character rendering broken

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Jungshik Shin]

I've just found that non-BMP (Unicode plane 1 and above) character rendering is
broken in 1.6 branch and the trunk. I went back to nsFontMetricsXft.cpp v 1.29
and went upward until I found the revision with regression. It's version 1.34
[1] for bug 223813 that broke the plane 1 rendering. I haven't yet figured out 
how it could broke the non-BMP Unicode character rendering.  

I think we have to fix this before releasing 1.6. It may not be important to
everyday use, yet, but it has a high 'geek' value. Besides, some Chinese people
may have some practical reasons for non-BMP character support (plane 2).

Tex's  page given in the URL field have links to fonts (Code2001 and other two
fonts) to install to test the demo page. Code2001 alone should be enough for
testing. 

[1]
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsFontMetricsXft.cpp&branch=&root=/cvsroot&subdir=mozilla/gfx/src/gtk&command=DIFF_FRAMESET&rev1=1.33&rev2=1.34

Comment 1

[Jungshik Shin]

I figured it out. |FindFont (PRUnichar)| (that used be called only with 'a'
before 1.34) is now called with actual Unicode characters to render. PRUnichar
(16bit short) is too narrow to cover the full range of Unicode characters
(21bit). We have to use PRUint32 (or FcChar32). 

Comment 2

[Jungshik Shin]

Attachment: patch

4-liner !

Comment 3

[Jungshik Shin]

Comment on attachment 137879 [details] [diff] [review]
patch

asking for r/sr.

bryner is away so that I'm trying blizzard and dbaron.

Comment 4

[Jungshik Shin]

FYI, gcc (and I guess the same is true of g++) doesn't warn (even with -Wall and
-pedantic) when numeric values are truncated. There's a bug filed on this at the
gcc bugzilla.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=2707

Comment 5

[Jungshik Shin]

setting the milestone to 1.6final

Comment 6

[Jungshik Shin]

Comment on attachment 137879 [details] [diff] [review]
patch

bryner, can you review? 

I'd like to get this in for 1.6 (and firebird 0.8 as well)..

Comment 7

[Jungshik Shin]

Comment on attachment 137879 [details] [diff] [review]
patch

thanks for r/sr. fix checked into the trunk.
 asking for a1.6

This patch fixes a broken non-BMP character  (Unicode characters above U+FFFF)
rendering. It worked in 1.4 but was regressed in 1.5. With the increasing
number of non-BMP characters added to Unicode and   a few good free non-BMP
fonts available, leaving this broken in 1.6 would be  bad in terms of 'PR'
because non-BMP support is  tested frequently in 'I18N benchmarks'.

Platform : Linux/*BSD and other Unix with Xft build (not a default build)
Risk : very very low. It's  a trivial fix (number truncating error that should
have been caught by g++ but was not because of a bug in g++)

P.S bryner, can I check in this for firebird 0.8 branch as well?

Comment 8

[chris hofmann]

Comment on attachment 137879 [details] [diff] [review]
patch

a=chofmann for 1.6

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 137879 [details] [diff] [review]
patch


Check in: { 01/04/2004 19:33	jshin%mailaps.org }
Check in: { 01/06/2004 16:20	jshin%mailaps.org	MOZILLA_1_6_BRANCH }

Comment 7:
{
P.S bryner, can I check in this for firebird 0.8 branch as well?
}

Comment 10

[chris hofmann]

should get sucked in to firebird branch on the next update...

marking fixed