[freetype2-demos] master 4e87d42 32/41: [ftinspect] Fix memory-related crash, and refactor outline-retaining...

[Werner Lemberg]

branch: master
commit 4e87d42d7e861afa21f9ae5a9b0e6c0354c4ab9f
Author: Charlie Jiang <w@chariri.moe>
Commit: Werner Lemberg <wl@gnu.org>

    [ftinspect] Fix memory-related crash, and refactor outline-retaining...
    
    graphics items. Also fix some compiler warnings.
    
    * src/ftinspect/glyphcomponents/glyphoutline.cpp,
      src/ftinspect/glyphcomponents/glyphoutline.hpp:
      Don't retain the `FT_Outline*` whose lifetime is bound to the glyph.
      Prepare the path in the ctor so the outline isn't saved.
      Add `GlyphUsingOutline` as the base class for all classes that have to
      retain a `FT_Outline` object.
    
    * src/ftinspect/glyphcomponents/glyphpointnumbers.cpp,
      src/ftinspect/glyphcomponents/glyphpointnumbers.hpp,
      src/ftinspect/glyphcomponents/glyphpoints.cpp,
      src/ftinspect/glyphcomponents/glyphpoints.hpp: Refactored.
    
    * src/ftinspect/engine/engine.hpp, src/ftinspect/panels/singular.hpp,
      src/ftinspect/engine/rendering.hpp
      Add default values to initialize member fields.
    
    * src/ftinspect/panels/singular.cpp:
      Pass the `FT_Library` into the graphics items.
    
    * src/ftinspect/models/fontinfomodels.cpp: Fix warning.
---
 src/ftinspect/engine/engine.hpp                    | 48 +++++++++---------
 src/ftinspect/engine/rendering.hpp                 |  8 +--
 src/ftinspect/glyphcomponents/glyphoutline.cpp     | 57 +++++++++++++++++-----
 src/ftinspect/glyphcomponents/glyphoutline.hpp     | 25 +++++++++-
 .../glyphcomponents/glyphpointnumbers.cpp          | 43 +++++-----------
 .../glyphcomponents/glyphpointnumbers.hpp          | 17 ++-----
 src/ftinspect/glyphcomponents/glyphpoints.cpp      | 44 +++++------------
 src/ftinspect/glyphcomponents/glyphpoints.hpp      | 10 ++--
 src/ftinspect/models/fontinfomodels.cpp            |  9 ++--
 src/ftinspect/panels/singular.cpp                  |  6 ++-
 src/ftinspect/panels/singular.hpp                  |  4 +-
 11 files changed, 138 insertions(+), 133 deletions(-)

diff --git a/src/ftinspect/engine/engine.hpp b/src/ftinspect/engine/engine.hpp
index ee71932..266599c 100644
--- a/src/ftinspect/engine/engine.hpp
+++ b/src/ftinspect/engine/engine.hpp
@@ -228,7 +228,7 @@ private:
 
   // font info
   int curFontIndex_ = -1;
-  int fontType_;
+  int fontType_ = FontType_Other;
   QString curFamilyName_;
   QString curStyleName_;
   int curNumGlyphs_ = -1;
@@ -242,11 +242,11 @@ private:
   std::vector<SFNTName> curSFNTNames_;
 
   // basic objects
-  FT_Library library_;
-  FTC_Manager cacheManager_;
-  FTC_ImageCache imageCache_;
-  FTC_SBitCache sbitsCache_;
-  FTC_CMapCache cmapCache_;
+  FT_Library library_ = NULL;
+  FTC_Manager cacheManager_ = NULL;
+  FTC_ImageCache imageCache_ = NULL;
+  FTC_SBitCache sbitsCache_ = NULL;
+  FTC_CMapCache cmapCache_ = NULL;
   EngineDefaultValues engineDefaults_;
 
   // settings
