[freetype2-demos] reorg-maingui f3400f1: [ftinspect] Move out font file managing/watching out to a new class.

[Werner Lemberg]

branch: reorg-maingui
commit f3400f18b864bca25307f96db138232abc42716f
Author: Charlie Jiang <w@chariri.moe>
Commit: Charlie Jiang <w@chariri.moe>

    [ftinspect] Move out font file managing/watching out to a new class.
    
    * src/ftinspect/CMakeLists.txt: Add `fontfilemanager.cpp`.
    
    * src/ftinspect/meson.build: Add `fontfilemanager.cpp`.
    
    * src/ftinspect/fontfilemanager.hpp: Font file managing/watching goes here.
    
    * src/ftinspect/fontfilemanager.cpp: Font file managing/watching goes here.
    
    * src/ftinspect/maingui.hpp: Remove `fontList`, `fontWatcher` and `timer`.
    
    * src/ftinspect/maingui.cpp: Remove code related to font file managing, they
    were all moved to `FontFileManager` class.
    
    * src/ftinspect/engine.hpp: `Engine` class now holds `FontFileManager`. 
Also,
    `MainGUI` will open and close fonts indirectly via `Engine`.
    
    * src/ftinspect/engine.cpp: Same as `engine.hpp`.
    
    TODO: Maybe we should remove dependency against `FontFileManager` from
    `MainGUI`?
---
 src/ftinspect/CMakeLists.txt             |   1 +
 src/ftinspect/engine/engine.cpp          |  26 +++++++-
 src/ftinspect/engine/engine.hpp          |  19 +++++-
 src/ftinspect/engine/fontfilemanager.cpp | 102 +++++++++++++++++++++++++++++++
 src/ftinspect/engine/fontfilemanager.hpp |  42 +++++++++++++
 src/ftinspect/maingui.cpp                |  72 +++++++++-------------
 src/ftinspect/maingui.hpp                |   7 +--
 src/ftinspect/meson.build                |   4 ++
 8 files changed, 220 insertions(+), 53 deletions(-)

diff --git a/src/ftinspect/CMakeLists.txt b/src/ftinspect/CMakeLists.txt
index c910e05..e3520ea 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 de984b2..0ea434f 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)
@@ -396,7 +396,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 +417,9 @@ Engine::removeFont(int fontIndex)
 
     iter = faceIDMap.erase(iter);
   }
+
+  if (closeFile)
+    fileManager.remove(fontIndex);
 }
 
 
@@ -489,6 +492,17 @@ Engine::loadOutline(int glyphIndex)
   return &outlineGlyph->outline;
 }
 
+int
+Engine::numberOfOpenedFonts()
+{
+  return fileManager.size();
+}
+
+void
+Engine::openFonts(QStringList fontFileNames)
+{
+  fileManager.append(fontFileNames);
+}
 
 void
 Engine::setCFFHintingMode(int mode)
@@ -617,5 +631,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 a11ea0a..b529089 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,10 @@ public:
   Engine(MainGUI*);
   ~Engine();
 
+  // Disable copying
+  Engine(const Engine& other) = delete;
+  Engine& operator=(const Engine& other) = delete;
+
   const QString& currentFamilyName();
   const QString& currentStyleName();
   QString glyphName(int glyphIndex);
@@ -54,7 +60,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 +83,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 +95,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 0000000..df969b3
--- /dev/null
+++ b/src/ftinspect/engine/fontfilemanager.cpp
@@ -0,0 +1,102 @@
+// fontfilemanager.cpp
+
+// Copyright (C) 2016-2022 by Werner Lemberg.
+
+
+#include "fontfilemanager.hpp"
+
+FontFileManager::FontFileManager()
+{
+  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::onTimerOrWatcherFire);
+  connect(watchTimer, &QTimer::timeout,
+          this, &FontFileManager::onTimerOrWatcherFire);
+}
+
+FontFileManager::~FontFileManager()
+{
+}
+
+int
+FontFileManager::size()
+{
+  return fontFileNameList.size();
+}
+
+void
+FontFileManager::append(QStringList newFileNames)
+{
+  for (auto& name : newFileNames)
+  {
+    auto info = QFileInfo(name);
+
+    // Filter non-file elements
+    if (!info.isFile())
+      continue;
+
+    // Uniquify elements
+    auto absPath = info.absoluteFilePath();
+    auto existing = false;
+    for (auto& existingName : fontFileNameList)
+      if (existingName.absoluteFilePath() == absPath)
+      {
+        existing = true;
+        break;
+      }
+    if (existing)
+      continue;
+
+    fontFileNameList.append(info);
+  }
+}
+
+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::onTimerOrWatcherFire()
+{
+  watchTimer->stop();
+  emit currentFileChanged();
+}
diff --git a/src/ftinspect/engine/fontfilemanager.hpp 
b/src/ftinspect/engine/fontfilemanager.hpp
new file mode 100644
index 0000000..1ad18ae
--- /dev/null
+++ b/src/ftinspect/engine/fontfilemanager.hpp
@@ -0,0 +1,42 @@
+// fontfilemanager.hpp
+
+// Copyright (C) 2022 by Charlie Jiang.
+
+#pragma once
+
+#include <QObject>
+#include <QList>
+#include <QFileSystemWatcher>
+#include <QTimer>
+#include <QFileInfo>
+
+// Class to manage all opened font files, as well as monitoring local file
+// change.
+
+class FontFileManager
+: public QObject
+{
+  Q_OBJECT
+public:
+  FontFileManager();
+  ~FontFileManager() override;
+
+  int size();
+  void append(QStringList newFileNames);
+  void remove(int index);
+
+  QFileInfo& operator[](int index);
+  void updateWatching(int index);
+  void timerStart();
+
+signals:
+  void currentFileChanged();
+
+private slots:
+  void onTimerOrWatcherFire();
+
+private:
+  QList<QFileInfo> fontFileNameList;
+  QFileSystemWatcher* fontWatcher;
+  QTimer* watchTimer;
+};
\ No newline at end of file
diff --git a/src/ftinspect/maingui.cpp b/src/ftinspect/maingui.cpp
index 90bd63a..2538dc6 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,25 @@ 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);
+      engine->removeFont(currentFontIndex, false);
     }
 
     fontFilenameLabel->setText(fontName);
@@ -200,8 +190,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 +419,7 @@ MainGUI::adjustGlyphIndex(int delta)
 void
 MainGUI::checkCurrentFontIndex()
 {
-  if (fontList.size() < 2)
+  if (engine->numberOfOpenedFonts() < 2)
   {
     previousFontButton->setEnabled(false);
     nextFontButton->setEnabled(false);
@@ -438,7 +429,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 +510,7 @@ MainGUI::previousFont()
 void
 MainGUI::nextFont()
 {
-  if (currentFontIndex < fontList.size() - 1)
+  if (currentFontIndex < engine->numberOfOpenedFonts() - 1)
   {
     currentFontIndex++;
     currentFaceIndex = 0;
@@ -1101,11 +1092,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 2b0b857..9f8b2c2 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 9d2abbd..3993144 100644
--- a/src/ftinspect/meson.build
+++ b/src/ftinspect/meson.build
@@ -21,15 +21,19 @@ 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',
   ])