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

include/core/SkPaint.h

 
 
 /*
  * Copyright 2006 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.
  */
 
 
@@ skipping 713 matching lines @@
         kUTF16_TextEncoding,    //!< the text parameters are UTF16
         kUTF32_TextEncoding,    //!< the text parameters are UTF32
         kGlyphID_TextEncoding   //!< the text parameters are glyph indices
     };
 
     TextEncoding getTextEncoding() const { return (TextEncoding)fTextEncoding; }
 
     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 {
+            kUnderlineThinknessIsValid_Flag = 0x01,

[reed1, 2014/02/24 13:26:22]

tiny nit: 

Since we're using 1 << 1 (which I like) for the 2nd bit, I think

kUnderlineThicknessIsValid_Flag = 1 << 0,

is very clear and consistent.

[h.joshi, 2014/02/24 18:21:44]

Done, fixed in new patch.

On 2014/02/24 13:26:22, reed1 wrote:

+            kUnderlinePositionIsValid_Flag = 1 << 1,
+        };
+
+        uint32_t    fFlags;       //!< Bit field to identify which values are unknown
         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

[reed1, 2014/02/24 13:26:22]

Documentation question about position.

If the thickness is 2 (lets say), what does position mean?

- the top of the "rectangle that is the underline" is 2 below the baseline?
- the center of the underline (however thick) is 2 below the baseline?
- other?

[h.joshi, 2014/02/24 18:22:39]

For Position we can have following values
 - Positive - means underline should be drawn above baseline.
 - Negative - means below baseline. 
 - Zero     - mean underline should be drawn on baseline.

On 2014/02/24 13:26:22, reed1 wrote:

+        SkScalar    fUnderlinePosition; //!< underline position, or 0 if cannot be determined
+
+        /** Method to get bit set for Font Metrics
+            @param flag  bit whose value we want to validate
+        */
+        void validate(FontMetrics::FontMetricsFlags flag) {
+            fFlags |= flag;
+        }
+
+        /** Method to check invalidate flag set for Font Metrics
+            @param flag  bit whose value we want to invalidate
+        */
+        void invalidate(FontMetrics::FontMetricsFlags flag) {
+            fFlags &= ~flag;
+        }
+
+        /** Method to get bit set for Font Metrics
+            @param flag  flag of type FontMetrics::FontMetricsFlags whose value we want to know
+            @param return true if FontMetrics bit false otherwise
+        */
+        bool isValid(uint32_t flag) const {
+          return SkToBool((fFlags & flag) == flag);
+        }

[reed1, 2014/02/24 13:26:22]

I don't think we should have any of these construction/setter helpers -- this
class is used by clients as purely something to read that Skia built. I would
remove validate/invalidate/isValid.

[h.joshi, 2014/02/24 18:21:44]

Done, removed getter and setter methods.

On 2014/02/24 13:26:22, reed1 wrote:

+
+        /**  If the fontmetrics has a valid underlinethickness, return true, and set the
+                thickness param to that value. If it doesn't return false and ignore the
+                thickness param.
+        */
+        bool hasUnderlineThickness(SkScalar* thickness) const;
+
+        /**  If the fontmetrics has a valid underlineposition, return true, and set the
+                thickness param to that value. If it doesn't return false and ignore the
+                thickness param.
+        */
+        bool hasUnderlinePosition(SkScalar* thickness) const;

[reed1, 2014/02/24 13:26:22]

These two getters are OK in my opinion, though not required. I think if we keep
them, they can both trivially be inlined (helps in the dox).

bool hasUnderlneThickness(thickenss) {
    if (fFlags & kUnderlin..._Flag) {
        *thickness = value;
        return true;
    }
    return false;
}

[h.joshi, 2014/02/24 18:21:44]

Done, these are made inline now in new patch.

On 2014/02/24 13:26:22, reed1 wrote:

+
     };
 
     /** Return the recommend spacing between lines (which will be
         fDescent - fAscent + fLeading).
         If metrics is not null, return in it the font metrics for the
         typeface/pointsize/etc. currently set in the paint.
         @param metrics      If not null, returns the font metrics for the
                             current typeface/pointsize/etc setting in this
                             paint.
         @param scale        If not 0, return width as if the canvas were scaled
@@ skipping 324 matching lines @@
 #ifdef SK_BUILD_FOR_ANDROID
     SkPaintOptionsAndroid fPaintOptionsAndroid;
 
     // In order for the == operator to work properly this must be the last field
     // in the struct so that we can do a memcmp to this field's offset.
     uint32_t        fGenerationID;
 #endif
 };
 
 #endif