@@ -255,31 +255,31 @@ private:
   // Sometimes the font may be valid (i.e. a face object can be retrieved), but
   // the size may be invalid (e.g. non-scalable fonts).
   // Therefore, we use a fallback face for all non-rendering work.
-  FT_Face ftFallbackFace_; // Never perform rendering or write to this!
-  FT_Size ftSize_;
+  FT_Face ftFallbackFace_ = NULL; // Never perform rendering or write to this!
+  FT_Size ftSize_ = NULL;
   FT_Palette_Data paletteData_ = {};
   FT_Color* palette_ = NULL;
 
   bool antiAliasingEnabled_ = true;
   bool usingPixelSize_ = false;
-  double pointSize_;
-  double pixelSize_;
-  unsigned int dpi_;
-
-  bool doHinting_;
-  bool doAutoHinting_;
-  bool doHorizontalHinting_;
-  bool doVerticalHinting_;
-  bool doBlueZoneHinting_;
-  bool showSegments_;
-  bool embeddedBitmap_;
-  bool useColorLayer_;
+  double pointSize_ = 20;
+  double pixelSize_ = 20;
+  unsigned int dpi_ = 98;
+
+  bool doHinting_ = false;
+  bool doAutoHinting_ = false;
+  bool doHorizontalHinting_ = false;
+  bool doVerticalHinting_ = false;
+  bool doBlueZoneHinting_ = false;
+  bool showSegments_ = false;
+  bool embeddedBitmap_ = false;
+  bool useColorLayer_ = false;
   int paletteIndex_ = -1;
-  int antiAliasingTarget_;
-  bool lcdSubPixelPositioning_;
-  int renderMode_;
+  int antiAliasingTarget_ = 0;
+  bool lcdSubPixelPositioning_ = false;
+  int renderMode_ = 0; 
 
-  unsigned long loadFlags_;
+  unsigned long loadFlags_ = FT_LOAD_DEFAULT;
 
   std::unique_ptr<RenderingEngine> renderingEngine_;
 
diff --git a/src/ftinspect/engine/rendering.hpp 
b/src/ftinspect/engine/rendering.hpp
index 95fe00d..73545ee 100644
--- a/src/ftinspect/engine/rendering.hpp
+++ b/src/ftinspect/engine/rendering.hpp
@@ -53,12 +53,12 @@ public:
 private:
   Engine* engine_;
 
-  QRgb backgroundColor_;
-  QRgb foregroundColor_;
-  double gamma_;
+  QRgb backgroundColor_ = 0;
+  QRgb foregroundColor_ = 0;
+  double gamma_ = 1.8;
   QVector<QRgb> foregroundTable_;
 
-  bool lcdUsesBGR_;
+  bool lcdUsesBGR_ = false;
 };
 
 
diff --git a/src/ftinspect/glyphcomponents/glyphoutline.cpp 
b/src/ftinspect/glyphcomponents/glyphoutline.cpp
index 2e13a31..604e73d 100644
--- a/src/ftinspect/glyphcomponents/glyphoutline.cpp
+++ b/src/ftinspect/glyphcomponents/glyphoutline.cpp
@@ -92,22 +92,20 @@ GlyphOutline::GlyphOutline(const QPen& pen,
 : outlinePen_(pen)
 {
   if (glyph->format != FT_GLYPH_FORMAT_OUTLINE)
-  {
-    outline_ = NULL;
     return;
-  }
-  outline_ = &reinterpret_cast<FT_OutlineGlyph>(glyph)->outline;
+  auto outline = &reinterpret_cast<FT_OutlineGlyph>(glyph)->outline;
+  FT_Outline_Decompose(outline, &outlineFuncs, &path_);
 
   FT_BBox cbox;
 
   qreal halfPenWidth = outlinePen_.widthF();
 
-  FT_Outline_Get_CBox(outline_, &cbox);
+  FT_Outline_Get_CBox(outline, &cbox);
 
   boundingRect_.setCoords(qreal(cbox.xMin) / 64 - halfPenWidth,
-                  -qreal(cbox.yMax) / 64 - halfPenWidth,
-                  qreal(cbox.xMax) / 64 + halfPenWidth,
-                  -qreal(cbox.yMin) / 64 + halfPenWidth);
+                          -qreal(cbox.yMax) / 64 - halfPenWidth,
+                          qreal(cbox.xMax) / 64 + halfPenWidth,
+                          -qreal(cbox.yMin) / 64 + halfPenWidth);
 }
 
 
