[Resending with smaller attachments] On Wed, 2012-07-25 at 16:21 -0700, Behdad Esfahbod wrote: > src/hb-old.cc | 51 > +++++++++++++++++++++++++++++++++---------- > src/hb-old/harfbuzz-shaper.h | 1 > src/hb-uniscribe.cc | 5 ++-- > 3 files changed, 44 insertions(+), 13 deletions(-) > > New commits: > commit 91e721ea8693205f4f738bca97a5055ee75cf463 > Author: Behdad Esfahbod <beh...@behdad.org> > Date: Wed Jul 25 19:20:34 2012 -0400 > > [hb-old] Fix clusters > > Unlike its "documentation", hb-old's log_clusters are, well, indeed > logical, not visual. Fixup. Adapted / copied from hb-uniscribe.
This commit seems to have broken Sinhala rendering with FreeSerif font for the following file: http://git.savannah.gnu.org/cgit/sinhala.git/plain/patches/icu-sinhala-rendering.txt I have attached the output of both hb-view and hb-shape. hb-shape output has not changed. hb-view output is very different. The aforementioned patch actually does fix a rendering problem with the standalone consonant ක (ka) when using the FreeSerif font. But breaks කො (ko), කෝ (koo), කෞ (kau), ක්ර (kra), ක්ය (kya). cya, # > diff --git a/src/hb-old.cc b/src/hb-old.cc > index a3b5679..e828ca8 100644 > --- a/src/hb-old.cc > +++ b/src/hb-old.cc > @@ -251,6 +251,8 @@ _hb_old_shape (hb_font_t *font, > > buffer->guess_properties (); > > + bool backward = HB_DIRECTION_IS_BACKWARD (buffer->props.direction); > + > #define FAIL(...) \ > HB_STMT_START { \ > DEBUG_MSG (OLD, NULL, __VA_ARGS__); \ > @@ -285,23 +287,23 @@ retry: > pchars[chars_len++] = 0xDC00 + ((c - 0x10000) & ((1 << 10) - 1)); > } > } > -#undef utf16_index > > > #define ALLOCATE_ARRAY(Type, name, len) \ > name = (Type *) scratch; \ > - scratch += len * sizeof (name[0]); \ > - scratch_size -= len * sizeof (name[0]); > + scratch += (len) * sizeof ((name)[0]); \ > + scratch_size -= (len) * sizeof ((name)[0]); > > > HB_ShaperItem item = {0}; > > ALLOCATE_ARRAY (const HB_UChar16, item.string, chars_len); > + ALLOCATE_ARRAY (unsigned short, item.log_clusters, chars_len + 2); > item.stringLength = chars_len; > item.item.pos = 0; > item.item.length = item.stringLength; > item.item.script = hb_old_script_from_script (buffer->props.script); > - item.item.bidiLevel = HB_DIRECTION_IS_FORWARD (buffer->props.direction) ? > 0 : 1; > + item.item.bidiLevel = backward ? 1 : 0; > > item.font = old_font; > item.face = old_face; > @@ -314,14 +316,17 @@ retry: > sizeof (HB_GlyphAttributes) + > sizeof (HB_Fixed) + > sizeof (HB_FixedPoint) + > - sizeof (unsigned short)); > + sizeof (uint32_t)); > > item.num_glyphs = num_glyphs; > ALLOCATE_ARRAY (HB_Glyph, item.glyphs, num_glyphs); > ALLOCATE_ARRAY (HB_GlyphAttributes, item.attributes, num_glyphs); > ALLOCATE_ARRAY (HB_Fixed, item.advances, num_glyphs); > ALLOCATE_ARRAY (HB_FixedPoint, item.offsets, num_glyphs); > - ALLOCATE_ARRAY (unsigned short, item.log_clusters, num_glyphs); > + uint32_t *vis_clusters; > + ALLOCATE_ARRAY (uint32_t, vis_clusters, num_glyphs); > + > +#undef ALLOCATE_ARRAY > > if (!HB_ShapeItem (&item)) > return false; > @@ -335,24 +340,48 @@ retry: > } > num_glyphs = item.num_glyphs; > > -#undef ALLOCATE_ARRAY > + /* Ok, we've got everything we need, now compose output buffer, > + * very, *very*, carefully! */ > + > + /* Calculate visual-clusters. That's what we ship. */ > + for (unsigned int i = 0; i < num_glyphs; i++) > + vis_clusters[i] = -1; > + for (unsigned int i = 0; i < buffer->len; i++) { > + uint32_t *p = > &vis_clusters[item.log_clusters[buffer->info[i].utf16_index()]]; > + *p = MIN (*p, buffer->info[i].cluster); > + } > + if (!backward) { > + for (unsigned int i = 1; i < num_glyphs; i++) > + if (vis_clusters[i] == -1) > + vis_clusters[i] = vis_clusters[i - 1]; > + } else { > + for (int i = num_glyphs - 2; i >= 0; i--) > + if (vis_clusters[i] == -1) > + vis_clusters[i] = vis_clusters[i + 1]; > + } > + > +#undef utf16_index > > + buffer->ensure (num_glyphs); > + if (buffer->in_error) > + FAIL ("Buffer in error"); > + > + > + buffer->len = num_glyphs; > hb_glyph_info_t *info = buffer->info; > for (unsigned int i = 0; i < num_glyphs; i++) > { > info[i].codepoint = item.glyphs[i]; > - info[i].cluster = item.log_clusters[i]; > + info[i].cluster = vis_clusters[i]; > > info[i].mask = item.advances[i]; > info[i].var1.u32 = item.offsets[i].x; > info[i].var2.u32 = item.offsets[i].y; > } > - buffer->len = num_glyphs; > > buffer->clear_positions (); > > - unsigned int count = buffer->len; > - for (unsigned int i = 0; i < count; ++i) { > + for (unsigned int i = 0; i < num_glyphs; ++i) { > hb_glyph_info_t *info = &buffer->info[i]; > hb_glyph_position_t *pos = &buffer->pos[i]; > > diff --git a/src/hb-old/harfbuzz-shaper.h b/src/hb-old/harfbuzz-shaper.h > index 3f32d47..ab65004 100644 > --- a/src/hb-old/harfbuzz-shaper.h > +++ b/src/hb-old/harfbuzz-shaper.h > @@ -251,6 +251,7 @@ struct HB_ShaperItem_ { > HB_Fixed *advances; /* output: <num_glyphs> advances > */ > HB_FixedPoint *offsets; /* output: <num_glyphs> offsets > */ > unsigned short *log_clusters; /* output: for each output > glyph, the index in the input of the start of its logical cluster */ > + /* XXX the discription for log_clusters is wrong. It maps each input > position to output glyph position! */ > > /* internal */ > HB_Bool kerning_applied; /* output: true if kerning was > applied by the shaper */ > diff --git a/src/hb-uniscribe.cc b/src/hb-uniscribe.cc > index d41542b..6b9a261 100644 > --- a/src/hb-uniscribe.cc > +++ b/src/hb-uniscribe.cc > @@ -257,8 +257,8 @@ retry: > > #define ALLOCATE_ARRAY(Type, name, len) \ > Type *name = (Type *) scratch; \ > - scratch += len * sizeof (name[0]); \ > - scratch_size -= len * sizeof (name[0]); > + scratch += (len) * sizeof ((name)[0]); \ > + scratch_size -= (len) * sizeof ((name)[0]); > > #define utf16_index() var1.u32 > > @@ -294,6 +294,7 @@ retry: > ALLOCATE_ARRAY (GOFFSET, offsets, glyphs_size); > ALLOCATE_ARRAY (uint32_t, vis_clusters, glyphs_size); > > +#undef ALLOCATE_ARRAY > > #define MAX_ITEMS 256 > > _______________________________________________ > HarfBuzz mailing list > HarfBuzz@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/harfbuzz
<<attachment: sinhala-old-hb-FreeSerif-broken.png>>
[sinh_ka_al=0+915] [ka_sinh=0+915] [ka_sinh=0+915|aa2_sinh=0+336] [ka_sinh=0+915|ae2_sinh=0+326] [ka_sinh=0+915|aee2_sinh=0+326] [sinh_ka_i2=0+915] [sinh_ka_ii2=0+915] [sinh_ka_u2=0+915] [sinh_ka_uu2=0+915] [ka_sinh=0+915|ru2_sinh=0+336] [e2_sinh=0+707|ka_sinh=0+915] [e2_sinh=0+707|sinh_ka_al=0+915] [ai2_sinh=0+1364|ka_sinh=0+915] [e2_sinh=0+707|ka_sinh=0+915|aa2_sinh=0+336] [e2_sinh=0+707|ka_sinh=0+915|aa2_sinh=0+336|al_sinh=0] [e2_sinh=0+707|ka_sinh=0+915|lu2_sinh=0+472] [ka_sinh=0+915|lu2_sinh=0+472] [ka_sinh=0+915|ruu2_sinh=0+696] [ka_sinh=0+915|llu2_sinh=0+492] [ka_sinh=0+915|ng_sinh=0+260] [ka_sinh=0+915|h2_sinh=0+260] [ka_sinh=0+915|rak1.sinh=0] [ka_sinh=0+915|sinh_yan=0+508] [sinh_ma.rep=0+736]
<<attachment: sinhala-old-hb-FreeSerif-working.png>>
[sinh_ka_al=0+915] [ka_sinh=0+915] [ka_sinh=0+915|aa2_sinh=0+336] [ka_sinh=0+915|ae2_sinh=0+326] [ka_sinh=0+915|aee2_sinh=0+326] [sinh_ka_i2=0+915] [sinh_ka_ii2=0+915] [sinh_ka_u2=0+915] [sinh_ka_uu2=0+915] [ka_sinh=0+915|ru2_sinh=0+336] [e2_sinh=0+707|ka_sinh=0+915] [e2_sinh=0+707|sinh_ka_al=0+915] [ai2_sinh=0+1364|ka_sinh=0+915] [e2_sinh=0+707|ka_sinh=0+915|aa2_sinh=0+336] [e2_sinh=0+707|ka_sinh=0+915|aa2_sinh=0+336|al_sinh=0] [e2_sinh=0+707|ka_sinh=0+915|lu2_sinh=0+472] [ka_sinh=0+915|lu2_sinh=0+472] [ka_sinh=0+915|ruu2_sinh=0+696] [ka_sinh=0+915|llu2_sinh=0+492] [ka_sinh=0+915|ng_sinh=0+260] [ka_sinh=0+915|h2_sinh=0+260] [ka_sinh=0+915|rak1.sinh=0] [ka_sinh=0+915|sinh_yan=0+508] [sinh_ma.rep=0+736]
_______________________________________________ HarfBuzz mailing list HarfBuzz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/harfbuzz