Updates from review.

Adds an array to store loggers to speed up look up.
Adds filter support to the new backend
Various other cleanup changes

.gitmodules

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

src/CMakeLists.txt

@@ -1,7 +1,5 @@
 # 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/citra.cpp

@@ -24,6 +24,7 @@
 #include "citra/config.h"
 #include "citra/emu_window/emu_window_sdl2.h"
 #include "common/logging/backend.h"
+#include "common/logging/backend_spdlog.h"
 #include "common/logging/filter.h"
 #include "common/logging/log.h"
 #include "common/scm_rev.h"
@@ -58,7 +59,7 @@ int main(int argc, char** argv) {
     auto argv_w = CommandLineToArgvW(GetCommandLineW(), &argc_w);
 
     if (argv_w == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to get command line arguments");
+        SLOG_CRITICAL(Frontend, "Failed to get command line arguments");
         return -1;
     }
 #endif
@@ -107,18 +108,21 @@ int main(int argc, char** argv) {
     LocalFree(argv_w);
 #endif
 
-    Log::Filter log_filter(Log::Level::Debug);
+    Log::Filter log_filter(::Log::Level::Info);
     Log::SetFilter(&log_filter);
+    Log::SpdLogSetFilter(&log_filter);
 
     MicroProfileOnThreadCreate("EmuThread");
     SCOPE_EXIT({ MicroProfileShutdown(); });
 
     if (filepath.empty()) {
-        LOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified");
+        SLOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified");
         return -1;
     }
 
     log_filter.ParseFilterString(Settings::values.log_filter);
+    Log::SetFilter(&log_filter);
+    Log::SpdLogSetFilter(&log_filter);
 
     // Apply the command line arguments
     Settings::values.gdbstub_port = gdb_port;
@@ -135,28 +139,28 @@ int main(int argc, char** argv) {
 
     switch (load_result) {
     case Core::System::ResultStatus::ErrorGetLoader:
-        LOG_CRITICAL(Frontend, "Failed to obtain loader for %s!", filepath.c_str());
+        SLOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filepath);
         return -1;
     case Core::System::ResultStatus::ErrorLoader:
-        LOG_CRITICAL(Frontend, "Failed to load ROM!");
+        SLOG_CRITICAL(Frontend, "Failed to load ROM!");
         return -1;
     case Core::System::ResultStatus::ErrorLoader_ErrorEncrypted:
-        LOG_CRITICAL(Frontend, "The game that you are trying to load must be decrypted before "
+        SLOG_CRITICAL(Frontend, "The game that you are trying to load must be decrypted before "
-                               "being used with Citra. \n\n For more information on dumping and "
+                                "being used with Citra. \n\n For more information on dumping and "
-                               "decrypting games, please refer to: "
+                                "decrypting games, please refer to: "
-                               "https://citra-emu.org/wiki/dumping-game-cartridges/");
+                                "https://citra-emu.org/wiki/dumping-game-cartridges/");
         return -1;
     case Core::System::ResultStatus::ErrorLoader_ErrorInvalidFormat:
-        LOG_CRITICAL(Frontend, "Error while loading ROM: The ROM format is not supported.");
+        SLOG_CRITICAL(Frontend, "Error while loading ROM: The ROM format is not supported.");
         return -1;
     case Core::System::ResultStatus::ErrorNotInitialized:
-        LOG_CRITICAL(Frontend, "CPUCore not initialized");
+        SLOG_CRITICAL(Frontend, "CPUCore not initialized");
         return -1;
     case Core::System::ResultStatus::ErrorSystemMode:
-        LOG_CRITICAL(Frontend, "Failed to determine system mode!");
+        SLOG_CRITICAL(Frontend, "Failed to determine system mode!");
         return -1;
     case Core::System::ResultStatus::ErrorVideoCore:
-        LOG_CRITICAL(Frontend, "VideoCore not initialized");
+        SLOG_CRITICAL(Frontend, "VideoCore not initialized");
         return -1;
     case Core::System::ResultStatus::Success:
         break; // Expected case

src/citra/config.cpp

@@ -27,17 +27,17 @@ bool Config::LoadINI(const std::string& default_contents, bool retry) {
     const char* location = this->sdl2_config_loc.c_str();
     if (sdl2_config->ParseError() < 0) {
         if (retry) {
-            LOG_WARNING(Config, "Failed to load %s. Creating file from defaults...", location);
+            SLOG_WARNING(Config, "Failed to load {}. Creating file from defaults...", location);
             FileUtil::CreateFullPath(location);
             FileUtil::WriteStringToFile(true, default_contents, location);
             sdl2_config = std::make_unique<INIReader>(location); // Reopen file
 
             return LoadINI(default_contents, false);
         }
-        LOG_ERROR(Config, "Failed.");
+        SLOG_ERROR(Config, "Failed.");
         return false;
     }
-    LOG_INFO(Config, "Successfully loaded %s", location);
+    SLOG_INFO(Config, "Successfully loaded {}", location);
     return true;
 }
 

src/citra/emu_window/emu_window_sdl2.cpp

@@ -65,7 +65,7 @@ EmuWindow_SDL2::EmuWindow_SDL2() {
 
     // Initialize the window
     if (SDL_Init(SDL_INIT_VIDEO) < 0) {
-        LOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting...");
+        SLOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting...");
         exit(1);
     }
 
@@ -88,19 +88,19 @@ EmuWindow_SDL2::EmuWindow_SDL2() {
                          SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI);
 
     if (render_window == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to create SDL2 window! Exiting...");
+        SLOG_CRITICAL(Frontend, "Failed to create SDL2 window! Exiting...");
         exit(1);
     }
 
     gl_context = SDL_GL_CreateContext(render_window);
 
     if (gl_context == nullptr) {
-        LOG_CRITICAL(Frontend, "Failed to create SDL2 GL context! Exiting...");
+        SLOG_CRITICAL(Frontend, "Failed to create SDL2 GL context! Exiting...");
         exit(1);
     }
 
     if (!gladLoadGLLoader(static_cast<GLADloadproc>(SDL_GL_GetProcAddress))) {
-        LOG_CRITICAL(Frontend, "Failed to initialize GL functions! Exiting...");
+        SLOG_CRITICAL(Frontend, "Failed to initialize GL functions! Exiting...");
         exit(1);
     }
 

src/citra_qt/game_list.cpp

@@ -316,8 +316,8 @@ 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())) {
-        SPDLOG_ERROR(Frontend, "Could not find game list folder at {}",
+        SLOG_ERROR(Frontend, "Could not find game list folder at {}",
-                     dir_path.toLocal8Bit().data());
+                   dir_path.toLocal8Bit().data());
         search_field->setFilterResult(0, 0);
         return;
     }
@@ -367,7 +367,7 @@ static bool HasSupportedFileExtension(const std::string& file_name) {
 
 void GameList::RefreshGameDirectory() {
     if (!UISettings::values.gamedir.isEmpty() && current_worker != nullptr) {
-        SPDLOG_INFO(Frontend, "Change detected in the games directory. Reloading game list.");
+        SLOG_INFO(Frontend, "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

@@ -30,6 +30,7 @@
 #include "citra_qt/main.h"
 #include "citra_qt/ui_settings.h"
 #include "common/logging/backend.h"
+#include "common/logging/backend_spdlog.h"
 #include "common/logging/filter.h"
 #include "common/logging/log.h"
 #include "common/logging/text_formatter.h"
@@ -321,13 +322,13 @@ bool GMainWindow::LoadROM(const QString& filename) {
     if (result != Core::System::ResultStatus::Success) {
         switch (result) {
         case Core::System::ResultStatus::ErrorGetLoader:
-            SPDLOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filename.toStdString());
+            SLOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filename.toStdString());
             QMessageBox::critical(this, tr("Error while loading ROM!"),
                                   tr("The ROM format is not supported."));
             break;
 
         case Core::System::ResultStatus::ErrorSystemMode:
-            SPDLOG_CRITICAL(Frontend, "Failed to load ROM!");
+            SLOG_CRITICAL(Frontend, "Failed to load ROM!");
             QMessageBox::critical(this, tr("Error while loading ROM!"),
                                   tr("Could not determine the system mode."));
             break;
@@ -376,7 +377,7 @@ bool GMainWindow::LoadROM(const QString& filename) {
 }
 
 void GMainWindow::BootGame(const QString& filename) {
-    SPDLOG_INFO(Frontend, "Citra starting...");
+    SLOG_INFO(Frontend, "Citra starting...");
     StoreRecentFile(filename); // Put the filename on top of the list
 
     if (!LoadROM(filename))
@@ -502,7 +503,7 @@ void GMainWindow::OnGameListOpenSaveFolder(u64 program_id) {
         return;
     }
 
-    SPDLOG_INFO(Frontend, "Opening save data path for program_id={0:#x}", program_id);
+    SLOG_INFO(Frontend, "Opening save data path for program_id={0:#x}", program_id);
     QDesktopServices::openUrl(QUrl::fromLocalFile(qpath));
 }
 
@@ -797,6 +798,7 @@ void GMainWindow::filterBarSetChecked(bool state) {
 int main(int argc, char* argv[]) {
     Log::Filter log_filter(Log::Level::Info);
     Log::SetFilter(&log_filter);
+    Log::SpdLogSetFilter(&log_filter);
 
     MicroProfileOnThreadCreate("Frontend");
     SCOPE_EXIT({ MicroProfileShutdown(); });
@@ -815,6 +817,8 @@ int main(int argc, char* argv[]) {
     GMainWindow main_window;
     // After settings have been loaded by GMainWindow, apply the filter
     log_filter.ParseFilterString(Settings::values.log_filter);
+    Log::SetFilter(&log_filter);
+    Log::SpdLogSetFilter(&log_filter);
 
     main_window.show();
     return app.exec();

src/common/logging/backend.cpp

@@ -87,6 +87,7 @@ const char* GetLogClassName(Class log_class) {
 #undef CLS
 #undef SUB
     case Class::Count:
+    default:
         UNREACHABLE();
     }
 }
@@ -103,6 +104,7 @@ const char* GetLevelName(Level log_level) {
         LVL(Error);
         LVL(Critical);
     case Level::Count:
+    default:
         UNREACHABLE();
     }
 #undef LVL

src/common/logging/backend_spdlog.cpp

@@ -1,14 +1,42 @@
+// Copyright 2017 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#include <array>
+#include <memory>
+#include <vector>
 #include <spdlog/spdlog.h>
 
 #include "common/assert.h"
 #include "common/logging/backend.h"
 #include "common/logging/backend_spdlog.h"
+#include "common/logging/filter.h"
 #include "common/logging/formatter.h"
 #include "common/string_util.h"
 
 namespace Log {
 
-static spdlog::level::level_enum GetLevel(Log::Level log_level) {
+class SpdLogBackend {
+public:
+    SpdLogBackend();
+    ~SpdLogBackend();
+
+    static std::shared_ptr<SpdLogBackend> Instance();
+
+    SpdLogBackend(SpdLogBackend const&) = delete;
+    const SpdLogBackend& operator=(SpdLogBackend const&) = delete;
+
+    using LogArray =
+        std::array<std::shared_ptr<spdlog::logger>, static_cast<u8>(Log::Class::Count)>;
+    const LogArray& GetLoggers() {
+        return loggers;
+    }
+
+private:
+    LogArray loggers;
+};
+
+static spdlog::level::level_enum GetSpdLogLevel(Log::Level log_level) {
     switch (log_level) {
     case Log::Level::Trace:
         return spdlog::level::trace;
@@ -27,8 +55,8 @@ static spdlog::level::level_enum GetLevel(Log::Level log_level) {
     }
 }
 
-SpdLogBackend& SpdLogBackend::Instance() {
+std::shared_ptr<SpdLogBackend> SpdLogBackend::Instance() {
-    static SpdLogBackend instance;
+    static auto instance = std::make_shared<SpdLogBackend>();
     return instance;
 }
 
@@ -36,23 +64,22 @@ SpdLogBackend::SpdLogBackend() {
     // setup the custom citra formatter
     spdlog::set_formatter(std::make_shared<Formatter>());
 
-    std::vector<spdlog::sink_ptr> sinks;
     // Define the sinks to be passed to the loggers
     // true means truncate file
     auto file_sink = std::make_shared<spdlog::sinks::simple_file_sink_mt>("citra_log.txt", true);
 #ifdef _WIN32
     auto color_sink = std::make_shared<spdlog::sinks::wincolor_stderr_sink_mt>();
 #else
-    auto stderr_sink = spdlog::sinks::stderr_sink_mt::instance();
+    auto color_sink = std::make_shared<spdlog::sinks::ansicolor_stdout_sink_mt>();
-    auto color_sink = std::make_shared<spdlog::sinks::ansicolor_sink>(stderr_sink);
 #endif
+    std::vector<spdlog::sink_ptr> sinks;
     sinks.push_back(std::move(file_sink));
     sinks.push_back(std::move(color_sink));
 
     // register all of loggers with spdlog
     for (u8 log_class = 0; log_class != static_cast<u8>(Log::Class::Count); ++log_class) {
-        spdlog::create(GetLogClassName(static_cast<Log::Class>(log_class)), begin(sinks),
+        loggers[log_class] = spdlog::create(GetLogClassName(static_cast<Log::Class>(log_class)),
-                       end(sinks));
+                                            begin(sinks), end(sinks));
     }
 }
 
@@ -62,11 +89,18 @@ SpdLogBackend::~SpdLogBackend() {
 
 void SpdLogImpl(Class log_class, Level log_level, const char* file, int line_num,
                 const char* function, const char* format, fmt::ArgList args) {
-    SpdLogBackend::Instance();
+    auto logger = SpdLogBackend::Instance()->GetLoggers()[static_cast<u8>(log_class)];
     fmt::MemoryWriter formatting_buffer;
     formatting_buffer << Common::TrimSourcePath(file) << ':' << function << ':' << line_num << ": "
                       << format;
-    auto& logger = spdlog::get(GetLogClassName(log_class));
+    logger->log(GetSpdLogLevel(log_level), formatting_buffer.c_str(), args);
-    logger->log(GetLevel(log_level), formatting_buffer.c_str(), args);
+}
+
+void SpdLogSetFilter(Filter* filter) {
+    auto loggers = SpdLogBackend::Instance()->GetLoggers();
+    auto class_level = filter->GetClassLevel();
+    for (u8 log_class = 0; log_class != static_cast<u8>(Log::Class::Count); ++log_class) {
+        loggers[log_class]->set_level(GetSpdLogLevel(class_level[log_class]));
+    }
 }
 }; // namespace Log

src/common/logging/backend_spdlog.h

@@ -1,19 +1,11 @@
-#include <memory>
+// Copyright 2017 Citra Emulator Project
-#include <vector>
+// Licensed under GPLv2 or any later version
-#include <spdlog/spdlog.h>
+// Refer to the license.txt file included.
-#include "common/logging/log.h"
+
+#pragma once
 
 namespace Log {
+class Filter;
 
-class SpdLogBackend {
+void SpdLogSetFilter(Filter* filter);
-public:
-    static SpdLogBackend& Instance();
-
-    SpdLogBackend(SpdLogBackend const&) = delete;
-    const SpdLogBackend& operator=(SpdLogBackend const&) = delete;
-
-private:
-    SpdLogBackend();
-    ~SpdLogBackend();
-};
 } // namespace Log

src/common/logging/filter.cpp

@@ -21,6 +21,10 @@ void Filter::SetClassLevel(Class log_class, Level level) {
     class_levels[static_cast<size_t>(log_class)] = level;
 }
 
+const std::array<Level, static_cast<size_t>(Class::Count)>& Filter::GetClassLevel() {
+    return class_levels;
+}
+
 void Filter::ParseFilterString(const std::string& filter_str) {
     auto clause_begin = filter_str.cbegin();
     while (clause_begin != filter_str.cend()) {
@@ -94,4 +98,4 @@ bool Filter::ParseFilterRule(const std::string::const_iterator begin,
 bool Filter::CheckMessage(Class log_class, Level level) const {
     return static_cast<u8>(level) >= static_cast<u8>(class_levels[static_cast<size_t>(log_class)]);
 }
-}
+} // namespace Log

src/common/logging/filter.h

@@ -25,6 +25,8 @@ public:
     void ResetAll(Level level);
     /// Sets the minimum level of `log_class` (and not of its subclasses) to `level`.
     void SetClassLevel(Class log_class, Level level);
+    /// Returns the list of levels that each logger is filtered to
+    const std::array<Level, static_cast<size_t>(Class::Count)>& GetClassLevel();
 
     /**
      * Parses a filter string and applies it to this filter.
@@ -50,4 +52,4 @@ public:
 private:
     std::array<Level, (size_t)Class::Count> class_levels;
 };
-}
+} // namespace Log

src/common/logging/formatter.cpp

@@ -1,3 +1,7 @@
+// Copyright 2017 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
 #include <chrono>
 #include <string>
 #include "common/assert.h"

src/common/logging/formatter.h

@@ -1,3 +1,9 @@
+// Copyright 2017 Citra Emulator Project
+// Licensed under GPLv2 or any later version
+// Refer to the license.txt file included.
+
+#pragma once
+
 #include <spdlog/spdlog.h>
 
 namespace Log {

src/common/logging/log.h

@@ -107,9 +107,11 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned
 #endif
     ;
 
+/// Logs a message to the spdlog sinks
 void SpdLogImpl(Class log_class, Level log_level, const char* file, int line_num,
                 const char* function, const char* format, fmt::ArgList args);
 
+/// Macro that creates a variadic template method and wraps the extra arguments into a fmt::ArgList
 FMT_VARIADIC(void, SpdLogImpl, Class, Level, const char*, int, const char*, const char*)
 
 } // namespace Log
@@ -137,25 +139,25 @@ FMT_VARIADIC(void, SpdLogImpl, Class, Level, const char*, int, const char*, cons
 
 // Define the spdlog level macros
 #ifdef _DEBUG
-#define SPDLOG_TRACE(log_class, fmt, ...)                                                          \
+#define SLOG_TRACE(log_class, fmt, ...)                                                            \
-    ::Log::SpdLogMessage(log_class, ::Log::Level::Trace, __FILE__, __LINE__, __func__, fmt,        \
+    ::Log::SpdLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__,         \
-                         ##__VA_ARGS__)
+                         __func__, fmt, ##__VA_ARGS__)
 #else
-#define SPDLOG_TRACE(log_class, fmt, ...) (void(0))
+#define SLOG_TRACE(log_class, fmt, ...) (void(0))
 #endif
 
-#define SPDLOG_DEBUG(log_class, fmt, ...)                                                          \
+#define SLOG_DEBUG(log_class, fmt, ...)                                                            \
     ::Log::SpdLogImpl(::Log::Class::log_class, ::Log::Level::Debug, __FILE__, __LINE__, __func__,  \
                       fmt, ##__VA_ARGS__)
-#define SPDLOG_INFO(log_class, fmt, ...)                                                           \
+#define SLOG_INFO(log_class, fmt, ...)                                                             \
     ::Log::SpdLogImpl(::Log::Class::log_class, ::Log::Level::Info, __FILE__, __LINE__, __func__,   \
                       fmt, ##__VA_ARGS__)
-#define SPDLOG_WARNING(log_class, fmt, ...)                                                        \
+#define SLOG_WARNING(log_class, fmt, ...)                                                          \
     ::Log::SpdLogImpl(::Log::Class::log_class, ::Log::Level::Warning, __FILE__, __LINE__,          \
                       __func__, fmt, ##__VA_ARGS__)
-#define SPDLOG_ERROR(log_class, fmt, ...)                                                          \
+#define SLOG_ERROR(log_class, fmt, ...)                                                            \
     ::Log::SpdLogImpl(::Log::Class::log_class, ::Log::Level::Error, __FILE__, __LINE__, __func__,  \
                       fmt, ##__VA_ARGS__)
-#define SPDLOG_CRITICAL(log_class, fmt, ...)                                                       \
+#define SLOG_CRITICAL(log_class, fmt, ...)                                                         \
     ::Log::SpdLogImpl(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__,         \
                       __func__, fmt, ##__VA_ARGS__)