From 451945950e12aff37cba8ad42f1ae63880dc0a1f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 29 Jan 2009 09:20:08 +0000 Subject: [PATCH] Remove ClassDef->Defined field. This is the comment accompanying it: 2009-01-29 Behdad Esfahbod * pango/opentype/harfbuzz-open.h: * pango/opentype/harfbuzz-gdef.c (Make_ClassRange), (HB_GDEF_Build_ClassDefinition): * pango/opentype/harfbuzz-gpos.c (Load_PosClassRule), (Load_ChainPosClassRule): * pango/opentype/harfbuzz-gsub.c (Load_SubClassRule), (Load_ChainSubClassRule): * pango/opentype/harfbuzz-open.c (Load_ClassDef1), (Load_ClassDef2), (_HB_OPEN_Load_ClassDefinition), (_HB_OPEN_Load_EmptyClassDefinition), (_HB_OPEN_Free_ClassDefinition): Remove ClassDef->Defined field. This is the comment accompanying it: The `Defined' field is not defined in the OpenType specification but apparently needed for processing fonts like trado.ttf: This font refers to a class which contains not a single element. We map such classes to class 0. The comment is correct that trado.ttf (MS Traditional Arabic) uses such classes. However, in my testing I couldn't identify any problems with the font if the special handling is removed. I also processed as many fonts as I could get my hand on and trado.ttf was the only not-totally-broken font hitting the special-case code. DejaVu fonts hit it too, but I'm sure they do not require the special-handling code. Most probably, that code introduces bugs in them. The special-casing was consuming lots of memory. EIGHT MEGABYTES for loading DejaVu Sans! While this could be complete fixed, I decided to remove the special-handling code altogether. I don't think it will make any real difference, and if it does, we'll fix fonts. Such hacks will not be in harfbuzz-ng anyway. Bug originally reported by nsf. svn path=/trunk/; revision=2819 --- ChangeLog | 37 +++++++++++++++++++++++++++++++++++++ pango/opentype/harfbuzz-gdef.c | 8 -------- pango/opentype/harfbuzz-gpos.c | 32 -------------------------------- pango/opentype/harfbuzz-gsub.c | 31 ------------------------------- pango/opentype/harfbuzz-open.c | 30 +++--------------------------- pango/opentype/harfbuzz-open.h | 8 -------- 6 files changed, 40 insertions(+), 106 deletions(-) diff --git a/ChangeLog b/ChangeLog index 959e67f..159967b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,42 @@ 2009-01-29 Behdad Esfahbod + * pango/opentype/harfbuzz-open.h: + * pango/opentype/harfbuzz-gdef.c (Make_ClassRange), + (HB_GDEF_Build_ClassDefinition): + * pango/opentype/harfbuzz-gpos.c (Load_PosClassRule), + (Load_ChainPosClassRule): + * pango/opentype/harfbuzz-gsub.c (Load_SubClassRule), + (Load_ChainSubClassRule): + * pango/opentype/harfbuzz-open.c (Load_ClassDef1), + (Load_ClassDef2), (_HB_OPEN_Load_ClassDefinition), + (_HB_OPEN_Load_EmptyClassDefinition), + (_HB_OPEN_Free_ClassDefinition): + Remove ClassDef->Defined field. This is the comment accompanying it: + + The `Defined' field is not defined in the OpenType specification + but apparently needed for processing fonts like trado.ttf: This + font refers to a class which contains not a single element. We + map such classes to class 0. + + The comment is correct that trado.ttf (MS Traditional Arabic) uses + such classes. However, in my testing I couldn't identify any + problems with the font if the special handling is removed. I also + processed as many fonts as I could get my hand on and trado.ttf was + the only not-totally-broken font hitting the special-case code. + DejaVu fonts hit it too, but I'm sure they do not require the + special-handling code. Most probably, that code introduces bugs + in them. + + The special-casing was consuming lots of memory. EIGHT MEGABYTES + for loading DejaVu Sans! While this could be complete fixed, I + decided to remove the special-handling code altogether. I don't + think it will make any real difference, and if it does, we'll fix + fonts. Such hacks will not be in harfbuzz-ng anyway. + + Bug originally reported by nsf. + +2009-01-29 Behdad Esfahbod + * pango/opentype/harfbuzz-impl.c (_hb_alloc): Use calloc(), instead of malloc()ing and memset()ing. diff --git a/pango/opentype/harfbuzz-gdef.c b/pango/opentype/harfbuzz-gdef.c index 225aa8b..8fecf4c 100644 --- a/pango/opentype/harfbuzz-gdef.c +++ b/pango/opentype/harfbuzz-gdef.c @@ -776,8 +776,6 @@ static HB_Error Make_ClassRange( HB_ClassDefinition* cd, crr[index].End = end; crr[index].Class = class; - cd->Defined[class] = TRUE; - return HB_Err_Ok; } @@ -807,11 +805,6 @@ HB_Error HB_GDEF_Build_ClassDefinition( HB_GDEFHeader* gdef, gcd->ClassFormat = 2; - /* A GlyphClassDef table contains at most 5 different class values */ - - if ( ALLOC_ARRAY( gcd->Defined, 5, HB_Bool ) ) - return error; - gcd->cd.cd2.ClassRangeCount = 0; gcd->cd.cd2.ClassRangeRecord = NULL; @@ -959,7 +952,6 @@ Fail3: FREE( gcd->cd.cd2.ClassRangeRecord ); Fail4: - FREE( gcd->Defined ); return error; } diff --git a/pango/opentype/harfbuzz-gpos.c b/pango/opentype/harfbuzz-gpos.c index 6583dee..a318ce1 100644 --- a/pango/opentype/harfbuzz-gpos.c +++ b/pango/opentype/harfbuzz-gpos.c @@ -3362,7 +3362,6 @@ static HB_Error Load_PosClassRule( HB_ContextPosFormat2* cpf2, HB_UShort* c; HB_PosLookupRecord* plr; - HB_Bool* d; if ( ACCESS_Frame( 4L ) ) @@ -3384,22 +3383,13 @@ static HB_Error Load_PosClassRule( HB_ContextPosFormat2* cpf2, return error; c = pcr->Class; - d = cpf2->ClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail2; for ( n = 0; n < count; n++ ) - { c[n] = GET_UShort(); - /* We check whether the specific class is used at all. If not, - class 0 is used instead. */ - - if ( !d[c[n]] ) - c[n] = 0; - } - FORGET_Frame(); pcr->PosLookupRecord = NULL; @@ -4376,7 +4366,6 @@ static HB_Error Load_ChainPosClassRule( HB_UShort* i; HB_UShort* l; HB_PosLookupRecord* plr; - HB_Bool* d; if ( ACCESS_Frame( 2L ) ) @@ -4397,22 +4386,13 @@ static HB_Error Load_ChainPosClassRule( return error; b = cpcr->Backtrack; - d = ccpf2->BacktrackClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail4; for ( n = 0; n < count; n++ ) - { b[n] = GET_UShort(); - /* We check whether the specific class is used at all. If not, - class 0 is used instead. */ - - if ( !d[b[n]] ) - b[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) @@ -4433,19 +4413,13 @@ static HB_Error Load_ChainPosClassRule( goto Fail4; i = cpcr->Input; - d = ccpf2->InputClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail3; for ( n = 0; n < count; n++ ) - { i[n] = GET_UShort(); - if ( !d[i[n]] ) - i[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) @@ -4466,19 +4440,13 @@ static HB_Error Load_ChainPosClassRule( goto Fail3; l = cpcr->Lookahead; - d = ccpf2->LookaheadClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail2; for ( n = 0; n < count; n++ ) - { l[n] = GET_UShort(); - if ( !d[l[n]] ) - l[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) diff --git a/pango/opentype/harfbuzz-gsub.c b/pango/opentype/harfbuzz-gsub.c index 067122d..f504bf0 100644 --- a/pango/opentype/harfbuzz-gsub.c +++ b/pango/opentype/harfbuzz-gsub.c @@ -1385,7 +1385,6 @@ static HB_Error Load_SubClassRule( HB_ContextSubstFormat2* csf2, HB_UShort* c; HB_SubstLookupRecord* slr; - HB_Bool* d; if ( ACCESS_Frame( 4L ) ) @@ -1407,21 +1406,13 @@ static HB_Error Load_SubClassRule( HB_ContextSubstFormat2* csf2, return error; c = scr->Class; - d = csf2->ClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail2; for ( n = 0; n < count; n++ ) - { c[n] = GET_UShort(); - /* We check whether the specific class is used at all. If not, - class 0 is used instead. */ - if ( !d[c[n]] ) - c[n] = 0; - } - FORGET_Frame(); scr->SubstLookupRecord = NULL; @@ -2379,7 +2370,6 @@ static HB_Error Load_ChainSubClassRule( HB_UShort* i; HB_UShort* l; HB_SubstLookupRecord* slr; - HB_Bool* d; if ( ACCESS_Frame( 2L ) ) @@ -2400,22 +2390,13 @@ static HB_Error Load_ChainSubClassRule( return error; b = cscr->Backtrack; - d = ccsf2->BacktrackClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail4; for ( n = 0; n < count; n++ ) - { b[n] = GET_UShort(); - /* We check whether the specific class is used at all. If not, - class 0 is used instead. */ - - if ( !d[b[n]] ) - b[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) @@ -2436,19 +2417,13 @@ static HB_Error Load_ChainSubClassRule( goto Fail4; i = cscr->Input; - d = ccsf2->InputClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail3; for ( n = 0; n < count; n++ ) - { i[n] = GET_UShort(); - if ( !d[i[n]] ) - i[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) @@ -2469,19 +2444,13 @@ static HB_Error Load_ChainSubClassRule( goto Fail3; l = cscr->Lookahead; - d = ccsf2->LookaheadClassDef.Defined; if ( ACCESS_Frame( count * 2L ) ) goto Fail2; for ( n = 0; n < count; n++ ) - { l[n] = GET_UShort(); - if ( !d[l[n]] ) - l[n] = 0; - } - FORGET_Frame(); if ( ACCESS_Frame( 2L ) ) diff --git a/pango/opentype/harfbuzz-open.c b/pango/opentype/harfbuzz-open.c index c69a407..e187916 100644 --- a/pango/opentype/harfbuzz-open.c +++ b/pango/opentype/harfbuzz-open.c @@ -942,7 +942,6 @@ static HB_Error Load_ClassDef1( HB_ClassDefinition* cd, HB_UShort n, count; HB_UShort* cva; - HB_Bool* d; HB_ClassDefFormat1* cdf1; @@ -967,7 +966,6 @@ static HB_Error Load_ClassDef1( HB_ClassDefinition* cd, if ( ALLOC_ARRAY( cdf1->ClassValueArray, count, HB_UShort ) ) return error; - d = cd->Defined; cva = cdf1->ClassValueArray; if ( ACCESS_Frame( count * 2L ) ) @@ -981,7 +979,6 @@ static HB_Error Load_ClassDef1( HB_ClassDefinition* cd, error = ERR(HB_Err_Invalid_SubTable); goto Fail; } - d[cva[n]] = TRUE; } FORGET_Frame(); @@ -1012,7 +1009,6 @@ static HB_Error Load_ClassDef2( HB_ClassDefinition* cd, HB_UShort n, count; HB_ClassRangeRecord* crr; - HB_Bool* d; HB_ClassDefFormat2* cdf2; @@ -1032,7 +1028,6 @@ static HB_Error Load_ClassDef2( HB_ClassDefinition* cd, if ( ALLOC_ARRAY( cdf2->ClassRangeRecord, count, HB_ClassRangeRecord ) ) return error; - d = cd->Defined; crr = cdf2->ClassRangeRecord; if ( ACCESS_Frame( count * 6L ) ) @@ -1056,8 +1051,6 @@ static HB_Error Load_ClassDef2( HB_ClassDefinition* cd, n--; count--; } - else - d[crr[n].Class] = TRUE; } FORGET_Frame(); @@ -1088,11 +1081,8 @@ _HB_OPEN_Load_ClassDefinition( HB_ClassDefinition* cd, { HB_Error error; - if ( ALLOC_ARRAY( cd->Defined, limit, HB_Bool ) ) - return error; - if ( ACCESS_Frame( 2L ) ) - goto Fail; + return error; cd->ClassFormat = GET_UShort(); @@ -1106,15 +1096,11 @@ _HB_OPEN_Load_ClassDefinition( HB_ClassDefinition* cd, } if ( error ) - goto Fail; + return error; cd->loaded = TRUE; return HB_Err_Ok; - -Fail: - FREE( cd->Defined ); - return error; } @@ -1123,20 +1109,12 @@ _HB_OPEN_Load_EmptyClassDefinition( HB_ClassDefinition* cd ) { HB_Error error; - if ( ALLOC_ARRAY( cd->Defined, 1, HB_Bool ) ) - return error; - cd->ClassFormat = 1; /* Meaningless */ - cd->Defined[0] = FALSE; if ( ALLOC_ARRAY( cd->cd.cd1.ClassValueArray, 1, HB_UShort ) ) - goto Fail; + return error; return HB_Err_Ok; - -Fail: - FREE( cd->Defined ); - return error; } HB_INTERNAL HB_Error @@ -1171,8 +1149,6 @@ _HB_OPEN_Free_ClassDefinition( HB_ClassDefinition* cd ) if ( !cd->loaded ) return; - FREE( cd->Defined ); - switch ( cd->ClassFormat ) { case 1: Free_ClassDef1( &cd->cd.cd1 ); break; diff --git a/pango/opentype/harfbuzz-open.h b/pango/opentype/harfbuzz-open.h index d02962b..2ed4440 100644 --- a/pango/opentype/harfbuzz-open.h +++ b/pango/opentype/harfbuzz-open.h @@ -240,18 +240,10 @@ struct HB_ClassDefFormat2_ typedef struct HB_ClassDefFormat2_ HB_ClassDefFormat2; -/* The `Defined' field is not defined in the OpenType specification but - apparently needed for processing fonts like trado.ttf: This font - refers to a class which contains not a single element. We map such - classes to class 0. */ - struct HB_ClassDefinition_ { HB_Bool loaded; - HB_Bool* Defined; /* array of Booleans. - If Defined[n] is FALSE, - class n contains no glyphs. */ HB_UShort ClassFormat; /* 1 or 2 */ union -- 2.7.4