SkOnce in is_lcd_supported instead of hand rolled double-checked locking.

src/ports/SkFontHost_FreeType.cpp

 
 /*
  * 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.
  */
 
+#include "SkAdvancedTypefaceMetrics.h"
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkColorPriv.h"
 #include "SkDescriptor.h"
 #include "SkFDot6.h"
 #include "SkFloatingPoint.h"
 #include "SkFontHost.h"
 #include "SkFontHost_FreeType_common.h"
 #include "SkGlyph.h"
 #include "SkMask.h"
 #include "SkMaskGamma.h"
 #include "SkOTUtils.h"
-#include "SkAdvancedTypefaceMetrics.h"
+#include "SkOnce.h"
 #include "SkScalerContext.h"
 #include "SkStream.h"
 #include "SkString.h"
 #include "SkTemplates.h"
 #include "SkThread.h"
 
 #if defined(SK_CAN_USE_DLOPEN)
 #include <dlfcn.h>
 #endif
 #include <ft2build.h>
@@ skipping 121 matching lines @@
 #endif
     }
 #else
     gLCDSupport = false;
 #endif
     gLCDSupportValid = true;
 
     return true;
 }
 
+static void determine_lcd_support(int) {
+    SkAutoMutexAcquire lock(gFTMutex);
+    if (!gLCDSupportValid) {
+        // This will determine LCD support as a side effect.
+        InitFreetype();
+        FT_Done_FreeType(gFTLibrary);
+    }
+    SkASSERT(gLCDSupportValid);
+}
+
 // Lazy, once, wrapper to ask the FreeType Library if it can support LCD text
 static bool is_lcd_supported() {
-    if (!gLCDSupportValid) {
-        SkAutoMutexAcquire  ac(gFTMutex);
+    SK_DECLARE_STATIC_ONCE(once);
+    SkOnce(&once, determine_lcd_support, 0);
 
-        if (!gLCDSupportValid) {
-            InitFreetype();
-            FT_Done_FreeType(gFTLibrary);
-        }
-    }
+    SkAutoMutexAcquire lock(gFTMutex);

[bungeman-skia, 2014/02/05 18:02:04]

Why are we still taking this lock, just to return a global?

[mtklein, 2014/02/05 18:10:26]

You know, that's a thought provoking question.

I was going to say, for consistency with the rest of the locking discipline
around these freetype globals.  gLCDSupport, gLCDExtra, and gLCDSupportValid are
weird near-consts that are the exception to the normal locking discipline rather
than the norm.

But now I'm looking at it, and clearly this is more complicated but no different
than just writing

static bool is_lcd_supported() {
  SkAutoMutexAcquire lock(gFTMutex);
  if (!gLCDSupportValid) { ... }
  return gLCDSupport;
}

It bothers me to have two locks involved here.  Going back to drawing board.

     return gLCDSupport;
 }
 
 class SkScalerContext_FreeType : public SkScalerContext_FreeType_Base {
 public:
     SkScalerContext_FreeType(SkTypeface*, const SkDescriptor* desc);
     virtual ~SkScalerContext_FreeType();
 
     bool success() const {
         return fFaceRec != NULL &&
@@ skipping 1494 matching lines @@
         *style = (SkTypeface::Style) tempStyle;
     }
     if (isFixedPitch) {
         *isFixedPitch = FT_IS_FIXED_WIDTH(face);
     }
 
     FT_Done_Face(face);
     FT_Done_FreeType(library);
     return true;
 }