Use standard inclusion scheme for FreeType headers.

Use standard inclusion scheme for FreeType headers.

The most recent FreeType release (2.5.1) has changed locations for header
files.  Using the standard way, this is not visible to applications.

[dak, 2013-12-10 12:29:30]

https://codereview.appspot.com/35580043/diff/1/lily/open-type-font.cc
File lily/open-type-font.cc (right):

https://codereview.appspot.com/35580043/diff/1/lily/open-type-font.cc#newcode26
lily/open-type-font.cc:26: #include FT_TRUETYPE_TABLES_H
This is inscrutable programming.  It relies on open-type-font.hh including
font-metric.hh which in turn includes freetype.hh which happens to include the
library header ft2build.h defining macros like FT_TRUETYPE_TABLES_H.

Please, if you use identifiers defined in a particular header file, include that
particular header file instead of assuming that if it magically seems to be
defined for some reason, everything will turn out fine.  The compile time
difference is minuscule if a header file turns out to get included more than
once.

https://codereview.appspot.com/35580043/diff/1/lily/pango-font.cc
File lily/pango-font.cc (right):

https://codereview.appspot.com/35580043/diff/1/lily/pango-font.cc#newcode25
lily/pango-font.cc:25: #include FT_XFREE86_H
This is even more reckless programming.  It relies on pango/pangoft2.h including
the particular header file from freetype that defines FT_XFREE86_H.

This issue is called "Use standard inclusion scheme for FreeType headers", and
the "standard inclusion scheme" will most definitely not state that Freetype
headers will be incidentally defined by loading some pango library.

That's not just inscrutable, it is completely out of our control.

[lemzwerg, 2013-12-10 21:20:54]

My patch is very simple, as you can see, just replacing the hardcoded header
paths with macros (this is what I refer as `standard inclusion scheme') to make
it compile.  The `reckless' programming was there before me, so to say, and I
admit that I haven't tried in any way to make it more elegant.

I fully agree that it makes sense to add

  #include "freetype.hh"

or

  #include <ft2build.h>

where FreeType header macro names are used.

[dak, 2013-12-11 00:59:50]

On 2013/12/10 21:20:54, lemzwerg wrote:
> My patch is very simple, as you can see, just replacing the hardcoded header
> paths with macros (this is what I refer as `standard inclusion scheme') to
make
> it compile.  The `reckless' programming was there before me, so to say, and I
> admit that I haven't tried in any way to make it more elegant.

No, it wasn't.  Before your change, header _names_ were used for inclusion. 
Those do not require to be "defined" before use.  After your change, header
_macros_ were used for inclusion.  Those require to be defined before use.

The problem of using macros without reliably ensuring their definition before
use was introduced by this patch.

> I fully agree that it makes sense to add
> 
>   #include "freetype.hh"
> 
> or
> 
>   #include <ft2build.h>
> 
> where FreeType header macro names are used.

What does Freetype state about where its header macro names are defined?  It is
obvious that ft2build.h will cause their definition (or this would not have
compiled) but it's not clear that this definition is not also incidental rather
than well-defined.

[lemzwerg, 2013-12-11 06:52:12]

> The problem of using macros without reliably ensuring their
> definition before use was introduced by this patch.

Right you are, I haven't thought of this.  Will post a follow-up patch to
Rietveld soon.

> What does Freetype state about where its header macro names
> are defined?  It is obvious that ft2build.h will cause their
> definition (or this would not have compiled) but it's not
> clear that this definition is not also incidental rather
> than well-defined.

Actually, it stated nothing!  I've just added a section to the FreeType API
documentation which describes that properly.  Thanks for the bug report, so to
say :-)

[lemzwerg, 2013-12-11 07:20:50]

> Will post a follow-up patch to Rietveld soon.

http://code.google.com/p/lilypond/issues/detail?id=3717