[freetype2-demos] gsoc-2022-chariri-final edda9b0f 2/6: [ftinspect] Move out font file managing/watching out to a new class.

[Werner Lemberg]

branch: gsoc-2022-chariri-final
commit edda9b0f33c01882352fb7ae316c59cf3f9e30cb
Author: Charlie Jiang <w@chariri.moe>
Commit: Charlie Jiang <w@chariri.moe>

    [ftinspect] Move out font file managing/watching out to a new class.
    
    Introduce the feature that rejects invalid files.
    
    * src/ftinspect/maingui.hpp, src/ftinspect/maingui.cpp: Move `fontList`,
      `fontWatcher`, `timer` and other code related to font file managing to
      new class `FontFileManager`.
    
    * src/ftinspect/engine.hpp, src/ftinspect/engine.cpp:
      `Engine` class now holds `FontFileManager`.
      Also, make `MainGUI` open and close fonts indirectly via `Engine`.
    
    * src/ftinspect/fontfilemanager.hpp, src/ftinspect/fontfilemanager.cpp:
      New files.
    
    * src/ftinspect/CMakeLists.txt, src/ftinspect/meson.build: Updated.
---
 src/ftinspect/CMakeLists.txt             |   1 +
 src/ftinspect/engine/engine.cpp          |  27 ++++-
 src/ftinspect/engine/engine.hpp          |  21 +++-
 src/ftinspect/engine/fontfilemanager.cpp | 178 +++++++++++++++++++++++++++++++
 src/ftinspect/engine/fontfilemanager.hpp |  53 +++++++++
 src/ftinspect/maingui.cpp                |  74 ++++++-------
 src/ftinspect/maingui.hpp                |   7 +-
 src/ftinspect/meson.build                |   5 +
 8 files changed, 313 insertions(+), 53 deletions(-)

diff --git a/src/ftinspect/CMakeLists.txt b/src/ftinspect/CMakeLists.txt
index fc730937..9f46be53 100644
--- a/src/ftinspect/CMakeLists.txt
+++ b/src/ftinspect/CMakeLists.txt
@@ -20,6 +20,7 @@ add_executable(ftinspect
   "maingui.cpp"
   
   "engine/engine.cpp"
+  "engine/fontfilemanager.cpp"
 
   "rendering/glyphbitmap.cpp"
   "rendering/glyphoutline.cpp"
diff --git a/src/ftinspect/engine/engine.cpp b/src/ftinspect/engine/engine.cpp
index de984b22..5b00cf9c 100644
--- a/src/ftinspect/engine/engine.cpp
+++ b/src/ftinspect/engine/engine.cpp
@@ -94,10 +94,10 @@ faceRequester(FTC_FaceID ftcFaceID,
   // index; note that the validity of both the face and named instance index
   // is checked by FreeType itself
   if (faceID.fontIndex < 0
-      || faceID.fontIndex >= gui->fontList.size())
+      || faceID.fontIndex >= gui->engine->numberOfOpenedFonts())
     return FT_Err_Invalid_Argument;
 
-  QString& font = gui->fontList[faceID.fontIndex];
+  QString font = gui->engine->fileManager[faceID.fontIndex].filePath();
   long faceIndex = faceID.faceIndex;
 
   if (faceID.namedInstanceIndex > 0)
@@ -117,6 +117,7 @@ faceRequester(FTC_FaceID ftcFaceID,
 /////////////////////////////////////////////////////////////////////////////
 
 Engine::Engine(MainGUI* g)
+: fileManager(this)
 {
   gui = g;
   ftSize = NULL;
@@ -396,7 +397,7 @@ Engine::loadFont(int fontIndex,
 
 
 void
-Engine::removeFont(int fontIndex)
+Engine::removeFont(int fontIndex, bool closeFile)
 {
   // we iterate over all triplets that contain the given font index
   // and remove them
@@ -417,6 +418,9 @@ Engine::removeFont(int fontIndex)
 
     iter = faceIDMap.erase(iter);
   }
+
+  if (closeFile)
+    fileManager.remove(fontIndex);
 }
 
 
@@ -489,6 +493,17 @@ Engine::loadOutline(int glyphIndex)
   return &outlineGlyph->outline;
 }
 
+int
+Engine::numberOfOpenedFonts()
+{
+  return fileManager.size();
+}
+
+void
+Engine::openFonts(QStringList fontFileNames)
+{
+  fileManager.append(fontFileNames, true);
+}
 
 void
 Engine::setCFFHintingMode(int mode)
@@ -617,5 +632,11 @@ Engine::update()
   }
 }
 
