Skip to content

Commit

Permalink
[arabic] Implement Windows-1256 private shaping
Browse files Browse the repository at this point in the history
Bug 1045139 - The Arabic text with "MS Sans Serif" font is rendered bad
https://bugzilla.mozilla.org/show_bug.cgi?id=1045139

This is only enabled on Windows platforms, and requires support from
Uniscribe to work.  But for clients that do hook up to Uniscribe, this
fixes shaping of Windows-1256-encoded bitmap fonts like "MS Sans Serif".

The code and table together have just less than a 1kb footprint when
enabled.

UNTESTED.  I might even have broken regular Arabic fallback shaping.
  • Loading branch information
behdad committed Jul 30, 2014
1 parent bf96b1b commit a97f537
Show file tree
Hide file tree
Showing 4 changed files with 442 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ HBSOURCES += \
hb-ot-shape-complex-arabic.cc \
hb-ot-shape-complex-arabic-fallback.hh \
hb-ot-shape-complex-arabic-table.hh \
hb-ot-shape-complex-arabic-win1256.hh \
hb-ot-shape-complex-default.cc \
hb-ot-shape-complex-hangul.cc \
hb-ot-shape-complex-hebrew.cc \
Expand Down
2 changes: 1 addition & 1 deletion src/check-includes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ echo 'Checking that source files #include "hb-*private.hh" first (or none)'

for x in $HBSOURCES; do
test -f "$srcdir/$x" && x="$srcdir/$x"
grep '#.*\<include\>' "$x" /dev/null | head -n 1
grep '#.*\<include\>' "$x" /dev/null | grep -v 'include _' | head -n 1
done |
grep -v '"hb-.*private[.]hh"' |
grep -v 'hb-private[.]hh:' |
Expand Down
125 changes: 112 additions & 13 deletions src/hb-ot-shape-complex-arabic-fallback.hh
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ arabic_fallback_synthesize_lookup_single (const hb_ot_shape_plan_t *plan HB_UNUS
num_glyphs++;
}

if (!num_glyphs)
return NULL;

/* Bubble-sort!
* May not be good-enough for presidential candidate interviews, but good-enough for us... */
hb_bubble_sort (&glyphs[0], num_glyphs, OT::GlyphID::cmp, &substitutes[0]);
Expand Down Expand Up @@ -158,6 +161,9 @@ arabic_fallback_synthesize_lookup_ligature (const hb_ot_shape_plan_t *plan HB_UN
}
}

if (!num_ligatures)
return NULL;

OT::Supplier<OT::GlyphID> first_glyphs_supplier (first_glyphs, num_first_glyphs);
OT::Supplier<unsigned int > ligature_per_first_glyph_count_supplier (ligature_per_first_glyph_count_list, num_first_glyphs);
OT::Supplier<OT::GlyphID> ligatures_supplier (ligature_list, num_ligatures);
Expand Down Expand Up @@ -198,13 +204,101 @@ struct arabic_fallback_plan_t
{
ASSERT_POD ();

unsigned int num_lookups;
bool free_lookups;

hb_mask_t mask_array[ARABIC_NUM_FALLBACK_FEATURES];
OT::SubstLookup *lookup_array[ARABIC_NUM_FALLBACK_FEATURES];
hb_ot_layout_lookup_accelerator_t accel_array[ARABIC_NUM_FALLBACK_FEATURES];
};

static const arabic_fallback_plan_t arabic_fallback_plan_nil = {};

#if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(HB_WITH_WIN1256)
#define HB_WITH_WIN1256
#endif

#ifdef HB_WITH_WIN1256
#include "hb-ot-shape-complex-arabic-win1256.hh"
#endif

struct ManifestLookup {
OT::USHORT fallbackType;
OT::OffsetTo<OT::SubstLookup> lookupOffset;
};
typedef OT::ArrayOf<ManifestLookup> Manifest;