@@ -123,14 +121,47 @@ GlyphOutline::paint(QPainter* painter,
                     const QStyleOptionGraphicsItem*,
                     QWidget*)
 {
-  if (!outline_)
-    return;
   painter->setPen(outlinePen_);
+  painter->drawPath(path_);
+}
+
+
+GlyphUsingOutline::GlyphUsingOutline(FT_Library library,
+                                     FT_Glyph glyph)
+: library_(library)
+{
+  if (glyph->format != FT_GLYPH_FORMAT_OUTLINE)
+  {
+    outlineValid_ = false;
+    return;
+  }
 
-  QPainterPath path;
-  FT_Outline_Decompose(outline_, &outlineFuncs, &path);
+  auto outline = &reinterpret_cast<FT_OutlineGlyph>(glyph)->outline;
 
-  painter->drawPath(path);
+  FT_BBox cbox;
+  FT_Outline_Get_CBox(outline, &cbox);
+  outlineValid_ = true;
+  FT_Outline_New(library, static_cast<unsigned int>(outline->n_points),
+                 outline->n_contours, &outline_);
+  FT_Outline_Copy(outline, &outline_);
+
+  // XXX fix bRect size
+  boundingRect_.setCoords(qreal(cbox.xMin) / 64, -qreal(cbox.yMax) / 64,
+                          qreal(cbox.xMax) / 64, -qreal(cbox.yMin) / 64);
+}
+
+
+GlyphUsingOutline::~GlyphUsingOutline()
+{
+  if (outlineValid_)
+    FT_Outline_Done(library_, &outline_);
+}
+
+
+QRectF
+GlyphUsingOutline::boundingRect() const
+{
+  return boundingRect_;
 }
 
 
diff --git a/src/ftinspect/glyphcomponents/glyphoutline.hpp 
b/src/ftinspect/glyphcomponents/glyphoutline.hpp
index 37c53a3..728cbc7 100644
--- a/src/ftinspect/glyphcomponents/glyphoutline.hpp
+++ b/src/ftinspect/glyphcomponents/glyphoutline.hpp
@@ -5,6 +5,7 @@
 
 #pragma once
 
+#include <QPainterPath>
 #include <QGraphicsItem>
 #include <QPen>
 
@@ -27,7 +28,29 @@ public:
 
 private:
   QPen outlinePen_;
-  FT_Outline* outline_;
+  QPainterPath path_;
+  QRectF boundingRect_;
+};
+
+
+/*
+ * This class is common for all classes holding an outline.
+ * But `GlyphOutline` class itself don't need to hold the outline...
+ */
+
+class GlyphUsingOutline
+: public QGraphicsItem
+{
+public:
+  GlyphUsingOutline(FT_Library library,
+                    FT_Glyph glyph);
+  ~GlyphUsingOutline() override;
+  QRectF boundingRect() const override;
+
+protected:
+  FT_Library library_;
+  FT_Outline outline_;
+  bool outlineValid_;
   QRectF boundingRect_;
 };
 
diff --git a/src/ftinspect/glyphcomponents/glyphpointnumbers.cpp 
b/src/ftinspect/glyphcomponents/glyphpointnumbers.cpp
index f52ed9d..c364b31 100644
--- a/src/ftinspect/glyphcomponents/glyphpointnumbers.cpp
+++ b/src/ftinspect/glyphcomponents/glyphpointnumbers.cpp
@@ -10,35 +10,14 @@
 #include <QVector2D>
 
 
