[HarfBuzz] Some optimizations

Tue Oct 16 23:56:06 PDT 2007

On Wednesday 17 October 2007, Behdad Esfahbod wrote:
> On Tue, 2007-10-16 at 03:22 -0400, Lars Knoll wrote:
> > On Monday 15 October 2007, Behdad Esfahbod wrote:
> > > Well, I went ahead and made some other changes that touched all over
> > > the code:
> > >
> > >   - Removed FT_Memory cruft, using malloc/realloc/free directly.
> >
> > I don't understand that. We have done that change in harfbuzz half a year
> > ago already in the master branch. See
> > http://gitweb.freedesktop.org/?p=harfbuzz.git;a=commitdiff;h=08bf1afd409c
> >161321eacd5761582a02e3d823ee
> >
> > Are you working against master or stable here?
>
> Well, stable.  And that's the problem.  Simon's work on getting rid of
> FreeType and adding Qt shapers was so interweaved that I had no chance
> of updating to the cleanup bits without pulling in shapers until I get
> to synching...

That's too bad, as we've done lots of cleanup work already in master. Why not 
just work on master and accept that the shapers are in there? It shouldn't 
hurt having the code in there.

> > >   - Marking harfbuzz-internal symbols HB_INTERNAL and define that
> > > macro to static in harfbuzz.c, to help compiler do a better job.
> >
> > That sounds like a valid and easy to apply patch.
>
> Yep.  Shouldn't take more than 10 minutes to redo.  Just mark _HB_* with
> it.
>
> > > Also made some other cleanups and moved pango to building harfbuzz.c
> > > instead of individual files.
> > >
> > > Anyway, seems like easiest way to deal with my changes is to wait until
> > > I merge this new tree with my harfbuzz-ng rewrite tree and make that
> > > the harfbuzz of future.
> > >
> > > In the mean time, I'm attaching the two real bugfixes here.  This
> > > should be of interest to Qt.  Just pushed them in pango-1-18 branch
> > > too.
> >
> > Thanks. Simon is travelling this week, and I'm also pretty tied up, but
> > we'll try to merge this and your last patch by next weeek.
>
> Forget about the large patch I sent, just pick up the small patch that
> fixes two bugs.  I'll get to grand-merging with Simon's work (master)
> when I make some progress with mmap()ed tables (merging is starting to
> look like a huge task btw, looking at gitk --all now).

Yes, we've done some rather large changes in the last half a year. On the 
upside, we are now basing Qt 4.4 completely on harfbuzz and have removed all 
shaping code in Qt.

Wouldn't it be better to base your work on top of master then to avoid the 
merging issues and duplicated work?

Cheers,
Lars

>
> Thanks,
> behdad
>
> > Cheers,
> > Lars
> >
> > > Summary:
> > >
> > > 2007-10-15  Behdad Esfahbod  <behdad at gnome.org>
> > >
> > >         * pango/opentype/harfbuzz-open.c (Load_ClassDef2),
> > >         (_HB_OPEN_Load_Device): Two small bug fixes backported
> > >         from trunk.  Namely: skip bad ClassDef2 entries and
> > >         skip invalid DeltaFormat device tables.  Makes Nafees
> > >         Nastaliq font work great, and avoids warnings with
> > >         some other fonts.
> > >
> > >
> > >
> > > Regards,
> > >
> > > behdad
> > >
> > > On Wed, 2007-10-10 at 20:33 -0400, Behdad Esfahbod wrote:
> > > > Hi,
> > > >
> > > > For Boston Summit this year I sat down optimizing harfbuzz buffer a
> > > > bit to avoid unnecessary copy/swap'ing when glyphs do not change. 
> > > > The idea is to use the same string for in and out, essentially
> > > > working in-place, until a change that doesn't fit in-place is needed
> > > > at which point we lazily create a separate out buffer and copy over.
> > > >
> > > > Unfortunately I didn't start this in a git-svn pango checkout and
> > > > ended up piling changes on top of each other.
> > > >
> > > > Around the final round it was about 1700 lines worth of diff, but to
> > > > find why a particular Invalid_SubTable error was being hit I needed
> > > > to see where the error is originating from, so I renamed remaining
> > > > FT_Err instances to HB_Err and made the entire code call _hb_err()
> > > > when returning an original error, so you can set a breakpoint on it. 
> > > > It helped debugging the bug in a minute, but also means that the
> > > > patch is over 7000 lines now :(.  Sorry Simon :(.
> > > >
> > > > Anyway, it's here:
> > > >
> > > >   http://bugzilla.gnome.org/show_bug.cgi?id=485559
> > > >
> > > > Quick summary:
> > > >
> > > > 2007-10-10  Behdad Esfahbod  <behdad at gnome.org>
> > > >
> > > >         Bug 485559 – Boston Summit HarfBuzz optimizations
> > > >
> > > >         * pango/opentype/*: HarfBuzz hacking to:
> > > >
> > > >           - Rename last remaining FT_Err stuff to HB_Err.
> > > >
> > > >           - Fix a couple invalid table paths to be permissive so
> > > >             fonts work better.  Particularly GDEF table for Nafees
> > > >             Nastaliq is loaded and works great now.
> > > >
> > > >           - Optimize harfbuzz buffer to not copy/swap for simple
> > > >           one-to-one and "copy" GSUB operations.
> > > >
> > > >         * pango/pango-ot*: Update to FT_Err to HB_Err renaming.
> > > >
> > > >
> > > > I think the easiest way to apply it to harfbuzz upstream is to rip
> > > > out the FT_Err and _hb_err changes out, apply it, and redo them... 
> > > > I'll give it a try if it's too much pain for Simon to do (which I
> > > > totally understand if it is), but other than that, it can use a pair
> > > > of eyes. Specially the harfbuzz-buffer changes.
> > > >
> > > > Tomorrow I'll make pango cache a single harfbuzz buffer instead of
> > > > recreating them all the time.
> > > >
> > > > A very warm feeling of doing this all is that I feel much more
> > > > comfortable with the GSUB code now.  GPOS still has a bit of mystery
> > > > to discover.  All in all quite good just in time for picking up the
> > > > rewrite again, which I'm starting next week.
> > > >
> > > > Regards,
> >
> > _______________________________________________
> > HarfBuzz mailing list
> > HarfBuzz at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/harfbuzz