Closed Bug 288836 Opened 16 years ago Closed 16 years ago

Update Bidi data tables to Unicode 4.1.0

Categories

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

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smontagu, Assigned: smontagu)

References

()

Details

Attachments

(1 file, 2 obsolete files)

This just requires regenerating data files (with some minor changes to the perl
scripts)
Status: NEW → ASSIGNED
Summary: Update Bidi data tables to Unicodce 4.1.0 → Update Bidi data tables to Unicode 4.1.0
Attached patch patch (obsolete) — Splinter Review
I'm not sure why BidiMirroring.txt is even in our tree; maybe I should just cvs
remove it.
Attachment #179460 - Flags: review?(jshin1987)
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...
(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.
(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. 
Attached patch new patch (obsolete) — Splinter Review
Attachment #179460 - Attachment is obsolete: true
Attachment #179476 - Flags: review?(jshin1987)
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
Attachment #179476 - Flags: review?(jshin1987) → review+
> 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.
Attachment #179476 - Attachment is obsolete: true
Attachment #179486 - Flags: superreview?(roc)
Attachment #179486 - Flags: review+
Attachment #179460 - Flags: review?(jshin1987)
Requesting blocking 1.8b2 pending sr
Flags: blocking1.8b2?
Comment on attachment 179486 [details] [diff] [review]
With suggested changes

rs=roc
Attachment #179486 - Flags: superreview?(roc) → superreview+
Attachment #179486 - Flags: approval1.8b2?
Comment on attachment 179486 [details] [diff] [review]
With suggested changes

a=mkaply
Attachment #179486 - Flags: approval1.8b2? → approval1.8b2+
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: blocking1.8b2?
Resolution: --- → FIXED
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.