-GlyphPointNumbers::GlyphPointNumbers(const QPen& onP,
+GlyphPointNumbers::GlyphPointNumbers(FT_Library library,
+                                     const QPen& onP,
                                      const QPen& offP,
                                      FT_Glyph glyph)
-: onPen_(onP),
+: GlyphUsingOutline(library, glyph),
+  onPen_(onP),
   offPen_(offP)
 {
-  if (glyph->format != FT_GLYPH_FORMAT_OUTLINE)
-  {
-    outline_ = NULL;
-    return;
-  }
-  outline_ = &reinterpret_cast<FT_OutlineGlyph>(glyph)->outline;
-
-  FT_BBox cbox;
-
-  FT_Outline_Get_CBox(outline_, &cbox);
-
-  // XXX fix bRect size
-  boundingRect_.setCoords(qreal(cbox.xMin) / 64,
-                  -qreal(cbox.yMax) / 64,
-                  qreal(cbox.xMax) / 64,
-                  -qreal(cbox.yMin) / 64);
-}
-
-
-QRectF
-GlyphPointNumbers::boundingRect() const
-{
-  return boundingRect_;
 }
 
 
@@ -47,10 +26,10 @@ GlyphPointNumbers::paint(QPainter* painter,
                          const QStyleOptionGraphicsItem* option,
                          QWidget*)
 {
-  if (!outline_)
+  if (!outlineValid_)
     return;
-  const qreal lod = option->levelOfDetailFromTransform(
-                              painter->worldTransform());
+  auto lod = QStyleOptionGraphicsItem::levelOfDetailFromTransform(
+    painter->worldTransform());
 
   // don't draw point numbers if magnification is too small
   if (lod >= 10)
@@ -74,9 +53,9 @@ GlyphPointNumbers::paint(QPainter* painter,
     painter->scale(1 / lod, 1 / lod);
 #endif
 
-    FT_Vector* points = outline_->points;
-    FT_Short* contours = outline_->contours;
-    char* tags = outline_->tags;
+    FT_Vector* points = outline_.points;
+    FT_Short* contours = outline_.contours;
+    char* tags = outline_.tags;
 
     QVector2D octants[8] = { QVector2D(1, 0),
                              QVector2D(0.707f, -0.707f),
@@ -89,7 +68,7 @@ GlyphPointNumbers::paint(QPainter* painter,
 
 
     short ptIdx = 0;
-    for (int contIdx = 0; contIdx < outline_->n_contours; contIdx++ )
+    for (int contIdx = 0; contIdx < outline_.n_contours; contIdx++ )
     {
       for (;;)
       {
diff --git a/src/ftinspect/glyphcomponents/glyphpointnumbers.hpp 
b/src/ftinspect/glyphcomponents/glyphpointnumbers.hpp
index ded1ce1..75f3f97 100644
--- a/src/ftinspect/glyphcomponents/glyphpointnumbers.hpp
+++ b/src/ftinspect/glyphcomponents/glyphpointnumbers.hpp
@@ -5,23 +5,16 @@
 
 #pragma once
 
-#include <QGraphicsItem>
-#include <QPen>
-
-#include <ft2build.h>
-#include <freetype/freetype.h>
-#include <freetype/ftglyph.h>
-#include <freetype/ftoutln.h>
-
+#include "glyphoutline.hpp"
 
 class GlyphPointNumbers
-: public QGraphicsItem
+: public GlyphUsingOutline
 {
 public:
-  GlyphPointNumbers(const QPen& onPen,
+  GlyphPointNumbers(FT_Library library,
+                    const QPen& onPen,
                     const QPen& offPen,
                     FT_Glyph glyph);
-  QRectF boundingRect() const override;
   void paint(QPainter* painter,
              const QStyleOptionGraphicsItem* option,
              QWidget* widget) override;
@@ -29,8 +22,6 @@ public:
 private:
   QPen onPen_;
   QPen offPen_;
-  FT_Outline* outline_;
-  QRectF boundingRect_;
 };
 
 
diff --git a/src/ftinspect/glyphcomponents/glyphpoints.cpp 
b/src/ftinspect/glyphcomponents/glyphpoints.cpp
index 4b35670..56a9c09 100644
--- a/src/ftinspect/glyphcomponents/glyphpoints.cpp
+++ b/src/ftinspect/glyphcomponents/glyphpoints.cpp
@@ -9,36 +9,14 @@
 #include <QStyleOptionGraphicsItem>
 
 
-GlyphPoints::GlyphPoints(const QPen& onP,
+GlyphPoints::GlyphPoints(FT_Library library,
+                         const QPen& onP,
                          const QPen& offP,
                          FT_Glyph glyph)
-: onPen_(onP),
+: GlyphUsingOutline(library, glyph),
+  onPen_(onP),
   offPen_(offP)
 {
-  if (glyph->format != FT_GLYPH_FORMAT_OUTLINE)
-  {
-    outline_ = NULL;
-    return;
-  }
-  outline_ = &reinterpret_cast<FT_OutlineGlyph>(glyph)->outline;
-
-  FT_BBox cbox;
-
-  qreal halfPenWidth = qMax(onPen_.widthF(), offPen_.widthF()) / 2;
-
-  FT_Outline_Get_CBox(outline_, &cbox);
-
-  boundingRect_.setCoords(qreal(cbox.xMin) / 64 - halfPenWidth,
-                  -qreal(cbox.yMax) / 64 - halfPenWidth,
-                  qreal(cbox.xMax) / 64 + halfPenWidth,
-                  -qreal(cbox.yMin) / 64 + halfPenWidth);
-}
-
-
-QRectF
-GlyphPoints::boundingRect() const
-{
-  return boundingRect_;
 }
 
 
@@ -47,7 +25,7 @@ GlyphPoints::paint(QPainter* painter,
                    const QStyleOptionGraphicsItem* option,
                    QWidget*)
 {
-  if (!outline_)
+  if (!outlineValid_)
     return;
 
   const qreal lod = option->levelOfDetailFromTransform(
@@ -90,21 +68,21 @@ GlyphPoints::paint(QPainter* painter,
     qreal onRadius = onPen_.widthF() / lod;
     qreal offRadius = offPen_.widthF() / lod;
 
-    for (int i = 0; i < outline_->n_points; i++)
+    for (int i = 0; i < outline_.n_points; i++)
     {
-      if (outline_->tags[i] & FT_CURVE_TAG_ON)
+      if (outline_.tags[i] & FT_CURVE_TAG_ON)
       {
         painter->setBrush(onBrush);
-        painter->drawEllipse(QPointF(qreal(outline_->points[i].x) / 64,
-                                     -qreal(outline_->points[i].y) / 64),
+        painter->drawEllipse(QPointF(qreal(outline_.points[i].x) / 64,
+                                     -qreal(outline_.points[i].y) / 64),
                              onRadius,
                              onRadius);
       }
       else
       {
         painter->setBrush(offBrush);
-        painter->drawEllipse(QPointF(qreal(outline_->points[i].x) / 64,
-                                     -qreal(outline_->points[i].y) / 64),
+        painter->drawEllipse(QPointF(qreal(outline_.points[i].x) / 64,
+                                     -qreal(outline_.points[i].y) / 64),
                              offRadius,
                              offRadius);
       }
diff --git a/src/ftinspect/glyphcomponents/glyphpoints.hpp 
b/src/ftinspect/glyphcomponents/glyphpoints.hpp
index d0f5d66..8dde35e 100644
--- a/src/ftinspect/glyphcomponents/glyphpoints.hpp
+++ b/src/ftinspect/glyphcomponents/glyphpoints.hpp
@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include "glyphoutline.hpp"
+
 #include <QGraphicsItem>
 #include <QPen>
 
@@ -15,13 +17,13 @@
 
 
 class GlyphPoints
-: public QGraphicsItem
+: public GlyphUsingOutline
 {
 public:
-  GlyphPoints(const QPen& onPen,
+  GlyphPoints(FT_Library library,
+              const QPen& onPen,
               const QPen& offPen,
               FT_Glyph glyph);
-  QRectF boundingRect() const override;
   void paint(QPainter* painter,
              const QStyleOptionGraphicsItem* option,
              QWidget* widget) override;
@@ -29,8 +31,6 @@ public:
 private:
   QPen onPen_;
   QPen offPen_;
-  FT_Outline* outline_;
-  QRectF boundingRect_;
 };
 
 
diff --git a/src/ftinspect/models/fontinfomodels.cpp 
b/src/ftinspect/models/fontinfomodels.cpp
index 107c42d..e99b757 100644
--- a/src/ftinspect/models/fontinfomodels.cpp
+++ b/src/ftinspect/models/fontinfomodels.cpp
@@ -5,6 +5,8 @@
 #include "fontinfomodels.hpp"
 #include "../engine/engine.hpp"
 
+#include <cstdint>
+
 int
 FixedSizeInfoModel::rowCount(const QModelIndex& parent) const
 {
@@ -474,8 +476,8 @@ CompositeGlyphsInfoModel::rowCount(const QModelIndex& 
parent) const
 {
   if (!parent.isValid())
     return static_cast<int>(glyphs_.size());
-  auto id = parent.internalId();
-  if (id < 0 || id >= nodes_.size())
+  uint64_t id = parent.internalId();
+  if (id >= nodes_.size())
     return 0;
   auto gid = nodes_[id].glyphIndex;
   auto iter = glyphMapper_.find(gid);
@@ -594,8 +596,7 @@ 
generatePositionTransformationText(CompositeGlyphInfo::SubGlyph const& info)
                      QString::number(info.transformation[3]));
     break;
   }
-
-  auto pos = info.position;
+  
   switch (info.positionType)
   {
   case CompositeGlyphInfo::SubGlyph::PT_Offset:
diff --git a/src/ftinspect/panels/singular.cpp 
b/src/ftinspect/panels/singular.cpp
index a2f327c..2cb2a20 100644
--- a/src/ftinspect/panels/singular.cpp
+++ b/src/ftinspect/panels/singular.cpp
@@ -115,7 +115,8 @@ SingularTab::drawGlyph()
 
     if (showPointsCheckBox_->isChecked())
     {
-      currentGlyphPointsItem_ = new GlyphPoints(graphicsDefault_->onPen,
+      currentGlyphPointsItem_ = new GlyphPoints(engine_->ftLibrary(),
+                                                graphicsDefault_->onPen,
                                                 graphicsDefault_->offPen,
                                                 glyph);
       currentGlyphPointsItem_->setZValue(1);
@@ -124,7 +125,8 @@ SingularTab::drawGlyph()
       if (showPointNumbersCheckBox_->isChecked())
       {
         currentGlyphPointNumbersItem_
-          = new GlyphPointNumbers(graphicsDefault_->onPen,
+          = new GlyphPointNumbers(engine_->ftLibrary(),
+                                  graphicsDefault_->onPen,
                                   graphicsDefault_->offPen,
                                   glyph);
         currentGlyphPointNumbersItem_->setZValue(1);
diff --git a/src/ftinspect/panels/singular.hpp 
b/src/ftinspect/panels/singular.hpp
index 6a1a5d4..ffb9cc1 100644
--- a/src/ftinspect/panels/singular.hpp
+++ b/src/ftinspect/panels/singular.hpp
@@ -63,8 +63,8 @@ protected:
   void resizeEvent(QResizeEvent* event) override;
 
 private:
-  int currentGlyphIndex_;
-  int currentGlyphCount_;
+  int currentGlyphIndex_ = 0;
+  int currentGlyphCount_ = 0;
 
   Engine* engine_;