Adding code to calculate Underline Thickness from Font Metrics, this will be useful when Skia is us…

include/core/SkPaint.h

Index: include/core/SkPaint.h
diff --git a/include/core/SkPaint.h b/include/core/SkPaint.h
index abb859950c5bb6a66aecccded690ca8cd6ae02a2..17dda8601b515c34fcc8fa17818e070b04aeac31 100644
--- a/include/core/SkPaint.h
+++ b/include/core/SkPaint.h
@@ -731,17 +731,45 @@ public:
     void setTextEncoding(TextEncoding encoding);
 
     struct FontMetrics {
+        /** Flags which indicate the confidence level of various metrics.
+            A set flag indicates that the metric may be trusted.
+        */
+        enum FontMetricsFlags {

[bungeman-skia, 2014/02/20 16:52:46]

Since this is in FontMetrics (already namespaced) this should be called
ValidityFlags. There is no need to repeat 'FontMetrics' here.

+            kUnderlineThinknessIsValid_FontMetricFlag = 0x01,
+            kUnderlinePositionIsValid_FontMetricsflag = 1 << 1,

[bungeman-skia, 2014/02/20 16:52:46]

kUnderlineThinkness_ValidityFlag
kUnderlinePosition_ValidityFlag

+        };
+
+        uint32_t    fFontMetricsFlag;       //!< Bit field to identify which values are unknown

[bungeman-skia, 2014/02/20 16:52:46]

fValidityFlags

         SkScalar    fTop;       //!< The greatest distance above the baseline for any glyph (will be <= 0)
         SkScalar    fAscent;    //!< The recommended distance above the baseline (will be <= 0)
         SkScalar    fDescent;   //!< The recommended distance below the baseline (will be >= 0)
         SkScalar    fBottom;    //!< The greatest distance below the baseline for any glyph (will be >= 0)
         SkScalar    fLeading;   //!< The recommended distance to add between lines of text (will be >= 0)
-        SkScalar    fAvgCharWidth;  //!< the average charactor width (>= 0)
-        SkScalar    fMaxCharWidth;  //!< the max charactor width (>= 0)
+        SkScalar    fAvgCharWidth;  //!< the average character width (>= 0)
+        SkScalar    fMaxCharWidth;  //!< the max character width (>= 0)
         SkScalar    fXMin;      //!< The minimum bounding box x value for all glyphs
         SkScalar    fXMax;      //!< The maximum bounding box x value for all glyphs
         SkScalar    fXHeight;   //!< The height of an 'x' in px, or 0 if no 'x' in face
         SkScalar    fCapHeight;  //!< The cap height (> 0), or 0 if cannot be determined.
+        SkScalar    fUnderlineThickness; //!< underline thickness, or 0 if cannot be determined
+        SkScalar    fUnderlinePosition; //!< underline position, or 0 if cannot be determined
+
+        /** Method to get bit set for Font Metrics
+            @param bitSet  bit whose value we want to know is set or not
+        */
+        void setFontMetricsFlag(FontMetrics::FontMetricsFlags bitSet);

[bungeman-skia, 2014/02/20 16:52:46]

validate(uint32_t flags) or validate(ValidityFlags flag) or remove.

Note that these getters and setters are somewhat superfluous. We only expect
Skia to be setting or unsetting these flags, and Skia is fine with inline bit
twiddling. Something like isValid is nice to have for users though. Since the
implementations of these three methods are trivial, they should be inline here
instead of implemented in the cpp file. No need to have the overhead of a
function call for a bit twiddle.

+
+        /** Method to get bit set for Font Metrics
+            @param bitSet  bit whose value we want to know is set or not
+        */
+        void unsetFontMetricsFlag(FontMetrics::FontMetricsFlags bitSet);

[bungeman-skia, 2014/02/20 16:52:46]

invalidate(uint32_t flags) or invalidate(ValidityFlags flag) or remove.

+
+        /** Method to get bit set for Font Metrics
+            @param bitSet  bit whose value we want to know is set or not
+            @param return true if FontMetrics bit i set false otherwise
+        */
+        bool checkFontMetricsFlag(FontMetrics::FontMetricsFlags bitSet) const;

[bungeman-skia, 2014/02/20 16:52:46]

isValid(uint32_t flags) or isValid(ValidityFlags flag)

Note that by taking an uint32_t here, it is possible to write something like

fontMetrics->isValid(SkPaint::FontMetrics::kUnderlineThinkness_ValidityFlag |
SkPaint::FontMetrics::kUnderlinePosition_ValidityFlag)

if one wishes to check both of these at the same time. In theory if the
implementation is inline here, something like

fontMetrics->isValid(SkPaint::FontMetrics::kUnderlineThinkness_ValidityFlag) &&
fontMetrics->isValid(SkPaint::FontMetrics::kUnderlinePosition_ValidityFlag)

should resolve to the same code with basic optimizations.


I believe that part of reed's earlier comment was that the user can still just
do

(fontMetrics->fValidityFlags &
SkPaint::FontMetrics::kUnderlineThinkness_ValidityFlag) &&
(fontMetrics->fValidityFlags &
SkPaint::FontMetrics::kUnderlinePosition_ValidityFlag)

themselves, something like isValid is just syntactic sugar which isn't really
required.

+
     };
 
     /** Return the recommend spacing between lines (which will be