[HarfBuzz] harfbuzz: Branch 'master' - 4 commits
Behdad Esfahbod
behdad at kemper.freedesktop.org
Mon Dec 17 20:26:21 PST 2012
src/hb-open-type-private.hh | 11 ++
src/hb-ot-layout-common-private.hh | 152 ++++++++++++++++++++++++++++++-------
src/hb-ot-layout.cc | 93 +++-------------------
3 files changed, 150 insertions(+), 106 deletions(-)
New commits:
commit efe252e6000558f78075adadb2a3dba25ab67c04
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Mon Dec 17 23:21:05 2012 -0500
[OTLayout] Fix 'size' featureParams implementation
Looks at alternate location now.
diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index 50ffa20..bff383a 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -261,7 +261,71 @@ struct FeatureParamsSize
{
inline bool sanitize (hb_sanitize_context_t *c) {
TRACE_SANITIZE (this);
- return TRACE_RETURN (c->check_struct (this));
+ if (unlikely (!c->check_struct (this))) return TRACE_RETURN (false);
+
+ /* This subtable has some "history", if you will. Some earlier versions of
+ * Adobe tools calculated the offset of the FeatureParams sutable from the
+ * beginning of the FeatureList table! Now, that is dealt with in the
+ * Feature implementation. But we still need to be able to tell junk from
+ * real data. Note: We don't check that the nameID actually exists.
+ *
+ * Read Roberts wrote on 9/15/06 on opentype-list at indx.co.uk :
+ *
+ * Yes, it is correct that a new version of the AFDKO (version 2.0) will be
+ * coming out soon, and that the makeotf program will build a font with a
+ * 'size' feature that is correct by the specification.
+ *
+ * The specification for this feature tag is in the "OpenType Layout Tag
+ * Registry". You can see a copy of this at:
+ * http://partners.adobe.com/public/developer/opentype/index_tag8.html#size
+ *
+ * Here is one set of rules to determine if the 'size' feature is built
+ * correctly, or as by the older versions of MakeOTF. You may be able to do
+ * better.
+ *
+ * Assume that the offset to the size feature is according to specification,
+ * and make the following value checks. If it fails, assume the the size
+ * feature is calculated as versions of MakeOTF before the AFDKO 2.0 built it.
+ * If this fails, reject the 'size' feature. The older makeOTF's calculated the
+ * offset from the beginning of the FeatureList table, rather than from the
+ * beginning of the 'size' Feature table.
+ *
+ * If "design size" == 0:
+ * fails check
+ *
+ * Else if ("subfamily identifier" == 0 and
+ * "range start" == 0 and
+ * "range end" == 0 and
+ * "range start" == 0 and
+ * "menu name ID" == 0)
+ * passes check: this is the format used when there is a design size
+ * specified, but there is no recommended size range.
+ *
+ * Else if ("design size" < "range start" or
+ * "design size" > "range end" or
+ * "range end" <= "range start" or
+ * "menu name ID" < 256 or
+ * "menu name ID" > 32767 or
+ * menu name ID is not a name ID which is actually in the name table)
+ * fails test
+ * Else
+ * passes test.
+ */
+
+ if (!designSize)
+ return TRACE_RETURN (false);
+ else if (subfamilyID == 0 &&
+ subfamilyNameID == 0 &&
+ rangeStart == 0 &&
+ rangeEnd == 0)
+ return TRACE_RETURN (true);
+ else if (designSize < rangeStart ||
+ designSize > rangeEnd ||
+ subfamilyNameID < 256 ||
+ subfamilyNameID > 32767)
+ return TRACE_RETURN (false);
+ else
+ return TRACE_RETURN (true);
}
USHORT designSize; /* Represents the design size in 720/inch
@@ -343,9 +407,7 @@ struct FeatureParamsCharacterVariants
return TRACE_RETURN (c->check_struct (this) &&
characters.sanitize (c));
}
- /* TODO: This is made private since we don't have the facilities in
- * FeatureParams to correctly sanitize this. */
- private:
+
USHORT format; /* Format number is set to 0. */
USHORT featUILableNameID; /* The ‘name’ table name ID that
* specifies a string (or strings,
@@ -380,24 +442,25 @@ struct FeatureParamsCharacterVariants
struct FeatureParams
{
- /* Note:
- *
- * FeatureParams structures unfortunately don't have a generic length argument,
- * so their length depends on the feature name / requested use. We don't have
- * that information at sanitize time. As such, we sanitize for the longest
- * subtable possible. This may nuke a possibly valid subtable if it's unfortunate
- * enough to happen at the very end of the GSUB/GPOS table. But that's very
- * unlikely (I hope!).
- *
- * When we fully implement FeatureParamsCharacterVariants, we should fix this
- * shortcoming...
- */
-
- inline bool sanitize (hb_sanitize_context_t *c) {
+ inline bool sanitize (hb_sanitize_context_t *c, hb_tag_t tag) {
TRACE_SANITIZE (this);
- return TRACE_RETURN (c->check_struct (this));
+ if (tag == HB_TAG ('s','i','z','e'))
+ return TRACE_RETURN (u.size.sanitize (c));
+ if ((tag & 0xFFFF0000) == HB_TAG ('s','s','\0','\0')) /* ssXX */
+ return TRACE_RETURN (u.stylisticSet.sanitize (c));
+ if ((tag & 0xFFFF0000) == HB_TAG ('c','v','\0','\0')) /* cvXX */
+ return TRACE_RETURN (u.characterVariants.sanitize (c));
+ return TRACE_RETURN (true);
}
+ inline const FeatureParamsSize& get_size_params (hb_tag_t tag) const
+ {
+ if (tag == HB_TAG ('s','i','z','e'))
+ return u.size;
+ return Null(FeatureParamsSize);
+ }
+
+ private:
union {
FeatureParamsSize size;
FeatureParamsStylisticSet stylisticSet;
@@ -426,26 +489,36 @@ struct Feature
if (unlikely (!(c->check_struct (this) && lookupIndex.sanitize (c))))
return TRACE_RETURN (false);
+ /* Some earlier versions of Adobe tools calculated the offset of the
+ * FeatureParams subtable from the beginning of the FeatureList table!
+ *
+ * If sanitizing "failed" for the FeatureParams subtable, try it with the
+ * alternative location. We would know sanitize "failed" if old value
+ * of the offset was non-zero, but it's zeroed now.
+ */
+
Offset orig_offset = featureParams;
- if (likely (featureParams.sanitize (c, this)))
+ if (unlikely (!featureParams.sanitize (c, this, closure ? closure->tag : HB_TAG_NONE)))
+ return TRACE_RETURN (false);
+
+ if (likely (!orig_offset))
return TRACE_RETURN (true);
- /* Some earlier versions of Adobe tools calculated the offset of the
- * FeatureParams sutable from the beginning of the FeatureList table!
- * Try that instead... */
- if (closure && closure->list_base)
+ if (featureParams == 0 && closure && closure->list_base && closure->list_base < this)
{
+ unsigned int new_offset_int = (unsigned int) orig_offset -
+ ((char *) this - (char *) closure->list_base);
+
Offset new_offset;
- new_offset.set (orig_offset - ((char *) this - (char *) closure->list_base));
/* Check that it did not overflow. */
- if (new_offset != (orig_offset - ((char *) this - (char *) closure->list_base)))
+ new_offset.set (new_offset_int);
+ if (new_offset == new_offset_int &&
+ featureParams.try_set (c, new_offset) &&
+ !featureParams.sanitize (c, this, closure ? closure->tag : HB_TAG_NONE))
return TRACE_RETURN (false);
-
- return TRACE_RETURN (featureParams.try_set (c, new_offset) &&
- featureParams.sanitize (c, this));
}
- return TRACE_RETURN (false);
+ return TRACE_RETURN (true);
}
OffsetTo<FeatureParams>
diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc
index f7a54bb..e4bac0a 100644
--- a/src/hb-ot-layout.cc
+++ b/src/hb-ot-layout.cc
@@ -647,104 +647,39 @@ hb_ot_layout_get_size_params (hb_face_t *face,
unsigned int *range_start, /* OUT. May be NULL */
unsigned int *range_end /* OUT. May be NULL */)
{
- bool ret = false;
const OT::GPOS &gpos = _get_gpos (face);
+ const hb_tag_t tag = HB_TAG ('s','i','z','e');
unsigned int num_features = gpos.get_feature_count ();
for (unsigned int i = 0; i < num_features; i++)
{
- if (HB_TAG ('s','i','z','e') == gpos.get_feature_tag (i))
+ if (tag == gpos.get_feature_tag (i))
{
const OT::Feature &f = gpos.get_feature (i);
- const OT::FeatureParamsSize ¶ms = f.get_feature_params ().u.size;
-
- /* This subtable has some "history", if you will. Some earlier versions of
- * Adobe tools calculated the offset of the FeatureParams sutable from the
- * beginning of the FeatureList table! Now, we don't check for that possibility,
- * but we want to at least detect junk data and reject it.
- *
- * Read Roberts wrote on 9/15/06 on opentype-list at indx.co.uk :
- *
- * Yes, it is correct that a new version of the AFDKO (version 2.0) will be
- * coming out soon, and that the makeotf program will build a font with a
- * 'size' feature that is correct by the specification.
- *
- * The specification for this feature tag is in the "OpenType Layout Tag
- * Registry". You can see a copy of this at:
- * http://partners.adobe.com/public/developer/opentype/index_tag8.html#size
- *
- * Here is one set of rules to determine if the 'size' feature is built
- * correctly, or as by the older versions of MakeOTF. You may be able to do
- * better.
- *
- * Assume that the offset to the size feature is according to specification,
- * and make the following value checks. If it fails, assume the the size
- * feature is calculated as versions of MakeOTF before the AFDKO 2.0 built it.
- * If this fails, reject the 'size' feature. The older makeOTF's calculated the
- * offset from the beginning of the FeatureList table, rather than from the
- * beginning of the 'size' Feature table.
- *
- * If "design size" == 0:
- * fails check
- *
- * Else if ("subfamily identifier" == 0 and
- * "range start" == 0 and
- * "range end" == 0 and
- * "range start" == 0 and
- * "menu name ID" == 0)
- * passes check: this is the format used when there is a design size
- * specified, but there is no recommended size range.
- *
- * Else if ("design size" < "range start" or
- * "design size" > "range end" or
- * "range end" <= "range start" or
- * "menu name ID" < 256 or
- * "menu name ID" > 32767 or
- * menu name ID is not a name ID which is actually in the name table)
- * fails test
- * Else
- * passes test.
- */
-
- if (!params.designSize)
- ret = false;
- else if (params.subfamilyID == 0 &&
- params.subfamilyNameID == 0 &&
- params.rangeStart == 0 &&
- params.rangeEnd == 0)
- ret = true;
- else if (params.designSize < params.rangeStart ||
- params.designSize > params.rangeEnd ||
- params.subfamilyNameID < 256 ||
- params.subfamilyNameID > 32767)
- ret = false;
- else
- ret = true;
+ const OT::FeatureParamsSize ¶ms = f.get_feature_params ().get_size_params (tag);
-#define PARAM(a, A) if (a) *a = params.A
- if (ret)
+ if (params.designSize)
{
+#define PARAM(a, A) if (a) *a = params.A
PARAM (design_size, designSize);
PARAM (subfamily_id, subfamilyID);
PARAM (subfamily_name_id, subfamilyNameID);
PARAM (range_start, rangeStart);
PARAM (range_end, rangeEnd);
- break;
- }
#undef PARAM
+
+ return true;
+ }
}
}
#define PARAM(a, A) if (a) *a = 0
- if (!ret)
- {
- PARAM (design_size, designSize);
- PARAM (subfamily_id, subfamilyID);
- PARAM (subfamily_name_id, subfamilyNameID);
- PARAM (range_start, rangeStart);
- PARAM (range_end, rangeEnd);
- }
+ PARAM (design_size, designSize);
+ PARAM (subfamily_id, subfamilyID);
+ PARAM (subfamily_name_id, subfamilyNameID);
+ PARAM (range_start, rangeStart);
+ PARAM (range_end, rangeEnd);
#undef PARAM
- return ret;
+ return false;
}
commit e77b4425746ac9eb407ca4e742d962f1955971b4
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Mon Dec 17 18:42:59 2012 -0500
[OTLayout] Fix tracing
diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh
index 6b9f721..5bfeb16 100644
--- a/src/hb-open-type-private.hh
+++ b/src/hb-open-type-private.hh
@@ -255,7 +255,8 @@ struct hb_sanitize_context_t
"may_edit(%u) [%p..%p] (%d bytes) in [%p..%p] -> %s",
this->edit_count,
p, p + len, len,
- this->start, this->end);
+ this->start, this->end,
+ this->writable ? "GRANTED" : "DENIED");
return TRACE_RETURN (this->writable);
}
commit 9b54562d63f1a9e0e5b33d71c32bd1588759ebf1
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Mon Dec 17 13:55:36 2012 -0500
[OTLayout] Towards correct FeatureParams handling
diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh
index 347e299..6b9f721 100644
--- a/src/hb-open-type-private.hh
+++ b/src/hb-open-type-private.hh
@@ -704,7 +704,13 @@ struct GenericOffsetTo : OffsetType
return TRACE_RETURN (likely (obj.sanitize (c, user_data)) || neuter (c));
}
- private:
+ inline bool try_set (hb_sanitize_context_t *c, const OffsetType &v) {
+ if (c->may_edit (this, this->static_size)) {
+ this->set (v);
+ return true;
+ }
+ return false;
+ }
/* Set the offset to Null */
inline bool neuter (hb_sanitize_context_t *c) {
if (c->may_edit (this, this->static_size)) {
diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index 1671717..50ffa20 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -423,8 +423,29 @@ struct Feature
inline bool sanitize (hb_sanitize_context_t *c,
const Record<Feature>::sanitize_closure_t *closure) {
TRACE_SANITIZE (this);
- return TRACE_RETURN (c->check_struct (this) && lookupIndex.sanitize (c) &&
- featureParams.sanitize (c, this));
+ if (unlikely (!(c->check_struct (this) && lookupIndex.sanitize (c))))
+ return TRACE_RETURN (false);
+
+ Offset orig_offset = featureParams;
+ if (likely (featureParams.sanitize (c, this)))
+ return TRACE_RETURN (true);
+
+ /* Some earlier versions of Adobe tools calculated the offset of the
+ * FeatureParams sutable from the beginning of the FeatureList table!
+ * Try that instead... */
+ if (closure && closure->list_base)
+ {
+ Offset new_offset;
+ new_offset.set (orig_offset - ((char *) this - (char *) closure->list_base));
+ /* Check that it did not overflow. */
+ if (new_offset != (orig_offset - ((char *) this - (char *) closure->list_base)))
+ return TRACE_RETURN (false);
+
+ return TRACE_RETURN (featureParams.try_set (c, new_offset) &&
+ featureParams.sanitize (c, this));
+ }
+
+ return TRACE_RETURN (false);
}
OffsetTo<FeatureParams>
commit 87e43b7f2be25840748f920ca33ff553833da45f
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Fri Dec 14 17:48:23 2012 -0500
[OTLayout] Wire tag and list start all the way to Feature
To fix FeatureParam issues. No actual fix yet, just plumbing.
diff --git a/src/hb-ot-layout-common-private.hh b/src/hb-ot-layout-common-private.hh
index da6c8f9..1671717 100644
--- a/src/hb-ot-layout-common-private.hh
+++ b/src/hb-ot-layout-common-private.hh
@@ -60,9 +60,14 @@ struct Record
return tag.cmp (a);
}
+ struct sanitize_closure_t {
+ hb_tag_t tag;
+ void *list_base;
+ };
inline bool sanitize (hb_sanitize_context_t *c, void *base) {
TRACE_SANITIZE (this);
- return TRACE_RETURN (c->check_struct (this) && offset.sanitize (c, base));
+ const sanitize_closure_t closure = {tag, base};
+ return TRACE_RETURN (c->check_struct (this) && offset.sanitize (c, base, &closure));
}
Tag tag; /* 4-byte Tag identifier */
@@ -192,7 +197,8 @@ struct LangSys
return reqFeatureIndex;;
}
- inline bool sanitize (hb_sanitize_context_t *c) {
+ inline bool sanitize (hb_sanitize_context_t *c,
+ const Record<LangSys>::sanitize_closure_t * = NULL) {
TRACE_SANITIZE (this);
return TRACE_RETURN (c->check_struct (this) && featureIndex.sanitize (c));
}
@@ -230,7 +236,8 @@ struct Script
inline bool has_default_lang_sys (void) const { return defaultLangSys != 0; }
inline const LangSys& get_default_lang_sys (void) const { return this+defaultLangSys; }
- inline bool sanitize (hb_sanitize_context_t *c) {
+ inline bool sanitize (hb_sanitize_context_t *c,
+ const Record<Script>::sanitize_closure_t * = NULL) {
TRACE_SANITIZE (this);
return TRACE_RETURN (defaultLangSys.sanitize (c, this) && langSys.sanitize (c, this));
}
@@ -413,7 +420,8 @@ struct Feature
inline const FeatureParams &get_feature_params (void) const
{ return this+featureParams; }
- inline bool sanitize (hb_sanitize_context_t *c) {
+ inline bool sanitize (hb_sanitize_context_t *c,
+ const Record<Feature>::sanitize_closure_t *closure) {
TRACE_SANITIZE (this);
return TRACE_RETURN (c->check_struct (this) && lookupIndex.sanitize (c) &&
featureParams.sanitize (c, this));
More information about the HarfBuzz
mailing list