+FontFileManager&
+Engine::fontFileManager()
+{
+  return fileManager;
+}
+
 
 // end of engine.cpp
diff --git a/src/ftinspect/engine/engine.hpp b/src/ftinspect/engine/engine.hpp
index a11ea0a2..72403e3e 100644
--- a/src/ftinspect/engine/engine.hpp
+++ b/src/ftinspect/engine/engine.hpp
@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include "fontfilemanager.hpp"
+
 #include <QString>
 #include <QMap>
 
@@ -44,6 +46,12 @@ public:
   Engine(MainGUI*);
   ~Engine();
 
+  // Disable copying
+  Engine(const Engine& other) = delete;
+  Engine& operator=(const Engine& other) = delete;
+
+  FT_Library ftLibrary() const { return library; }
+
   const QString& currentFamilyName();
   const QString& currentStyleName();
   QString glyphName(int glyphIndex);
@@ -54,7 +62,11 @@ public:
                long faceIndex,
                int namedInstanceIndex); // return number of glyphs
   FT_Outline* loadOutline(int glyphIndex);
-  void removeFont(int fontIndex);
+
+  int numberOfOpenedFonts();
+  void openFonts(QStringList fontFileNames);
+  void removeFont(int fontIndex, bool closeFile = true);
+
   void setCFFHintingMode(int mode);
   void setTTInterpreterVersion(int version);
   void update();
@@ -73,6 +85,11 @@ public:
     FontType_Other
   };
 
+  // XXX We should prepend '_' to all private member variable so we can create
+  // getter without naming conflict... e.g. var named _fontFileManager while
+  // getter named fontFileManager
+  FontFileManager& fontFileManager();
+
 private:
   MainGUI* gui;
 
@@ -80,6 +97,8 @@ private:
   FTC_IDType faceCounter; // a running number used to initialize `faceIDMap'
   QMap<FaceID, FTC_IDType> faceIDMap;
 
+  FontFileManager fileManager;
+
   QString curFamilyName;
   QString curStyleName;
 