static bool
arabic_fallback_plan_init_win1256 (arabic_fallback_plan_t *fallback_plan,
const hb_ot_shape_plan_t *plan,
hb_font_t *font)
{
#ifdef HB_WITH_WIN1256
/* Does this font look like it's Windows-1256-encoded? */
hb_codepoint_t g;
if (!(hb_font_get_glyph (font, 0x0627u, 0, &g) && g == 199 /* ALEF */ &&
hb_font_get_glyph (font, 0x0644u, 0, &g) && g == 225 /* LAM */ &&
hb_font_get_glyph (font, 0x0649u, 0, &g) && g == 236 /* ALEF MAKSURA */ &&
hb_font_get_glyph (font, 0x064Au, 0, &g) && g == 237 /* YEH */ &&
hb_font_get_glyph (font, 0x0652u, 0, &g) && g == 250 /* SUKUN */))
return false;

const Manifest &manifest = reinterpret_cast<const Manifest&> (arabic_win1256_gsub_lookups.manifest);
ASSERT_STATIC (sizeof (arabic_win1256_gsub_lookups.manifestData) / sizeof (ManifestLookup)
<= ARABIC_NUM_FALLBACK_FEATURES);

This comment has been minimized.

Copy link
@jfkthame

jfkthame Aug 1, 2014

Collaborator

After I tried adding an entry for mediLamLookup to the manifest, the build failed at this assert. Is it really true that the manifestData needs to correspond in some way to the fallback-features? I don't think I'm entirely following this.....

This comment has been minimized.

Copy link
@behdad

behdad Aug 1, 2014

Author Member

The array that holds the lookups is sized according to fallback-features. I'll fix.

This comment has been minimized.

Copy link
@behdad

behdad Aug 1, 2014

Author Member

Fixed. PTAL.

/* TODO sanitize the table? */

unsigned j = 0;
unsigned int count = manifest.len;
for (unsigned int i = 0; i < count; i++)
{
fallback_plan->mask_array[j] = plan->map.get_1_mask (manifest[i].fallbackType);
if (fallback_plan->mask_array[j])
{
fallback_plan->lookup_array[j] = const_cast<OT::SubstLookup*> (&(&manifest+manifest[i].lookupOffset));
if (fallback_plan->lookup_array[j])
{
fallback_plan->accel_array[j].init (*fallback_plan->lookup_array[j]);
j++;
}
}
}

fallback_plan->num_lookups = j;
fallback_plan->free_lookups = false;

return j > 0;
#else
return false;
#endif
}

static bool
arabic_fallback_plan_init_unicode (arabic_fallback_plan_t *fallback_plan,
const hb_ot_shape_plan_t *plan,
hb_font_t *font)
{
unsigned int j = 0;
for (unsigned int i = 0; i < ARABIC_NUM_FALLBACK_FEATURES; i++)
{
fallback_plan->mask_array[j] = plan->map.get_1_mask (arabic_fallback_features[i]);
if (fallback_plan->mask_array[j])
{
fallback_plan->lookup_array[j] = arabic_fallback_synthesize_lookup (plan, font, i);
if (fallback_plan->lookup_array[j])
{
fallback_plan->accel_array[j].init (*fallback_plan->lookup_array[j]);
j++;
}
}
}

fallback_plan->num_lookups = j;
fallback_plan->free_lookups = true;

return j > 0;
}

static arabic_fallback_plan_t *
arabic_fallback_plan_create (const hb_ot_shape_plan_t *plan,
hb_font_t *font)
Expand All @@ -213,17 +307,21 @@ arabic_fallback_plan_create (const hb_ot_shape_plan_t *plan,
if (unlikely (!fallback_plan))
return const_cast<arabic_fallback_plan_t *> (&arabic_fallback_plan_nil);

for (unsigned int i = 0; i < ARABIC_NUM_FALLBACK_FEATURES; i++)
{
fallback_plan->mask_array[i] = plan->map.get_1_mask (arabic_fallback_features[i]);
if (fallback_plan->mask_array[i]) {
fallback_plan->lookup_array[i] = arabic_fallback_synthesize_lookup (plan, font, i);
if (fallback_plan->lookup_array[i])
fallback_plan->accel_array[i].init (*fallback_plan->lookup_array[i]);
}
}
fallback_plan->num_lookups = 0;
fallback_plan->free_lookups = false;

/* Try synthesizing GSUB table using Unicode Arabic Presentation Forms,
* in case the font has cmap entries for the presentation-forms characters. */
if (arabic_fallback_plan_init_unicode (fallback_plan, plan, font))
return fallback_plan;

return fallback_plan;
/* See if this looks like a Windows-1256-encoded font. If it does, use a
* hand-coded GSUB table. */
if (arabic_fallback_plan_init_win1256 (fallback_plan, plan, font))
return fallback_plan;

free (fallback_plan);
return const_cast<arabic_fallback_plan_t *> (&arabic_fallback_plan_nil);
}

static void
Expand All @@ -232,11 +330,12 @@ arabic_fallback_plan_destroy (arabic_fallback_plan_t *fallback_plan)
if (!fallback_plan || fallback_plan == &arabic_fallback_plan_nil)
return;

for (unsigned int i = 0; i < ARABIC_NUM_FALLBACK_FEATURES; i++)
for (unsigned int i = 0; i < fallback_plan->num_lookups; i++)
if (fallback_plan->lookup_array[i])
{
fallback_plan->accel_array[i].fini (fallback_plan->lookup_array[i]);
free (fallback_plan->lookup_array[i]);
if (fallback_plan->free_lookups)
free (fallback_plan->lookup_array[i]);
}

free (fallback_plan);
Expand All @@ -248,7 +347,7 @@ arabic_fallback_plan_shape (arabic_fallback_plan_t *fallback_plan,
hb_buffer_t *buffer)
{
OT::hb_apply_context_t c (0, font, buffer);
for (unsigned int i = 0; i < ARABIC_NUM_FALLBACK_FEATURES; i++)
for (unsigned int i = 0; i < fallback_plan->num_lookups; i++)
if (fallback_plan->lookup_array[i]) {
c.set_lookup_mask (fallback_plan->mask_array[i]);
hb_ot_layout_substitute_lookup (&c,
Expand Down
Loading

0 comments on commit a97f537

Please sign in to comment.