From 747ff1f65e1a3f974ddf79ed31be5b19226df2a9 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Thu, 22 Jun 2017 23:02:57 -0600 Subject: [PATCH] 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 | 3 -- src/CMakeLists.txt | 2 - src/citra/citra.cpp | 30 +++++++------ src/citra/config.cpp | 6 +-- src/citra/emu_window/emu_window_sdl2.cpp | 8 ++-- src/citra_qt/game_list.cpp | 6 +-- src/citra_qt/main.cpp | 12 +++-- src/common/logging/backend.cpp | 2 + src/common/logging/backend_spdlog.cpp | 56 +++++++++++++++++++----- src/common/logging/backend_spdlog.h | 22 +++------- src/common/logging/filter.cpp | 6 ++- src/common/logging/filter.h | 4 +- src/common/logging/formatter.cpp | 4 ++ src/common/logging/formatter.h | 6 +++ src/common/logging/log.h | 20 +++++---- 15 files changed, 118 insertions(+), 69 deletions(-) diff --git a/.gitmodules b/.gitmodules index 5f8a5ab5e..ba3749031 100644 --- a/.gitmodules +++ b/.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 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9f7c1e247..a45439481 100644 --- a/src/CMakeLists.txt +++ b/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) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index dd357ff72..b9452eed5 100644 --- a/src/citra/citra.cpp +++ b/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 " - "being used with Citra. \n\n For more information on dumping and " - "decrypting games, please refer to: " - "https://citra-emu.org/wiki/dumping-game-cartridges/"); + 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 " + "decrypting games, please refer to: " + "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 diff --git a/src/citra/config.cpp b/src/citra/config.cpp index f08b4069c..5a1e3faf7 100644 --- a/src/citra/config.cpp +++ b/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(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; } diff --git a/src/citra/emu_window/emu_window_sdl2.cpp b/src/citra/emu_window/emu_window_sdl2.cpp index 47aadd60c..da8bb8ed6 100644 --- a/src/citra/emu_window/emu_window_sdl2.cpp +++ b/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(SDL_GL_GetProcAddress))) { - LOG_CRITICAL(Frontend, "Failed to initialize GL functions! Exiting..."); + SLOG_CRITICAL(Frontend, "Failed to initialize GL functions! Exiting..."); exit(1); } diff --git a/src/citra_qt/game_list.cpp b/src/citra_qt/game_list.cpp index 35ac7b47e..957f39bc9 100644 --- a/src/citra_qt/game_list.cpp +++ b/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 {}", - dir_path.toLocal8Bit().data()); + SLOG_ERROR(Frontend, "Could not find game list folder at {}", + 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); } diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index ec94a0f59..6796b7256 100644 --- a/src/citra_qt/main.cpp +++ b/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(); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 42f6a9918..28240644b 100644 --- a/src/common/logging/backend.cpp +++ b/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 diff --git a/src/common/logging/backend_spdlog.cpp b/src/common/logging/backend_spdlog.cpp index f3a7ef5f7..271cc7b24 100644 --- a/src/common/logging/backend_spdlog.cpp +++ b/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 +#include +#include #include #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 Instance(); + + SpdLogBackend(SpdLogBackend const&) = delete; + const SpdLogBackend& operator=(SpdLogBackend const&) = delete; + + using LogArray = + std::array, static_cast(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() { - static SpdLogBackend instance; +std::shared_ptr SpdLogBackend::Instance() { + static auto instance = std::make_shared(); return instance; } @@ -36,23 +64,22 @@ SpdLogBackend::SpdLogBackend() { // setup the custom citra formatter spdlog::set_formatter(std::make_shared()); - std::vector sinks; // Define the sinks to be passed to the loggers // true means truncate file auto file_sink = std::make_shared("citra_log.txt", true); #ifdef _WIN32 auto color_sink = std::make_shared(); #else - auto stderr_sink = spdlog::sinks::stderr_sink_mt::instance(); - auto color_sink = std::make_shared(stderr_sink); + auto color_sink = std::make_shared(); #endif + std::vector 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(Log::Class::Count); ++log_class) { - spdlog::create(GetLogClassName(static_cast(log_class)), begin(sinks), - end(sinks)); + loggers[log_class] = spdlog::create(GetLogClassName(static_cast(log_class)), + 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(log_class)]; fmt::MemoryWriter formatting_buffer; formatting_buffer << Common::TrimSourcePath(file) << ':' << function << ':' << line_num << ": " << format; - auto& logger = spdlog::get(GetLogClassName(log_class)); - logger->log(GetLevel(log_level), formatting_buffer.c_str(), args); + logger->log(GetSpdLogLevel(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(Log::Class::Count); ++log_class) { + loggers[log_class]->set_level(GetSpdLogLevel(class_level[log_class])); + } } }; // namespace Log diff --git a/src/common/logging/backend_spdlog.h b/src/common/logging/backend_spdlog.h index f53f974f8..8c70289cc 100644 --- a/src/common/logging/backend_spdlog.h +++ b/src/common/logging/backend_spdlog.h @@ -1,19 +1,11 @@ -#include -#include -#include -#include "common/logging/log.h" +// Copyright 2017 Citra Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once namespace Log { +class Filter; -class SpdLogBackend { -public: - static SpdLogBackend& Instance(); - - SpdLogBackend(SpdLogBackend const&) = delete; - const SpdLogBackend& operator=(SpdLogBackend const&) = delete; - -private: - SpdLogBackend(); - ~SpdLogBackend(); -}; +void SpdLogSetFilter(Filter* filter); } // namespace Log \ No newline at end of file diff --git a/src/common/logging/filter.cpp b/src/common/logging/filter.cpp index 12e5bb45d..fb6f09061 100644 --- a/src/common/logging/filter.cpp +++ b/src/common/logging/filter.cpp @@ -21,6 +21,10 @@ void Filter::SetClassLevel(Class log_class, Level level) { class_levels[static_cast(log_class)] = level; } +const std::array(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(level) >= static_cast(class_levels[static_cast(log_class)]); } -} +} // namespace Log diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index b51df61de..e8e3526a9 100644 --- a/src/common/logging/filter.h +++ b/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(Class::Count)>& GetClassLevel(); /** * Parses a filter string and applies it to this filter. @@ -50,4 +52,4 @@ public: private: std::array class_levels; }; -} +} // namespace Log diff --git a/src/common/logging/formatter.cpp b/src/common/logging/formatter.cpp index b3164de7e..52b091c1e 100644 --- a/src/common/logging/formatter.cpp +++ b/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 #include #include "common/assert.h" diff --git a/src/common/logging/formatter.h b/src/common/logging/formatter.h index ef33b66a6..acda71d29 100644 --- a/src/common/logging/formatter.h +++ b/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 namespace Log { diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 383f895cb..1054c4a52 100644 --- a/src/common/logging/log.h +++ b/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, ...) \ - ::Log::SpdLogMessage(log_class, ::Log::Level::Trace, __FILE__, __LINE__, __func__, fmt, \ - ##__VA_ARGS__) +#define SLOG_TRACE(log_class, fmt, ...) \ + ::Log::SpdLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__, \ + __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__)