Parser for new fonts.xml format

src/ports/SkFontConfigParser_android.h

 /*
  * Copyright 2011 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SKFONTCONFIGPARSER_ANDROID_H_
 #define SKFONTCONFIGPARSER_ANDROID_H_
 
 #include "SkTypes.h"
 
 #include "SkPaintOptionsAndroid.h"
 #include "SkString.h"
 #include "SkTDArray.h"
 
 struct FontFileInfo {
-    FontFileInfo() : fIndex(0) { }
+    FontFileInfo() : fIndex(0), fWeight(0) { }
 
     SkString              fFileName;
     int                   fIndex;
     SkPaintOptionsAndroid fPaintOptions;
+    int                   fWeight;
 };
 
 /**
  * The FontFamily data structure is created during parsing and handed back to
  * Skia to fold into its representation of font families. fNames is the list of
  * font names that alias to a font family. fontFileArray is the list of information

[palmer, 2014/08/06 21:05:30]

"fontFileArray" should be "fFontFiles"? (Which itself is a a bad variable name;
why not |fFontFileInfos|?)

[tomhudson, 2014/08/07 14:39:43]

Comment fix done. I disagree on the naming as a general rule: we wouldn't praise
"int fInt", we'd want the name to tell us something about the meaning of the
variable, not just its types (and this is a bad pattern ALL OVER CHROME). Here
we have a struct with a reasonable name (personally I'd consider shortening the
struct name to FileInfo but move it into the SkFontConfigParser namespace?), so
the field name is going to echo it some; I find |fFonts| to be clearer than
fFontInfos.

(We expect in a future Android release to support multiple fonts inside the same
file, anyway, which is what fIndex is for.)

  * about each file.  Order is the priority order for the font. This is

[palmer, 2014/08/06 21:05:30]

It's |order|, not |Order|; but also, why not |fOrder|?

Overall, better names would obviate this documentation, and then you wouldn't
have inconsistencies between the docs and the code.

Does |fVariant| need explanation? Does it need to be initialized in the
constructor?

[tomhudson, 2014/08/07 14:39:43]

I suspect this is code archaeology: in my experience with older Skia code,
protected class members get fCamelCase, while public struct members
haveNoLeadingTag. That's not what the style guide says, though, so fixed.

(Policy aside: this was a last minute change for Android functionality, so we
were focused on minimal changes, not even style fix-ups. It was bad to not
update the comment, however).
I'd argue that it's bad doc, but that good doc is not replaced by good naming.
Heavily revised.
Was already initialized in constructor?

  * used internally to determine the order in which to place fallback fonts as
  * they are read from the configuration files.
  */
 struct FontFamily {
-    FontFamily() : fIsFallbackFont(false), order(-1) {}
+    FontFamily()
+        : fVariant(SkPaintOptionsAndroid::kDefault_Variant)
+        , order(-1)
+        , fIsFallbackFont(false) { }
 
-    SkTArray<SkString> fNames;
-    SkTArray<FontFileInfo> fFontFiles;
-    bool fIsFallbackFont;
-    int order; // only used internally by SkFontConfigParser
+    SkTArray<SkString>                 fNames;
+    SkTArray<FontFileInfo>             fFontFiles;
+    SkLanguage                         fLanguage;
+    SkPaintOptionsAndroid::FontVariant fVariant;
+    int                                order; // only used internally by SkFontConfigParser
+    bool                               fIsFallbackFont;
 };
 
 namespace SkFontConfigParser {
 
 /**
  * Parses all system font configuration files and returns the results in an
  * array of FontFamily structures.
  */
 void GetFontFamilies(SkTDArray<FontFamily*> &fontFamilies);
 
 /**
  * Parses all test font configuration files and returns the results in an
  * array of FontFamily structures.
  */
 void GetTestFontFamilies(SkTDArray<FontFamily*> &fontFamilies,
                          const char* testMainConfigFile,
                          const char* testFallbackConfigFile);
 
 SkString GetLocale();
 
 } // SkFontConfigParser namespace
 
 #endif /* SKFONTCONFIGPARSER_ANDROID_H_ */