[HarfBuzz] HarfBuzz rewrite

[Lars Knoll]

On Wednesday 03 January 2007 18:30, Behdad Esfahbod wrote:
> On Wed, 2007-01-03 at 09:16 -0500, Simon Hausmann wrote:
> > On Wednesday 03 January 2007 10:41, Behdad Esfahbod wrote:
> > > On Wed, 2007-01-03 at 03:02 -0500, Lars Knoll wrote:
> > > > This is a little problematic, as the compiler is free to add paddings
> > > > inside
> > > > the structs (actually it's dependent on the ABI you have on the OS).
> > > > It
> > > > should work fine on x86, but you might run into problems on risk
> > > > architectures. gcc has a workaround for this (the packed attribute).
> > > > This is
> > > > something we'll need to look into for other compilers (which we
> > > > support with
> > > > Qt).
> > >
> > > Thanks Lars.  I already faced such problems as padding and alignment
> > > and have solved some, and have plans to solve the rest.  The neat thing
> > > is, it's a single place (the DEFINE_INT_TYPE macro) you need to tweak
> > > for that.  You can use an array of chars for example, etc etc.  I'll
> > > get to that when the rest is done.
> >
> > Lars and I just had a look at it and we're afraid that it may not be
> > possible to get the padding/alignment working portably for all compilers
> > :-(
>
> Even if the only basic type I use in my structs is char?  I did my Tag
> type as a char[4] and that fixed the alignment problem on gcc.  I can
> replace all other type definitions with things like "char a,b,c,d".  Do
> you have any examples that that doesn't work?

Well, you might be lucky, or you might not. We've had too many issues with 
these kinds of things before to trust them. Your approach is unfortunately 
not portable as the C/C++ specs explicitly state that padding might be added 
by the compiler/ABI.

> > Maybe it'd be easier to just have (initially generate) little wrapper
> > classes with member functions. For example:
> >
> > struct OpenTypeFontFace
> > {
> >     enum Offsets {
> >         Tag_Offset = 0,
> >         SFntVersion_Offset = 4,
> >         NumTables_Offset = 8,
> >         SearchRange_Offset = 12,
> >         ...
> >     };
> >     OpenTypeFontFace(const char *_data) : data(_data) {}
> >
> >     uint32 tag() const { return fromBE32<uint32>(data + Tag_Offset); }
> >     uint32 sfntVersion() const { return fromBE32<uint32>(data +
> > SFntVersion_Offset); } uint32 numTables() const { return
> > fromBE32<uint32>(data + NumTables_Offset); } ...
> >
> > private:
> >     const char *data;
> > };
> >
> > That keeps the code using it still very readable/maintainable and should
> > work nicely for all platforms.
>
> That works too.  But needs some more work.  But do you have to use a
> wrapper at all?  Can't you do the same thing using "this"?

The work needed is not a whole lot more. Most of the code could be generated. 
Using it is almost exactly the same as your approach (you write "table.tag()" 
instead of "table.tag").

You could use "this" as well, but that would require ugly reinterpret_cast's 
all over the place. The way we have above, you can just use the object value 
based on the stack. Both approaches will generate pretty much exactly the 
same assembler code, but the approach above doesn't require any casting.

Cheers,
Lars