diff --git a/src/ftinspect/engine/fontfilemanager.cpp 
b/src/ftinspect/engine/fontfilemanager.cpp
new file mode 100644
index 00000000..13c5cbdb
--- /dev/null
+++ b/src/ftinspect/engine/fontfilemanager.cpp
@@ -0,0 +1,178 @@
+// fontfilemanager.cpp
+
+// Copyright (C) 2022 by Charlie Jiang.
+
+
+#include "fontfilemanager.hpp"
+
+#include <QCoreApplication>
+#include <QGridLayout>
+#include <QMessageBox>
+
+#include "engine.hpp"
+
+
+FontFileManager::FontFileManager(Engine* engine)
+: engine(engine)
+{
+  fontWatcher = new QFileSystemWatcher(this);
+  // if the current input file is invalid we retry once a second to load it
+  watchTimer = new QTimer;
+  watchTimer->setInterval(1000);
+
+  connect(fontWatcher, &QFileSystemWatcher::fileChanged,
+          this, &FontFileManager::onWatcherFire);
+  connect(watchTimer, &QTimer::timeout,
+          this, &FontFileManager::onTimerFire);
+}
+
+
+int
+FontFileManager::size()
+{
+  return fontFileNameList.size();
+}
+
+
+void
+FontFileManager::append(QStringList const& newFileNames, bool alertNotExist)
+{
+  QStringList failedFiles;
+  for (auto& name : newFileNames)
+  {
+    auto info = QFileInfo(name);
+    info.setCaching(false);
+
+    // Filter non-file elements
+    if (!info.isFile())
+    {
+      if (alertNotExist)
+        failedFiles.append(name);
+      continue;
+    }
+
+    auto err = validateFontFile(name);
+    if (err)
+    {
+      if (alertNotExist)
+      {
+        auto errString = FT_Error_String(err);
+        if (!errString)
+          failedFiles.append(QString("- %1: %2")
+                             .arg(name)
+                             .arg(err));
+        else
+          failedFiles.append(QString("- %1: %2 (%3)")
+                             .arg(name)
+                             .arg(errString)
+                             .arg(err));
+      }
+      continue;
+    }
+
+    // Uniquify elements
+    auto absPath = info.absoluteFilePath();
+    auto existing = false;
+    for (auto& existingName : fontFileNameList)
+      if (existingName.absoluteFilePath() == absPath)
+      {
+        existing = true;
+        break;
+      }
+    if (existing)
+      continue;
+
+    if (info.size() >= INT_MAX)
+      return; // Prevent overflowing
+    fontFileNameList.append(info);
+  }
+
+  if (alertNotExist && !failedFiles.empty())
+  {
+    auto msg = new QMessageBox;
+    msg->setAttribute(Qt::WA_DeleteOnClose);
+    msg->setStandardButtons(QMessageBox::Ok);
+    if (failedFiles.size() == 1)
+    {
+      msg->setWindowTitle(tr("Failed to load file"));
+      msg->setText(tr("File failed to load:\n%1").arg(failedFiles.join("\n")));
+    }
+    else
+    {
+      msg->setWindowTitle(tr("Failed to load some files"));
+      msg->setText(tr("Files failed to 
load:\n%1").arg(failedFiles.join("\n")));
+    }
+    
+    msg->setIcon(QMessageBox::Warning);
+    msg->setModal(false);
+    msg->open();
+  }
+}
+
+
+void
+FontFileManager::remove(int index)
+{
+  if (index < 0 || index >= size())
+    return;
+
+  fontWatcher->removePath(fontFileNameList[index].filePath());
+  fontFileNameList.removeAt(index);
+}
+
+
+QFileInfo&
+FontFileManager::operator[](int index)
+{
+  return fontFileNameList[index];
+}
+
+
+void
+FontFileManager::updateWatching(int index)
+{
+  QFileInfo& fileInfo = fontFileNameList[index];
+
+  auto watching = fontWatcher->files();
+  if (!watching.empty())
+    fontWatcher->removePaths(watching);
+
+  // Qt's file watcher doesn't handle symlinks;
+  // we thus fall back to polling
+  if (fileInfo.isSymLink() || !fileInfo.exists())
+    watchTimer->start();
+  else
+    fontWatcher->addPath(fileInfo.filePath());
+}
+
+
+void
+FontFileManager::timerStart()
+{
+  watchTimer->start();
+}
+
+
+void
+FontFileManager::onWatcherFire()
+{
+  watchTimer->stop();
+  emit currentFileChanged();
+}
+
+
+FT_Error
+FontFileManager::validateFontFile(QString const& fileName)
+{
+  return FT_New_Face(engine->ftLibrary(), fileName.toUtf8(), -1, NULL);
+}
+
+
+void
+FontFileManager::onTimerFire()
+{
+  onWatcherFire();
+}
+
+
+// end of fontfilemanager.hpp
diff --git a/src/ftinspect/engine/fontfilemanager.hpp 
b/src/ftinspect/engine/fontfilemanager.hpp
new file mode 100644
index 00000000..93916852
--- /dev/null
+++ b/src/ftinspect/engine/fontfilemanager.hpp
@@ -0,0 +1,53 @@
+// fontfilemanager.hpp
+
+// Copyright (C) 2022 by Charlie Jiang.
+
+#pragma once
+
+#include <QObject>
+#include <QList>
+#include <QFileSystemWatcher>
+#include <QTimer>
+#include <QFileInfo>
+
+#include <freetype/freetype.h>
+
+
+// Class to manage all opened font files, as well as monitoring local file
+// change.
+
+class Engine;
+class FontFileManager
+: public QObject
+{
+  Q_OBJECT
+public:
+  FontFileManager(Engine* engine);
+  ~FontFileManager() override = default;
+
+  int size();
+  void append(QStringList const& newFileNames, bool alertNotExist = false);
+  void remove(int index);
+
+  QFileInfo& operator[](int index);
+  void updateWatching(int index);
+  void timerStart();
+  
+signals:
+  void currentFileChanged();
+
+private slots:
+  void onTimerFire();
+  void onWatcherFire();
+
+private:
+  Engine* engine;
+  QList<QFileInfo> fontFileNameList;
+  QFileSystemWatcher* fontWatcher;
+  QTimer* watchTimer;
+
+  FT_Error validateFontFile(QString const& fileName);
+};
+
+
+// end of fontfilemanager.hpp
diff --git a/src/ftinspect/maingui.cpp b/src/ftinspect/maingui.cpp
index 90bd63ac..2b887c77 100644
--- a/src/ftinspect/maingui.cpp
+++ b/src/ftinspect/maingui.cpp
@@ -7,7 +7,6 @@
 #include "rendering/grid.hpp"
 
 #include <QApplication>
-#include <QDir>
 #include <QFileDialog>
 #include <QMessageBox>
 #include <QSettings>
@@ -19,11 +18,6 @@ MainGUI::MainGUI()
 {
   engine = NULL;
 
-  fontWatcher = new QFileSystemWatcher;
-  // if the current input file is invalid we retry once a second to load it
-  timer = new QTimer;
-  timer->setInterval(1000);
-
   setGraphicsDefaults();
   createLayout();
   createConnections();
@@ -46,7 +40,13 @@ MainGUI::~MainGUI()
 void
 MainGUI::update(Engine* e)
 {
+  if (engine)
+    disconnect(&engine->fontFileManager(), 
&FontFileManager::currentFileChanged,
+        this, &MainGUI::watchCurrentFont);
+
   engine = e;
+  connect(&engine->fontFileManager(), &FontFileManager::currentFileChanged,
+          this, &MainGUI::watchCurrentFont);
 }
 
 
@@ -94,7 +94,7 @@ MainGUI::aboutQt()
 void
 MainGUI::loadFonts()
 {
-  int oldSize = fontList.size();
+  int oldSize = engine->numberOfOpenedFonts();
 
   QStringList files = QFileDialog::getOpenFileNames(
                         this,
@@ -104,11 +104,10 @@ MainGUI::loadFonts()
                         NULL,
                         QFileDialog::ReadOnly);
 
-  // XXX sort data, uniquify elements
-  fontList.append(files);
+  engine->openFonts(files);
 
   // if we have new fonts, set the current index to the first new one
-  if (oldSize < fontList.size())
+  if (oldSize < engine->numberOfOpenedFonts())
     currentFontIndex = oldSize;
 
   showFont();
@@ -118,18 +117,17 @@ MainGUI::loadFonts()
 void
 MainGUI::closeFont()
 {
-  if (currentFontIndex < fontList.size())
+  if (currentFontIndex < engine->numberOfOpenedFonts())
   {
     engine->removeFont(currentFontIndex);
-    fontWatcher->removePath(fontList[currentFontIndex]);
-    fontList.removeAt(currentFontIndex);
   }
 
   // show next font after deletion, i.e., retain index if possible
-  if (fontList.size())
+  int num = engine->numberOfOpenedFonts();
+  if (num)
   {
-    if (currentFontIndex >= fontList.size())
-      currentFontIndex = fontList.size() - 1;
+    if (currentFontIndex >= num)
+      currentFontIndex = num - 1;
   }
   else
     currentFontIndex = 0;
@@ -141,7 +139,6 @@ MainGUI::closeFont()
 void
 MainGUI::watchCurrentFont()
 {
-  timer->stop();
   showFont();
 }
 
@@ -151,32 +148,27 @@ MainGUI::showFont()
 {
   // we do lazy computation of FT_Face objects
 
-  if (currentFontIndex < fontList.size())
+  if (currentFontIndex < engine->numberOfOpenedFonts())
   {
-    QString& font = fontList[currentFontIndex];
-    QFileInfo fileInfo(font);
+    QFileInfo& fileInfo = engine->fontFileManager()[currentFontIndex];
     QString fontName = fileInfo.fileName();
 
-    if (fileInfo.exists())
+    engine->fontFileManager().updateWatching(currentFontIndex);
+    if (fileInfo.isSymLink())
     {
-      // Qt's file watcher doesn't handle symlinks;
-      // we thus fall back to polling
-      if (fileInfo.isSymLink())
-      {
-        fontName.prepend("<i>");
-        fontName.append("</i>");
-        timer->start();
-      }
-      else
-        fontWatcher->addPath(font);
+      fontName.prepend("<i>");
+      fontName.append("</i>");
     }
-    else
+
+    if (!fileInfo.exists())
     {
       // On Unix-like systems, the symlink's target gets opened; this
       // implies that deletion of a symlink doesn't make `engine->loadFont'
       // fail since it operates on a file handle pointing to the target.
       // For this reason, we remove the font to enforce a reload.
-      engine->removeFont(currentFontIndex);
+      // However, we're just removing it from the Engine cache,
+      // not deleting the entry in the font file manager
+      engine->removeFont(currentFontIndex, false);
     }
 
     fontFilenameLabel->setText(fontName);
@@ -200,8 +192,9 @@ MainGUI::showFont()
     // (file, face, instance) triplet is invalid or missing;
     // we thus start our timer to periodically test
     // whether the font starts working
-    if (currentFontIndex < fontList.size())
-      timer->start();
+    if (currentFontIndex > 0
+        && currentFontIndex < engine->numberOfOpenedFonts())
+      engine->fontFileManager().timerStart();
   }
 
   fontNameLabel->setText(QString("%1 %2")
@@ -428,7 +421,7 @@ MainGUI::adjustGlyphIndex(int delta)
 void
 MainGUI::checkCurrentFontIndex()
 {
-  if (fontList.size() < 2)
+  if (engine->numberOfOpenedFonts() < 2)
   {
     previousFontButton->setEnabled(false);
     nextFontButton->setEnabled(false);
@@ -438,7 +431,7 @@ MainGUI::checkCurrentFontIndex()
     previousFontButton->setEnabled(false);
     nextFontButton->setEnabled(true);
   }
-  else if (currentFontIndex >= fontList.size() - 1)
+  else if (currentFontIndex >= engine->numberOfOpenedFonts() - 1)
   {
     previousFontButton->setEnabled(true);
     nextFontButton->setEnabled(false);
@@ -519,7 +512,7 @@ MainGUI::previousFont()
 void
 MainGUI::nextFont()
 {
-  if (currentFontIndex < fontList.size() - 1)
+  if (currentFontIndex < engine->numberOfOpenedFonts() - 1)
   {
     currentFontIndex++;
     currentFaceIndex = 0;
@@ -1101,11 +1094,6 @@ MainGUI::createConnections()
   glyphNavigationMapper->setMapping(toP100Buttonx, 100);
   glyphNavigationMapper->setMapping(toP1000Buttonx, 1000);
   glyphNavigationMapper->setMapping(toEndButtonx, 0x10000);
-
-  connect(fontWatcher, SIGNAL(fileChanged(const QString&)),
-          SLOT(watchCurrentFont()));
-  connect(timer, SIGNAL(timeout()),
-          SLOT(watchCurrentFont()));
 }
 
 
diff --git a/src/ftinspect/maingui.hpp b/src/ftinspect/maingui.hpp
index 2b0b857c..9f8b2c20 100644
--- a/src/ftinspect/maingui.hpp
+++ b/src/ftinspect/maingui.hpp
@@ -95,8 +95,7 @@ private slots:
 
 private:
   Engine* engine;
-
-  QStringList fontList;
+  
   int currentFontIndex;
 
   long currentNumberOfFaces;
@@ -141,8 +140,6 @@ private:
 
   QDoubleSpinBox *sizeDoubleSpinBox;
 
-  QFileSystemWatcher *fontWatcher;
-
   QGraphicsScene *glyphScene;
   QGraphicsViewx *glyphView;
 
@@ -221,8 +218,6 @@ private:
 
   QTabWidget *tabWidget;
 
-  QTimer *timer;
-
   QVBoxLayout *generalTabLayout;
   QVBoxLayout *leftLayout;
   QVBoxLayout *rightLayout;
diff --git a/src/ftinspect/meson.build b/src/ftinspect/meson.build
index 39019774..892aff29 100644
--- a/src/ftinspect/meson.build
+++ b/src/ftinspect/meson.build
@@ -22,21 +22,26 @@ qt5_dep = dependency('qt5',
 if qt5_dep.found()
   sources = files([
     'engine/engine.cpp',
+    'engine/fontfilemanager.cpp',
+
     'rendering/glyphbitmap.cpp',
     'rendering/glyphoutline.cpp',
     'rendering/glyphpointnumbers.cpp',
     'rendering/glyphpoints.cpp',
     'rendering/grid.cpp',
+
     'widgets/qcomboboxx.cpp',
     'widgets/qgraphicsviewx.cpp',
     'widgets/qpushbuttonx.cpp',
     'widgets/qspinboxx.cpp',
+
     'ftinspect.cpp',
     'maingui.cpp',
   ])
 
   moc_files = qt5.preprocess(
     moc_headers: [
+      'engine/fontfilemanager.hpp',
       'widgets/qcomboboxx.hpp',
       'widgets/qgraphicsviewx.hpp',
       'widgets/qpushbuttonx.hpp',