From f9d6f7bc329ed5928683653a1bb4a99c87f1a793 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Tue, 16 May 2017 21:11:30 -0600 Subject: [PATCH] Functional spdlogging implementation with fmtlib --- .gitmodules | 3 ++ externals/CMakeLists.txt | 1 + src/CMakeLists.txt | 2 + src/citra_qt/game_list.cpp | 6 +-- src/citra_qt/main.cpp | 11 +++-- src/common/CMakeLists.txt | 3 +- src/common/logging/backend_spdlog.cpp | 67 +++++++++++---------------- src/common/logging/backend_spdlog.h | 4 +- src/common/logging/formatter.cpp | 10 ++-- src/common/logging/formatter.h | 2 +- src/common/logging/log.h | 16 +++++-- 11 files changed, 62 insertions(+), 63 deletions(-) diff --git a/.gitmodules b/.gitmodules index ba3749031..5f8a5ab5e 100644 --- a/.gitmodules +++ b/.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 diff --git a/externals/CMakeLists.txt b/externals/CMakeLists.txt index fdf15518c..51276bb44 100644 --- a/externals/CMakeLists.txt +++ b/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) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a45439481..9f7c1e247 100644 --- a/src/CMakeLists.txt +++ b/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) diff --git a/src/citra_qt/game_list.cpp b/src/citra_qt/game_list.cpp index 1bb076299..bf1b11940 100644 --- a/src/citra_qt/game_list.cpp +++ b/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); } diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 4f5b2ddab..80a04d836 100644 --- a/src/citra_qt/main.cpp +++ b/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!", - filename.toStdString().c_str()); + SPDLOG_CRITICAL("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: - 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)); } diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index c8dfafb24..2386f7ebb 100644 --- a/src/common/CMakeLists.txt +++ b/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 microprofile spdlog) +target_link_libraries(common PUBLIC Boost::boost fmt microprofile spdlog) if (ARCHITECTURE_x86_64) target_link_libraries(common PRIVATE xbyak) diff --git a/src/common/logging/backend_spdlog.cpp b/src/common/logging/backend_spdlog.cpp index 91f803c0b..102903f2e 100644 --- a/src/common/logging/backend_spdlog.cpp +++ b/src/common/logging/backend_spdlog.cpp @@ -1,8 +1,31 @@ +#include + #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 SpdLogBackend::GetLogger(u32 logger) const { +const std::shared_ptr& SpdLogBackend::GetLogger(u32 logger) const { return loggers[logger]; } u32 SpdLogBackend::RegisterLogger(const char* class_name) { - loggers.push_back(std::make_shared(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) { - 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 -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) { +void SpdLogImpl(u32 logger, Level log_level, const char* format, fmt::ArgList& args) { auto log = SpdLogBackend::instance().GetLogger(logger); - fmt::MemoryWriter formatting_buffer; - formatting_buffer << Common::TrimSourcePath(filename) << ':' << function << ':' << line_nr - << ": " << format; - log->log(GetLevel(log_level), formatting_buffer.c_str(), arg, args...); -} - -template -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()); + log->log(GetLevel(log_level), format, args); } u32 RegisterLogger(const char* class_name) { diff --git a/src/common/logging/backend_spdlog.h b/src/common/logging/backend_spdlog.h index 934fd25ce..db1a3229a 100644 --- a/src/common/logging/backend_spdlog.h +++ b/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 GetLogger(u32 logger) const; + const std::shared_ptr& GetLogger(u32 logger) const; private: SpdLogBackend(); diff --git a/src/common/logging/formatter.cpp b/src/common/logging/formatter.cpp index 87db782c6..56f6c6998 100644 --- a/src/common/logging/formatter.cpp +++ b/src/common/logging/formatter.cpp @@ -3,11 +3,7 @@ #include "common/assert.h" #include "common/logging/formatter.h" -namespace Log { - -Formatter::Formatter() {} - -const char* GetLevelName(spdlog::level_t log_level) { +static 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; diff --git a/src/common/logging/formatter.h b/src/common/logging/formatter.h index 586334c96..ef33b66a6 100644 --- a/src/common/logging/formatter.h +++ b/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; diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 1a73e3343..32154cde3 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -4,6 +4,7 @@ #pragma once +#include #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 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); -template -void SpdLogMessage(u32 logger, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const T& msg); + const char* function, const Arg1& format, const Args&... args) { + typedef fmt::internal::ArgArray ArgArray; + typename ArgArray::Type array{ArgArray::template make>(args)...}; + 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);