From 54cbe6702c1e4c934c60512367abaf801294c1bb Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Tue, 6 Mar 2018 16:41:08 +0330 Subject: [PATCH] [ot-color] Further improvements on COLR/CPAL implementation (#859) * Implemented a bsearch on get_base_glyph_record * Made get_color_record_argb actually work --- src/hb-ot-color-colr-table.hh | 35 ++++++++++++++----------- src/hb-ot-color-cpal-table.hh | 59 +++++++++++++++---------------------------- src/hb-ot-color.h | 12 ++------- 3 files changed, 42 insertions(+), 64 deletions(-) diff --git a/src/hb-ot-color-colr-table.hh b/src/hb-ot-color-colr-table.hh index f9d65aa..15bfafb 100644 --- a/src/hb-ot-color-colr-table.hh +++ b/src/hb-ot-color-colr-table.hh @@ -80,41 +80,46 @@ struct COLR { TRACE_SANITIZE (this); if (!(c->check_struct (this) && - c->check_array ((const void*) &layerRecordsOffset, sizeof (LayerRecord), numLayerRecords) && - c->check_array ((const void*) &baseGlyphRecords, sizeof (BaseGlyphRecord), numBaseGlyphRecords))) + c->check_array ((const void*) &layerRecordsOffsetZ, sizeof (LayerRecord), numLayerRecords) && + c->check_array ((const void*) &baseGlyphRecordsZ, sizeof (BaseGlyphRecord), numBaseGlyphRecords))) return_trace (false); - const BaseGlyphRecord* base_glyph_records = &baseGlyphRecords (this); + const BaseGlyphRecord* base_glyph_records = &baseGlyphRecordsZ (this); for (unsigned int i = 0; i < numBaseGlyphRecords; ++i) if (base_glyph_records[i].firstLayerIndex + base_glyph_records[i].numLayers > numLayerRecords) return_trace (false); - /* XXX values of LayerRecord structs should be sanitized */ - return_trace (true); } inline const bool get_base_glyph_record ( hb_codepoint_t glyph_id, unsigned int &first_layer, unsigned int &num_layers) const { - /* TODO replace with bsearch */ - const BaseGlyphRecord* base_glyph_records = &baseGlyphRecords (this); - unsigned int records = numBaseGlyphRecords; - for (unsigned int i = 0; i < records; ++i) - if (base_glyph_records[i].gID == glyph_id) + const BaseGlyphRecord* base_glyph_records = &baseGlyphRecordsZ (this); + unsigned int min = 0, max = numBaseGlyphRecords - 1; + while (min <= max) + { + unsigned int mid = (min + max) / 2; + hb_codepoint_t gID = base_glyph_records[mid].gID; + if (gID > glyph_id) + max = mid - 1; + else if (gID < glyph_id) + min = mid + 1; + else { - first_layer = base_glyph_records[i].firstLayerIndex; - num_layers = base_glyph_records[i].numLayers; + first_layer = base_glyph_records[mid].firstLayerIndex; + num_layers = base_glyph_records[mid].numLayers; return true; } + } return false; } inline void get_layer_record (int layer, hb_codepoint_t &glyph_id, unsigned int &palette_index) const { - const LayerRecord* records = &layerRecordsOffset (this); + const LayerRecord* records = &layerRecordsOffsetZ (this); glyph_id = records[layer].gID; palette_index = records[layer].paletteIndex; } @@ -123,9 +128,9 @@ struct COLR HBUINT16 version; /* Table version number */ HBUINT16 numBaseGlyphRecords; /* Number of Base Glyph Records */ LOffsetTo - baseGlyphRecords; /* Offset to Base Glyph records. */ + baseGlyphRecordsZ; /* Offset to Base Glyph records. */ LOffsetTo - layerRecordsOffset; /* Offset to Layer Records */ + layerRecordsOffsetZ; /* Offset to Layer Records */ HBUINT16 numLayerRecords; /* Number of Layer Records */ public: DEFINE_SIZE_STATIC (14); diff --git a/src/hb-ot-color-cpal-table.hh b/src/hb-ot-color-cpal-table.hh index 135862d..d274187 100644 --- a/src/hb-ot-color-cpal-table.hh +++ b/src/hb-ot-color-cpal-table.hh @@ -40,25 +40,6 @@ namespace OT { -struct ColorRecord -{ - friend struct CPAL; - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (true); - } - - protected: - HBUINT8 blue; - HBUINT8 green; - HBUINT8 red; - HBUINT8 alpha; - public: - DEFINE_SIZE_STATIC (4); -}; - struct CPALV1Tail { friend struct CPAL; @@ -96,6 +77,8 @@ struct CPALV1Tail DEFINE_SIZE_STATIC (12); }; +typedef HBUINT32 BGRAColor; + struct CPAL { static const hb_tag_t tableTag = HB_OT_TAG_CPAL; @@ -103,16 +86,13 @@ struct CPAL inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (!(c->check_struct (this) && - colorRecords.sanitize (c))) - return_trace (false); - - unsigned int palettes = numPalettes; - if (!c->check_array (colorRecordIndices, sizeof (HBUINT16), palettes)) + if (!(c->check_struct (this) && // This checks colorRecordIndicesX sanity also, see #get_size + c->check_array ((const void*) &colorRecordsZ, sizeof (BGRAColor), numColorRecords))) return_trace (false); - for (unsigned int i = 0; i < palettes; ++i) - if (colorRecordIndices[i] + numPaletteEntries > colorRecords.get_size ()) + // Check for indices sanity so no need for doing it runtime + for (unsigned int i = 0; i < numPalettes; ++i) + if (colorRecordIndicesX[i] + numPaletteEntries > numColorRecords) return_trace (false); // If version is zero, we are done here; otherwise we need to check tail also @@ -120,12 +100,12 @@ struct CPAL return_trace (true); const CPALV1Tail &v1 = StructAfter (*this); - return_trace (v1.sanitize (c, palettes)); + return_trace (v1.sanitize (c, numPalettes)); } inline unsigned int get_size (void) const { - return min_size + numPalettes * 2; + return min_size + numPalettes * sizeof (HBUINT16); } inline hb_ot_color_palette_flags_t get_palette_flags (unsigned int palette) const @@ -151,14 +131,14 @@ struct CPAL return numPalettes; } - inline void get_color_record (int palette_index, uint8_t &r, uint8_t &g, - uint8_t &b, uint8_t &a) const + inline hb_ot_color_t get_color_record_argb (unsigned int color_index, unsigned int palette) const { - // We should check if palette_index is in range as it is not done on COLR sanitization - r = colorRecords[palette_index].red; - g = colorRecords[palette_index].green; - b = colorRecords[palette_index].blue; - a = colorRecords[palette_index].alpha; + if (color_index >= numPaletteEntries || palette >= numPalettes) + return 0; + + const BGRAColor* records = &colorRecordsZ(this); + // No need for more range check as it is already done on #sanitize + return records[colorRecordIndicesX[palette] + color_index]; } protected: @@ -166,11 +146,12 @@ struct CPAL /* Version 0 */ HBUINT16 numPaletteEntries; HBUINT16 numPalettes; - ArrayOf colorRecords; - HBUINT16 colorRecordIndices[VAR]; // VAR=numPalettes + HBUINT16 numColorRecords; + LOffsetTo colorRecordsZ; + HBUINT16 colorRecordIndicesX[VAR]; // VAR=numPalettes /*CPALV1Tail v1[VAR];*/ public: - DEFINE_SIZE_ARRAY (12, colorRecordIndices); + DEFINE_SIZE_ARRAY (12, colorRecordIndicesX); }; } /* namespace OT */ diff --git a/src/hb-ot-color.h b/src/hb-ot-color.h index 8c276e5..44b3406 100644 --- a/src/hb-ot-color.h +++ b/src/hb-ot-color.h @@ -39,19 +39,11 @@ HB_BEGIN_DECLS /** * hb_ot_color_t: - * @red: the intensity of the red channel - * @green: the intensity of the green channel - * @blue: the intensity of the blue channel - * @alpha: the transparency - * - * Structure for holding color values. + * ARGB data type for holding color values. * * Since: REPLACEME */ -typedef struct -{ - uint8_t red, green, blue, alpha; -} hb_ot_color_t; +typedef uint32_t hb_ot_color_t; /** -- 2.7.4