Functional spdlogging implementation with fmtlib

.gitmodules

@@ -28,3 +28,6 @@
 [submodule "spdlog"]
     path = externals/spdlog
     url = https://github.com/gabime/spdlog.git
+[submodule "fmt"]
+    path = externals/fmt
+    url = https://github.com/fmtlib/fmt

externals/CMakeLists.txt

@@ -51,4 +51,5 @@ endif()
 
 # Spdlog
 add_library(spdlog INTERFACE)
+target_compile_definitions(spdlog INTERFACE -DSPDLOG_FMT_EXTERNAL=1)
 target_include_directories(spdlog INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/spdlog/include)

src/CMakeLists.txt

@@ -1,5 +1,7 @@
 # Enable modules to include each other's files
 include_directories(.)
+# Include fmtlib so it can be used across the application for logging
+include_directories(../externals/fmt)
 
 add_subdirectory(common)
 add_subdirectory(core)

src/citra_qt/game_list.cpp

@@ -16,7 +16,7 @@
 #include "game_list_p.h"
 #include "ui_settings.h"
 
-REGISTER_LOGGER("Class Name");
+REGISTER_LOGGER("Game List");
 
 GameList::SearchField::KeyReleaseEater::KeyReleaseEater(GameList* gamelist) {
     this->gamelist = gamelist;
@@ -319,7 +319,7 @@ void GameList::PopupContextMenu(const QPoint& menu_location) {
 void GameList::PopulateAsync(const QString& dir_path, bool deep_scan) {
     if (!FileUtil::Exists(dir_path.toStdString()) ||
         !FileUtil::IsDirectory(dir_path.toStdString())) {
-        LOG_ERROR(Frontend, "Could not find game list folder at %s", dir_path.toLocal8Bit().data());
+        SPDLOG_ERROR("Could not find game list folder at {}", dir_path.toLocal8Bit().data());
         search_field->setFilterResult(0, 0);
         return;
     }
@@ -369,7 +369,7 @@ static bool HasSupportedFileExtension(const std::string& file_name) {
 
 void GameList::RefreshGameDirectory() {
     if (!UISettings::values.gamedir.isEmpty() && current_worker != nullptr) {
-        LOG_INFO(Frontend, "Change detected in the games directory. Reloading game list.");
+        SPDLOG_INFO("Change detected in the games directory. Reloading game list.");
         search_field->clear();
         PopulateAsync(UISettings::values.gamedir, UISettings::values.gamedir_deepscan);
     }

src/citra_qt/main.cpp

@@ -48,6 +48,8 @@
 Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin);
 #endif
 
+REGISTER_LOGGER("Main");
+
 GMainWindow::GMainWindow() : config(new Config()), emu_thread(nullptr) {
     Pica::g_debug_context = Pica::DebugContext::Construct();
     setAcceptDrops(true);
@@ -321,14 +323,13 @@ bool GMainWindow::LoadROM(const QString& filename) {
     if (result != Core::System::ResultStatus::Success) {
         switch (result) {
         case Core::System::ResultStatus::ErrorGetLoader:
-            LOG_CRITICAL(Frontend, "Failed to obtain loader for %s!",
+            SPDLOG_CRITICAL("Failed to obtain loader for {}!", filename.toStdString());
-                         filename.toStdString().c_str());
             QMessageBox::critical(this, tr("Error while loading ROM!"),
                                   tr("The ROM format is not supported."));
             break;
 
         case Core::System::ResultStatus::ErrorSystemMode:
-            LOG_CRITICAL(Frontend, "Failed to load ROM!");
+            SPDLOG_CRITICAL("Failed to load ROM!");
             QMessageBox::critical(this, tr("Error while loading ROM!"),
                                   tr("Could not determine the system mode."));
             break;
@@ -377,7 +378,7 @@ bool GMainWindow::LoadROM(const QString& filename) {
 }
 
 void GMainWindow::BootGame(const QString& filename) {
-    LOG_INFO(Frontend, "Citra starting...");
+    SPDLOG_INFO("Citra starting...");
     StoreRecentFile(filename); // Put the filename on top of the list
 
     if (!LoadROM(filename))
@@ -503,7 +504,7 @@ void GMainWindow::OnGameListOpenSaveFolder(u64 program_id) {
         return;
     }
 
-    LOG_INFO(Frontend, "Opening save data path for program_id=%" PRIu64, program_id);
+    SPDLOG_INFO("Opening save data path for program_id={0:#x}", program_id);
     QDesktopServices::openUrl(QUrl::fromLocalFile(qpath));
 }
 

src/common/CMakeLists.txt

@@ -99,8 +99,7 @@ endif()
 create_directory_groups(${SRCS} ${HEADERS})
 
 add_library(common STATIC ${SRCS} ${HEADERS})
-
+target_link_libraries(common PUBLIC Boost::boost fmt microprofile spdlog)
-target_link_libraries(common PUBLIC Boost::boost microprofile spdlog)
 
 if (ARCHITECTURE_x86_64)
     target_link_libraries(common PRIVATE xbyak)

src/common/logging/backend_spdlog.cpp

@@ -1,8 +1,31 @@
+#include <spdlog/spdlog.h>
+
 #include "common/assert.h"
 #include "common/logging/backend_spdlog.h"
 #include "common/logging/formatter.h"
 #include "common/string_util.h"
 
+static spdlog::level::level_enum GetLevel(Log::Level log_level) {
+    switch (log_level) {
+    case Log::Level::Trace:
+        return spdlog::level::trace;
+    case Log::Level::Debug:
+        return spdlog::level::debug;
+    case Log::Level::Info:
+        return spdlog::level::info;
+    case Log::Level::Warning:
+        return spdlog::level::warn;
+    case Log::Level::Error:
+        return spdlog::level::err;
+    case Log::Level::Critical:
+        return spdlog::level::critical;
+    default:
+        UNREACHABLE();
+        break;
+    }
+    return spdlog::level::off;
+}
+
 namespace Log {
 
 SpdLogBackend& SpdLogBackend::instance() {
@@ -31,54 +54,18 @@ SpdLogBackend::~SpdLogBackend() {
     spdlog::drop_all();
 }
 
-const std::shared_ptr<spdlog::logger> SpdLogBackend::GetLogger(u32 logger) const {
+const std::shared_ptr<spdlog::logger>& SpdLogBackend::GetLogger(u32 logger) const {
     return loggers[logger];
 }
 
 u32 SpdLogBackend::RegisterLogger(const char* class_name) {
-    loggers.push_back(std::make_shared<spdlog::logger>(class_name, sinks.begin(), sinks.end()));
+    loggers.push_back(spdlog::create(class_name, sinks.begin(), sinks.end()));
     return loggers.size() - 1;
 }
 
-spdlog::level_t GetLevel(Level log_level) {
+void SpdLogImpl(u32 logger, Level log_level, const char* format, fmt::ArgList& args) {
-    switch (log_level) {
-    case Level::Trace:
-        return spdlog::level::trace;
-    case Level::Debug:
-        return spdlog::level::debug;
-    case Level::Info:
-        return spdlog::level::info;
-    case Level::Warning:
-        return spdlog::level::warn;
-    case Level::Error:
-        return spdlog::level::err;
-    case Level::Critical:
-        return spdlog::level::critical;
-    default:
-        UNREACHABLE();
-        break;
-    }
-    return spdlog::level::off;
-}
-
-template <typename Arg1, typename... Args>
-void SpdLogMessage(u32 logger, Level log_level, const char* filename, unsigned int line_nr,
-                   const char* function, const char* format, const Arg1& arg, const Args&... args) {
     auto log = SpdLogBackend::instance().GetLogger(logger);
-    fmt::MemoryWriter formatting_buffer;
+    log->log(GetLevel(log_level), format, args);
-    formatting_buffer << Common::TrimSourcePath(filename) << ':' << function << ':' << line_nr
-                      << ": " << format;
-    log->log(GetLevel(log_level), formatting_buffer.c_str(), arg, args...);
-}
-
-template <typename T>
-void SpdLogMessage(u32 logger, Level log_level, const char* filename, unsigned int line_nr,
-                   const char* function, const T& msg) {
-    auto log = SpdLogBackend::instance().GetLogger(logger);
-    fmt::MemoryWriter formatting_buffer;
-    formatting_buffer << Common::TrimSourcePath(filename) << ':' << function << ':' << line_nr
-                      << ": " << msg;
-    log->log(GetLevel(log_level), formatting_buffer.c_str());
 }
 
 u32 RegisterLogger(const char* class_name) {

src/common/logging/backend_spdlog.h

@@ -10,11 +10,11 @@ public:
     static SpdLogBackend& instance();
 
     SpdLogBackend(SpdLogBackend const&) = delete;
-    void operator=(SpdLogBackend const&) = delete;
+    SpdLogBackend& operator=(SpdLogBackend const&) = delete;
 
     u32 RegisterLogger(const char* class_name);
 
-    const std::shared_ptr<spdlog::logger> GetLogger(u32 logger) const;
+    const std::shared_ptr<spdlog::logger>& GetLogger(u32 logger) const;
 
 private:
     SpdLogBackend();

src/common/logging/formatter.cpp

@@ -3,11 +3,7 @@
 #include "common/assert.h"
 #include "common/logging/formatter.h"
 
-namespace Log {
+static const char* GetLevelName(spdlog::level_t log_level) {
-
-Formatter::Formatter() {}
-
-const char* GetLevelName(spdlog::level_t log_level) {
     switch (log_level) {
     case spdlog::level::trace:
         return "Trace";
@@ -16,7 +12,7 @@ const char* GetLevelName(spdlog::level_t log_level) {
     case spdlog::level::info:
         return "Info";
     case spdlog::level::warn:
-        return "Warn";
+        return "Warning";
     case spdlog::level::err:
         return "Error";
     case spdlog::level::critical:
@@ -27,6 +23,8 @@ const char* GetLevelName(spdlog::level_t log_level) {
     }
 }
 
+namespace Log {
+
 void Formatter::format(spdlog::details::log_msg& msg) {
     using std::chrono::steady_clock;
     using std::chrono::duration_cast;

src/common/logging/formatter.h

@@ -5,7 +5,7 @@ namespace Log {
 class Formatter : public spdlog::formatter {
 
 public:
-    explicit Formatter();
+    explicit Formatter() = default;
     Formatter(const Formatter&) = delete;
     Formatter& operator=(const Formatter&) = delete;
     void format(spdlog::details::log_msg& msg) override;

src/common/logging/log.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <fmt/format.h>
 #include "common/common_types.h"
 
 namespace Log {
@@ -106,12 +107,19 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned
 #endif
     ;
 
+void SpdLogImpl(u32 logger, Level log_level, const char* format, fmt::ArgList& args);
+
 template <typename Arg1, typename... Args>
 void SpdLogMessage(u32 logger, Level log_level, const char* filename, unsigned int line_nr,
-                   const char* function, const char* format, const Arg1& arg, const Args&... args);
+                   const char* function, const Arg1& format, const Args&... args) {
-template <typename T>
+    typedef fmt::internal::ArgArray<sizeof...(Args)> ArgArray;
-void SpdLogMessage(u32 logger, Level log_level, const char* filename, unsigned int line_nr,
+    typename ArgArray::Type array{ArgArray::template make<fmt::BasicFormatter<char>>(args)...};
-                   const char* function, const T& msg);
+    fmt::MemoryWriter formatting_buffer;
+    formatting_buffer << Common::TrimSourcePath(filename) << ':' << function << ':' << line_nr
+                      << ": " << format;
+    SpdLogImpl(logger, log_level, formatting_buffer.c_str(),
+               fmt::ArgList(fmt::internal::make_type(args...), array));
+}
 
 u32 RegisterLogger(const char* class_name);