288836 - Update Bidi data tables to Unicode 4.1.0

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Simon Montagu :smontagu]

This just requires regenerating data files (with some minor changes to the perl
scripts)

Comment 1

[Simon Montagu :smontagu]

Attachment: patch

I'm not sure why BidiMirroring.txt is even in our tree; maybe I should just cvs
remove it.

Comment 2

[Jungshik Shin]

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

If PRUint16 (instead of PRUint8) has to be used (because some symmetry pairs
are far apart from each other), I guess we don't have to bother to use
exclusive-OR (provided that no mirroring is necessary for non-BMP characters)

Btw, what do you think of using a single 2-d array for symmtable? In that case,
a 256-element PRUint8 array (again, assuming that only BMP characters need
mirroing is necessary to map |(u & 0xFFFFFF00) >> 8| to the 1st index of 2-d
symmtable. Well, it may not save us much...

Comment 3

[Simon Montagu :smontagu]

(In reply to comment #2)
> (From update of attachment 179460 [details] [diff] [review] [edit])
> If PRUint16 (instead of PRUint8) has to be used (because some symmetry pairs
> are far apart from each other), I guess we don't have to bother to use
> exclusive-OR (provided that no mirroring is necessary for non-BMP characters)

I'm sorry, I'm not sure that I follow this. Do you mean that the table could
just contain the mirrored characters themselves?

> Btw, what do you think of using a single 2-d array for symmtable? In that case,
> a 256-element PRUint8 array (again, assuming that only BMP characters need
> mirroing is necessary to map |(u & 0xFFFFFF00) >> 8| to the 1st index of 2-d
> symmtable. Well, it may not save us much...
> 

That's probably a good idea, considering that the number of blocks with mirrored
forms is growing.

Comment 4

[Jungshik Shin]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 179460 [details] [diff] [review] [edit] [edit])
> > If PRUint16 (instead of PRUint8) has to be used (because some symmetry pairs
> > are far apart from each other), I guess we don't have to bother to use
> > exclusive-OR (provided that no mirroring is necessary for non-BMP characters)
> 
> I'm sorry, I'm not sure that I follow this. Do you mean that the table could
> just contain the mirrored characters themselves?

Yes, that's what I meant. I thought exclusive-OR was used to save memory (and
'add' operation - symmtable_20[u & 0xff] + 0x2000), but I may be missing something. 

Comment 5

[Simon Montagu :smontagu]

Attachment: new patch

Comment 6

[Jungshik Shin]

Comment on attachment 179476 [details] [diff] [review]
new patch


>+  if (!(u & 0xFFFF0000)) {

Wouldn't |u < 0x10000| be clearer? 

>+    PRUint8 index = symmtable_index[(u & 0xFFFFFF00) >> 8];
>+    if (index && symmtable[index - 1] [u & 0xFF]) {
>+      return symmtable[index - 1] [u & 0xFF];
>+    }
>   }
>   return u;
> }

Although not so convenient for checking the table by quick inspection, treating
'self-mirrored' characters the same way as others (that is, storing their own
code points in |symmtable|) can save |&& symmtable[index - 1] [u & 0xFF]|. 

With that, r=jshin

Comment 7

[Simon Montagu :smontagu]

> Although not so convenient for checking the table by quick inspection, treating
> 'self-mirrored' characters the same way as others (that is, storing their own
> code points in |symmtable|) can save |&& symmtable[index - 1] [u & 0xFF]|. 

I thought it would make the tables more or less incomprehensible, but I guess we
don't need to care too much about human readability. New patch coming up.

Comment 8

[Simon Montagu :smontagu]

Attachment: With suggested changes

Comment 9

[Simon Montagu :smontagu]

Requesting blocking 1.8b2 pending sr

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 179486 [details] [diff] [review]
With suggested changes

rs=roc

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 179486 [details] [diff] [review]
With suggested changes

a=mkaply

Comment 12

[Simon Montagu :smontagu]

Checked in.