[instancer] fix potential memory leak for compiled glyph bytes
Also calculate max_offsets after glyph bytes are compiled, cause byte
length of a glyph might change after compile
diff --git a/src/OT/glyf/SubsetGlyph.hh b/src/OT/glyf/SubsetGlyph.hh
index c275048..795925b 100644
--- a/src/OT/glyf/SubsetGlyph.hh
+++ b/src/OT/glyf/SubsetGlyph.hh
@@ -21,18 +21,10 @@
bool serialize (hb_serialize_context_t *c,
bool use_short_loca,
- const hb_subset_plan_t *plan,
- hb_font_t *font)
+ const hb_subset_plan_t *plan)
{
TRACE_SERIALIZE (this);
- if (font)
- {
- const OT::glyf_accelerator_t &glyf = *font->face->table.glyf;
- if (!this->compile_bytes_with_deltas (plan, font, glyf))
- return_trace (false);
- }
-
hb_bytes_t dest_glyph = dest_start.copy (c);
dest_glyph = hb_bytes_t (&dest_glyph, dest_glyph.length + dest_end.copy (c).length);
unsigned int pad_length = use_short_loca ? padding () : 0;
diff --git a/src/OT/glyf/glyf.hh b/src/OT/glyf/glyf.hh
index 60a253b..d3e6ed4 100644
--- a/src/OT/glyf/glyf.hh
+++ b/src/OT/glyf/glyf.hh
@@ -43,14 +43,13 @@
bool serialize (hb_serialize_context_t *c,
Iterator it,
bool use_short_loca,
- const hb_subset_plan_t *plan,
- hb_font_t *font)
+ const hb_subset_plan_t *plan)
{
TRACE_SERIALIZE (this);
unsigned init_len = c->length ();
for (auto &_ : it)
- if (unlikely (!_.serialize (c, use_short_loca, plan, font)))
+ if (unlikely (!_.serialize (c, use_short_loca, plan)))
return false;
/* As a special case when all glyph in the font are empty, add a zero byte
@@ -76,9 +75,6 @@
glyf *glyf_prime = c->serializer->start_embed <glyf> ();
if (unlikely (!c->serializer->check_success (glyf_prime))) return_trace (false);
- hb_vector_t<glyf_impl::SubsetGlyph> glyphs;
- _populate_subset_glyphs (c->plan, glyphs);
-
hb_font_t *font = nullptr;
if (c->plan->normalized_coords)
{
@@ -86,52 +82,60 @@
if (unlikely (!font)) return false;
}
- auto padded_offsets =
- + hb_iter (glyphs)
- | hb_map (&glyf_impl::SubsetGlyph::padded_size)
- ;
+ hb_vector_t<unsigned> padded_offsets;
+ unsigned num_glyphs = c->plan->num_output_glyphs ();
+ if (unlikely (!padded_offsets.resize (num_glyphs)))
+ return false;
+
+ hb_vector_t<glyf_impl::SubsetGlyph> glyphs;
+ if (!_populate_subset_glyphs (c->plan, font, glyphs))
+ return false;
+
+ if (font)
+ hb_font_destroy (font);
+
+ unsigned max_offset = 0;
+ for (unsigned i = 0; i < num_glyphs; i++)
+ {
+ padded_offsets[i] = glyphs[i].padded_size ();
+ max_offset += padded_offsets[i];
+ }
bool use_short_loca = false;
if (likely (!c->plan->force_long_loca))
- {
- unsigned max_offset = + padded_offsets | hb_reduce (hb_add, 0);
use_short_loca = max_offset < 0x1FFFF;
- }
-
- if (!glyf_prime->serialize (c->serializer, glyphs.writer (), use_short_loca, c->plan, font))
- return false;
if (!use_short_loca) {
- padded_offsets =
- + hb_iter (glyphs)
- | hb_map (&glyf_impl::SubsetGlyph::length)
- ;
+ for (unsigned i = 0; i < num_glyphs; i++)
+ padded_offsets[i] = glyphs[i].length ();
}
- if (font)
+ if (!glyf_prime->serialize (c->serializer, glyphs.writer (), use_short_loca, c->plan))
{
- if (!c->plan->pinned_at_default)
- _free_compiled_subset_glyphs (&glyphs);
- hb_font_destroy (font);
+ if (c->plan->normalized_coords && !c->plan->pinned_at_default)
+ _free_compiled_subset_glyphs (glyphs, glyphs.length - 1);
+ return false;
}
if (unlikely (c->serializer->in_error ())) return_trace (false);
+
return_trace (c->serializer->check_success (glyf_impl::_add_loca_and_head (c->plan,
- padded_offsets,
+ padded_offsets.iter (),
use_short_loca)));
}
- void
+ bool
_populate_subset_glyphs (const hb_subset_plan_t *plan,
+ hb_font_t *font,
hb_vector_t<glyf_impl::SubsetGlyph> &glyphs /* OUT */) const;
hb_font_t *
_create_font_for_instancing (const hb_subset_plan_t *plan) const;
- void _free_compiled_subset_glyphs (hb_vector_t<glyf_impl::SubsetGlyph> *glyphs) const
+ void _free_compiled_subset_glyphs (hb_vector_t<glyf_impl::SubsetGlyph> &glyphs, unsigned index) const
{
- for (auto _ : *glyphs)
- _.free_compiled_bytes ();
+ for (unsigned i = 0; i <= index && i < glyphs.length; i++)
+ glyphs[i].free_compiled_bytes ();
}
protected:
@@ -393,14 +397,16 @@
};
-inline void
+inline bool
glyf::_populate_subset_glyphs (const hb_subset_plan_t *plan,
+ hb_font_t *font,
hb_vector_t<glyf_impl::SubsetGlyph>& glyphs /* OUT */) const
{
OT::glyf_accelerator_t glyf (plan->source);
unsigned num_glyphs = plan->num_output_glyphs ();
- if (!glyphs.resize (num_glyphs)) return;
+ if (!glyphs.resize (num_glyphs)) return false;
+ unsigned idx = 0;
for (auto p : plan->glyph_map->iter ())
{
unsigned new_gid = p.second;
@@ -422,7 +428,20 @@
subset_glyph.drop_hints_bytes ();
else
subset_glyph.dest_start = subset_glyph.source_glyph.get_bytes ();
+
+ if (font)
+ {
+ if (unlikely (!subset_glyph.compile_bytes_with_deltas (plan, font, glyf)))
+ {
+ // when pinned at default, only bounds are updated, thus no need to free
+ if (!plan->pinned_at_default && idx > 0)
+ _free_compiled_subset_glyphs (glyphs, idx - 1);
+ return false;
+ }
+ idx++;
+ }
}
+ return true;
}
inline hb_font_t *
