[Devel] Re: Patch catalog for OpenType code

[Lars Knoll]

Hi Owen,

thanks a lot for this work. The list is quite informative and should make it 
possible to get back to a unified codebase rather soon.

> I've now finished an initial patch at cataloging all the various changes
> to the different branches. The result can be found at:
>
>  http://people.redhat.com/otaylor/opentype-patches/
>
> The red letters indicate that the particular patch needs attention for
> that codebase.
>
> The 5 patches under "Patches from Qt" that are marked with '?'
> characters are places where I felt that I needed more information about
> the changes. Lars's comments would be particularly valuable here, but
> I'd be interested in input from Werner as well (especially about
> 'qt-double-subscript')
> 
> It should be noted that the 'qt-' patches are rather approximate, since
> instead of deriving them from CVS history as I did for the others, I
> took the opentype/ directory from qt-3.2.0, munged it a bit, diffed that
> against pango-1.0.5, and then split the result apart by hand.

It looks like you more or less separated them correctly. Some of these changes 
were done merely for performance reasons. These patches are:
qt-bin-searches
qt-check-property
parts of qt-one-feature-at-a-time
parts of qt-optimise-out-switches
and the code inlining.

I did extensive measurements using timing and (k)cachegrind for this. The most 
important changes here are qt-check-property and avoiding the constant 
creation of temporary strings in qt-one-feature-at-a-time. Code inlining also 
helped a lot. Alltogether this gave us a speedup of a factor of around 5 
(IIRC) for shaping of indic text.

In your comment to qt-check-property you're right, with your comment. Caching 
of the class on a per glyph basis could help even more, however I wanted to 
avoid such bigger changes while working on the code.

qt_double_subscript: 

I unfortunately don't really remember what the code broke, but I think it had 
to do with the handling of akhant ligatures in mangal in the properties that 
get applied after 'akhn'.

The OT specs clearly say that a glyph not defined in the class definition 
table falls into class 0, but if I understand the code correctly that's not 
what it does. It asks some BacktrackClassDef.Defined array for the class and 
if this doesn't give true, it resets the class to 0. As I said in the comment 
I don't know what this is supposed to do, but I had some shaping problems 
with it. 

We should find out if this code is really needed (ie the patch is wrong). If 
not, this could really simplify some parts of the code I didn't want to touch 
until now.

qt-ligature-component: 

Another fix for indic. I think this one was to fix positioning of the reph 
glyph on some characters. The problem appeared either with Mangal or 
RagHindi.

qt-one-feature-at-a-time:

There were two reasons for the patch. First of all, we can cache the string 
and avoid the mallocs and frees we had before with XXX_Apply_String (leading 
to a huge speed improvement). The other reason is that the MS specs on indic 
scripts state that features have to be separately for each syllable and one 
at a time. XXX_Apply_String didn't allow this in a halfway efficient way.

Before I did that change, we tried to pass the whole string to 
XXX_Apply_String, but that broke badly for eg. tibetan.
 
> And in the opposite direction the Pango codebase shares a lot of
> systematic changes with the Qt code that is derived from it
> as compared to the original freetype code.
>
> So, where there is a Pango version of a freetype- or qt- patch, it's
> likely to be easier to apply to Qt or FreeType than the original patch.

Probably, since pango is sort of in the middle between the versions. I'll go 
through the diffs in the next days and will try to add them to Qt (depending 
on the severity of the problem to 3.2.x or to our main development branch). 

I would be really happy if we could reach a common codebase in the near 
future.

Cheers,
Lars