From 0daac3020ed8f347ef696097a1f980eb30858b37 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Mon, 19 Feb 2018 17:51:27 -0700 Subject: [PATCH 01/18] Logging: Add customizable logging backends and fmtlib based macros * Change the logging backend to support multiple sinks through the Backend Interface * Add a new set of logging macros to use fmtlib instead. * Qt: Compile as GUI application on windows to make the console hidden by default. Add filter configuration and a button to open log location. * SDL: Migrate to the new logging macros --- src/citra/citra.cpp | 67 ++++---- src/citra/config.cpp | 18 +- src/citra/emu_window/emu_window_sdl2.cpp | 8 +- src/citra_qt/CMakeLists.txt | 6 + src/citra_qt/configuration/config.cpp | 14 +- .../configuration/configure_debug.cpp | 23 +++ src/citra_qt/configuration/configure_debug.ui | 41 +++++ src/citra_qt/debugger/console.cpp | 34 ++++ src/citra_qt/debugger/console.h | 14 ++ src/citra_qt/main.cpp | 11 +- src/citra_qt/ui_settings.h | 2 + src/common/CMakeLists.txt | 2 +- src/common/common_paths.h | 4 + src/common/file_util.cpp | 19 ++- src/common/file_util.h | 6 +- src/common/logging/backend.cpp | 156 +++++++++++++++--- src/common/logging/backend.h | 82 ++++++++- src/common/logging/filter.h | 4 +- src/common/logging/log.h | 34 +++- src/common/logging/text_formatter.cpp | 40 +---- src/common/logging/text_formatter.h | 16 +- src/common/string_util.cpp | 24 ++- src/common/string_util.h | 15 +- src/core/settings.cpp | 3 +- src/core/settings.h | 24 ++- 25 files changed, 527 insertions(+), 140 deletions(-) create mode 100644 src/citra_qt/debugger/console.cpp create mode 100644 src/citra_qt/debugger/console.h diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index c98cbb33e..eb7a7d181 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -25,6 +25,7 @@ #include "citra/config.h" #include "citra/emu_window/emu_window_sdl2.h" +#include "common/common_paths.h" #include "common/file_util.h" #include "common/logging/backend.h" #include "common/logging/filter.h" @@ -60,39 +61,39 @@ static void PrintVersion() { static void OnStateChanged(const Network::RoomMember::State& state) { switch (state) { case Network::RoomMember::State::Idle: - LOG_DEBUG(Network, "Network is idle"); + NGLOG_DEBUG(Network, "Network is idle"); break; case Network::RoomMember::State::Joining: - LOG_DEBUG(Network, "Connection sequence to room started"); + NGLOG_DEBUG(Network, "Connection sequence to room started"); break; case Network::RoomMember::State::Joined: - LOG_DEBUG(Network, "Successfully joined to the room"); + NGLOG_DEBUG(Network, "Successfully joined to the room"); break; case Network::RoomMember::State::LostConnection: - LOG_DEBUG(Network, "Lost connection to the room"); + NGLOG_DEBUG(Network, "Lost connection to the room"); break; case Network::RoomMember::State::CouldNotConnect: - LOG_ERROR(Network, "State: CouldNotConnect"); + NGLOG_ERROR(Network, "State: CouldNotConnect"); exit(1); break; case Network::RoomMember::State::NameCollision: - LOG_ERROR( + NGLOG_ERROR( Network, "You tried to use the same nickname then another user that is connected to the Room"); exit(1); break; case Network::RoomMember::State::MacCollision: - LOG_ERROR(Network, "You tried to use the same MAC-Address then another user that is " - "connected to the Room"); + NGLOG_ERROR(Network, "You tried to use the same MAC-Address then another user that is " + "connected to the Room"); exit(1); break; case Network::RoomMember::State::WrongPassword: - LOG_ERROR(Network, "Room replied with: Wrong password"); + NGLOG_ERROR(Network, "Room replied with: Wrong password"); exit(1); break; case Network::RoomMember::State::WrongVersion: - LOG_ERROR(Network, - "You are using a different version then the room you are trying to connect to"); + NGLOG_ERROR(Network, + "You are using a different version then the room you are trying to connect to"); exit(1); break; default: @@ -119,7 +120,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"); + NGLOG_CRITICAL(Frontend, "Failed to get command line arguments"); return -1; } #endif @@ -155,7 +156,7 @@ int main(int argc, char** argv) { break; case 'i': { const auto cia_progress = [](size_t written, size_t total) { - LOG_INFO(Frontend, "%02zu%%", (written * 100 / total)); + NGLOG_INFO(Frontend, "{:02d}%", (written * 100 / total)); }; if (Service::AM::InstallCIA(std::string(optarg), cia_progress) != Service::AM::InstallStatus::Success) @@ -223,23 +224,27 @@ int main(int argc, char** argv) { LocalFree(argv_w); #endif - Log::Filter log_filter(Log::Level::Debug); - Log::SetFilter(&log_filter); - MicroProfileOnThreadCreate("EmuThread"); SCOPE_EXIT({ MicroProfileShutdown(); }); if (filepath.empty()) { - LOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified"); + NGLOG_CRITICAL(Frontend, "Failed to load ROM: No ROM specified"); return -1; } if (!movie_record.empty() && !movie_play.empty()) { - LOG_CRITICAL(Frontend, "Cannot both play and record a movie"); + NGLOG_CRITICAL(Frontend, "Cannot both play and record a movie"); return -1; } + Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter); + Log::SetGlobalFilter(log_filter); + + Log::AddBackend(std::make_unique()); + FileUtil::CreateFullPath(FileUtil::GetUserPath(D_LOGS_IDX)); + Log::AddBackend( + std::make_unique(FileUtil::GetUserPath(D_LOGS_IDX) + LOG_FILE)); // Apply the command line arguments Settings::values.gdbstub_port = gdb_port; @@ -258,28 +263,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()); + NGLOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filepath); return -1; case Core::System::ResultStatus::ErrorLoader: - LOG_CRITICAL(Frontend, "Failed to load ROM!"); + NGLOG_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/"); + NGLOG_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."); + NGLOG_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"); + NGLOG_CRITICAL(Frontend, "CPUCore not initialized"); return -1; case Core::System::ResultStatus::ErrorSystemMode: - LOG_CRITICAL(Frontend, "Failed to determine system mode!"); + NGLOG_CRITICAL(Frontend, "Failed to determine system mode!"); return -1; case Core::System::ResultStatus::ErrorVideoCore: - LOG_CRITICAL(Frontend, "VideoCore not initialized"); + NGLOG_CRITICAL(Frontend, "VideoCore not initialized"); return -1; case Core::System::ResultStatus::Success: break; // Expected case @@ -291,11 +296,11 @@ int main(int argc, char** argv) { if (auto member = Network::GetRoomMember().lock()) { member->BindOnChatMessageRecieved(OnMessageReceived); member->BindOnStateChanged(OnStateChanged); - LOG_DEBUG(Network, "Start connection to %s:%u with nickname %s", address.c_str(), port, - nickname.c_str()); + NGLOG_DEBUG(Network, "Start connection to {}:{} with nickname {}", address, port, + nickname); member->Join(nickname, address.c_str(), port, 0, Network::NoPreferredMac, password); } else { - LOG_ERROR(Network, "Could not access RoomMember"); + NGLOG_ERROR(Network, "Could not access RoomMember"); return 0; } } diff --git a/src/citra/config.cpp b/src/citra/config.cpp index b95812964..0ec7bc6b2 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); + NGLOG_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."); + NGLOG_ERROR(Config, "Failed."); return false; } - LOG_INFO(Config, "Successfully loaded %s", location); + NGLOG_INFO(Config, "Successfully loaded {}", location); return true; } @@ -49,10 +49,18 @@ static const std::array default_buttons static const std::array, Settings::NativeAnalog::NumAnalogs> default_analogs{{ { - SDL_SCANCODE_UP, SDL_SCANCODE_DOWN, SDL_SCANCODE_LEFT, SDL_SCANCODE_RIGHT, SDL_SCANCODE_D, + SDL_SCANCODE_UP, + SDL_SCANCODE_DOWN, + SDL_SCANCODE_LEFT, + SDL_SCANCODE_RIGHT, + SDL_SCANCODE_D, }, { - SDL_SCANCODE_I, SDL_SCANCODE_K, SDL_SCANCODE_J, SDL_SCANCODE_L, SDL_SCANCODE_D, + SDL_SCANCODE_I, + SDL_SCANCODE_K, + SDL_SCANCODE_J, + SDL_SCANCODE_L, + SDL_SCANCODE_D, }, }}; diff --git a/src/citra/emu_window/emu_window_sdl2.cpp b/src/citra/emu_window/emu_window_sdl2.cpp index 1a6fd0fa3..cf44ea991 100644 --- a/src/citra/emu_window/emu_window_sdl2.cpp +++ b/src/citra/emu_window/emu_window_sdl2.cpp @@ -66,7 +66,7 @@ EmuWindow_SDL2::EmuWindow_SDL2() { // Initialize the window if (SDL_Init(SDL_INIT_VIDEO) < 0) { - LOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting..."); + NGLOG_CRITICAL(Frontend, "Failed to initialize SDL2! Exiting..."); exit(1); } @@ -89,19 +89,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: %s", SDL_GetError()); + NGLOG_CRITICAL(Frontend, "Failed to create SDL2 window: {}", SDL_GetError()); exit(1); } gl_context = SDL_GL_CreateContext(render_window); if (gl_context == nullptr) { - LOG_CRITICAL(Frontend, "Failed to create SDL2 GL context: %s", SDL_GetError()); + NGLOG_CRITICAL(Frontend, "Failed to create SDL2 GL context: {}", SDL_GetError()); exit(1); } if (!gladLoadGLLoader(static_cast(SDL_GL_GetProcAddress))) { - LOG_CRITICAL(Frontend, "Failed to initialize GL functions: %s", SDL_GetError()); + NGLOG_CRITICAL(Frontend, "Failed to initialize GL functions: {}", SDL_GetError()); exit(1); } diff --git a/src/citra_qt/CMakeLists.txt b/src/citra_qt/CMakeLists.txt index b0fc1acca..8ad1b3c5f 100644 --- a/src/citra_qt/CMakeLists.txt +++ b/src/citra_qt/CMakeLists.txt @@ -28,6 +28,8 @@ add_executable(citra-qt configuration/configure_system.h configuration/configure_web.cpp configuration/configure_web.h + debugger/console.h + debugger/console.cpp debugger/graphics/graphics.cpp debugger/graphics/graphics.h debugger/graphics/graphics_breakpoint_observer.cpp @@ -138,6 +140,10 @@ if (APPLE) target_sources(citra-qt PRIVATE ${MACOSX_ICON}) set_target_properties(citra-qt PROPERTIES MACOSX_BUNDLE TRUE) set_target_properties(citra-qt PROPERTIES MACOSX_BUNDLE_INFO_PLIST ${CMAKE_CURRENT_SOURCE_DIR}/Info.plist) +elseif(WIN32) + # compile as a win32 gui application instead of a console application + target_link_libraries(citra-qt PRIVATE Qt5::WinMain) + set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "/SUBSYSTEM:WINDOWS") endif() create_target_directory_groups(citra-qt) diff --git a/src/citra_qt/configuration/config.cpp b/src/citra_qt/configuration/config.cpp index 602a19e13..c18422e3f 100644 --- a/src/citra_qt/configuration/config.cpp +++ b/src/citra_qt/configuration/config.cpp @@ -24,10 +24,18 @@ const std::array Config::default_button const std::array, Settings::NativeAnalog::NumAnalogs> Config::default_analogs{{ { - Qt::Key_Up, Qt::Key_Down, Qt::Key_Left, Qt::Key_Right, Qt::Key_D, + Qt::Key_Up, + Qt::Key_Down, + Qt::Key_Left, + Qt::Key_Right, + Qt::Key_D, }, { - Qt::Key_I, Qt::Key_K, Qt::Key_J, Qt::Key_L, Qt::Key_D, + Qt::Key_I, + Qt::Key_K, + Qt::Key_J, + Qt::Key_L, + Qt::Key_D, }, }}; @@ -216,6 +224,7 @@ void Config::ReadValues() { UISettings::values.confirm_before_closing = qt_config->value("confirmClose", true).toBool(); UISettings::values.first_start = qt_config->value("firstStart", true).toBool(); UISettings::values.callout_flags = qt_config->value("calloutFlags", 0).toUInt(); + UISettings::values.show_console = qt_config->value("showConsole", false).toBool(); qt_config->endGroup(); } @@ -357,6 +366,7 @@ void Config::SaveValues() { qt_config->setValue("confirmClose", UISettings::values.confirm_before_closing); qt_config->setValue("firstStart", UISettings::values.first_start); qt_config->setValue("calloutFlags", UISettings::values.callout_flags); + qt_config->setValue("showConsole", UISettings::values.show_console); qt_config->endGroup(); } diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index 48f57739e..9744ac6c0 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -2,13 +2,27 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include +#include + #include "citra_qt/configuration/configure_debug.h" +#include "citra_qt/debugger/console.h" +#include "citra_qt/ui_settings.h" +#include "common/file_util.h" +#include "common/logging/backend.h" +#include "common/logging/filter.h" +#include "common/logging/log.h" +#include "core/core.h" #include "core/settings.h" #include "ui_configure_debug.h" ConfigureDebug::ConfigureDebug(QWidget* parent) : QWidget(parent), ui(new Ui::ConfigureDebug) { ui->setupUi(this); this->setConfiguration(); + connect(ui->open_log_button, &QPushButton::pressed, []() { + QString path = QString::fromStdString(FileUtil::GetUserPath(D_LOGS_IDX)); + QDesktopServices::openUrl(QUrl::fromLocalFile(path)); + }); } ConfigureDebug::~ConfigureDebug() {} @@ -17,11 +31,20 @@ void ConfigureDebug::setConfiguration() { ui->toggle_gdbstub->setChecked(Settings::values.use_gdbstub); ui->gdbport_spinbox->setEnabled(Settings::values.use_gdbstub); ui->gdbport_spinbox->setValue(Settings::values.gdbstub_port); + ui->toggle_console->setEnabled(!Core::System::GetInstance().IsPoweredOn()); + ui->toggle_console->setChecked(UISettings::values.show_console); + ui->log_filter_edit->setText(QString::fromStdString(Settings::values.log_filter)); } void ConfigureDebug::applyConfiguration() { Settings::values.use_gdbstub = ui->toggle_gdbstub->isChecked(); Settings::values.gdbstub_port = ui->gdbport_spinbox->value(); + UISettings::values.show_console = ui->toggle_console->isChecked(); + Settings::values.log_filter = ui->log_filter_edit->text().toStdString(); + Debugger::ToggleConsole(); + Log::Filter filter; + filter.ParseFilterString(Settings::values.log_filter); + Log::SetGlobalFilter(filter); Settings::Apply(); } diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index a10bea2f4..118e91cf1 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -72,6 +72,47 @@ + + + + Logging + + + + + + + + Global Log Filter + + + + + + + + + + + + + + Show Log Console (Windows Only) + + + + + + + Open Log Location + + + + + + + + diff --git a/src/citra_qt/debugger/console.cpp b/src/citra_qt/debugger/console.cpp new file mode 100644 index 000000000..9d80d108e --- /dev/null +++ b/src/citra_qt/debugger/console.cpp @@ -0,0 +1,34 @@ +// Copyright 2018 Citra Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#ifdef _WIN32 +#include + +#include +#endif + +#include "citra_qt/debugger/console.h" +#include "citra_qt/ui_settings.h" + +namespace Debugger { +void ToggleConsole() { +#ifdef _WIN32 + if (UISettings::values.show_console) { + if (AllocConsole()) { + freopen_s((FILE**)stdin, "CONIN$", "r", stdin); + freopen_s((FILE**)stdout, "CONOUT$", "w", stdout); + freopen_s((FILE**)stderr, "CONOUT$", "w", stderr); + } + } else { + if (FreeConsole()) { + // In order to close the console, we have to also detach the streams on it. + // Just redirect them to NUL if there is no console window + freopen_s((FILE**)stdin, "NUL", "r", stdin); + freopen_s((FILE**)stdout, "NUL", "w", stdout); + freopen_s((FILE**)stderr, "NUL", "w", stderr); + } + } +#endif +} +} // namespace Debugger diff --git a/src/citra_qt/debugger/console.h b/src/citra_qt/debugger/console.h new file mode 100644 index 000000000..3baf0fdd4 --- /dev/null +++ b/src/citra_qt/debugger/console.h @@ -0,0 +1,14 @@ +// Copyright 2018 Citra Emulator Project +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +namespace Debugger { + +/** + * Uses the WINAPI to hide or show the stderr console. This function is a placeholder until we can + * get a real qt logging window which would work for all platforms. + */ +void ToggleConsole(); +} // namespace Debugger \ No newline at end of file diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index 2c54c1539..d60d8f511 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -20,6 +20,7 @@ #include "citra_qt/compatdb.h" #include "citra_qt/configuration/config.h" #include "citra_qt/configuration/configure_dialog.h" +#include "citra_qt/debugger/console.h" #include "citra_qt/debugger/graphics/graphics.h" #include "citra_qt/debugger/graphics/graphics_breakpoints.h" #include "citra_qt/debugger/graphics/graphics_cmdlists.h" @@ -34,6 +35,7 @@ #include "citra_qt/main.h" #include "citra_qt/ui_settings.h" #include "citra_qt/updater/updater.h" +#include "common/common_paths.h" #include "common/logging/backend.h" #include "common/logging/filter.h" #include "common/logging/log.h" @@ -369,6 +371,7 @@ void GMainWindow::RestoreUIState() { ui.action_Show_Status_Bar->setChecked(UISettings::values.show_status_bar); statusBar()->setVisible(ui.action_Show_Status_Bar->isChecked()); + Debugger::ToggleConsole(); } void GMainWindow::ConnectWidgetEvents() { @@ -1295,8 +1298,7 @@ void GMainWindow::SyncMenuUISettings() { #endif int main(int argc, char* argv[]) { - Log::Filter log_filter(Log::Level::Info); - Log::SetFilter(&log_filter); + Log::AddBackend(std::make_unique()); MicroProfileOnThreadCreate("Frontend"); SCOPE_EXIT({ MicroProfileShutdown(); }); @@ -1314,7 +1316,12 @@ int main(int argc, char* argv[]) { GMainWindow main_window; // After settings have been loaded by GMainWindow, apply the filter + Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter); + Log::SetGlobalFilter(log_filter); + FileUtil::CreateFullPath(FileUtil::GetUserPath(D_LOGS_IDX)); + Log::AddBackend( + std::make_unique(FileUtil::GetUserPath(D_LOGS_IDX) + LOG_FILE)); main_window.show(); return app.exec(); diff --git a/src/citra_qt/ui_settings.h b/src/citra_qt/ui_settings.h index caf6aea6a..d994cd5c5 100644 --- a/src/citra_qt/ui_settings.h +++ b/src/citra_qt/ui_settings.h @@ -56,6 +56,8 @@ struct Values { std::vector shortcuts; uint32_t callout_flags; + + bool show_console; }; extern Values values; diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index d136213cb..c0c1a338d 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -93,7 +93,7 @@ endif() create_target_directory_groups(common) -target_link_libraries(common PUBLIC Boost::boost microprofile) +target_link_libraries(common PUBLIC Boost::boost fmt microprofile) if (ARCHITECTURE_x86_64) target_link_libraries(common PRIVATE xbyak) endif() diff --git a/src/common/common_paths.h b/src/common/common_paths.h index d5b510cdb..f6d9ea303 100644 --- a/src/common/common_paths.h +++ b/src/common/common_paths.h @@ -36,8 +36,12 @@ #define SDMC_DIR "sdmc" #define NAND_DIR "nand" #define SYSDATA_DIR "sysdata" +#define LOG_DIR "log" // Filenames +// Files in the directory returned by GetUserPath(D_LOGS_IDX) +#define LOG_FILE "citra_log.txt" + // Files in the directory returned by GetUserPath(D_CONFIG_IDX) #define EMU_CONFIG "emu.ini" #define DEBUGGER_CONFIG "debugger.ini" diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 5ab036b34..446bd4398 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -715,6 +715,8 @@ const std::string& GetUserPath(const unsigned int DirIDX, const std::string& new paths[D_SDMC_IDX] = paths[D_USER_IDX] + SDMC_DIR DIR_SEP; paths[D_NAND_IDX] = paths[D_USER_IDX] + NAND_DIR DIR_SEP; paths[D_SYSDATA_IDX] = paths[D_USER_IDX] + SYSDATA_DIR DIR_SEP; + // TODO: Put the logs in a better location for each OS + paths[D_LOGS_IDX] = paths[D_USER_IDX] + LOG_DIR DIR_SEP; } if (!newPath.empty()) { @@ -873,20 +875,19 @@ bool IOFile::Flush() { } bool IOFile::Resize(u64 size) { - if (!IsOpen() || - 0 != + if (!IsOpen() || 0 != #ifdef _WIN32 - // ector: _chsize sucks, not 64-bit safe - // F|RES: changed to _chsize_s. i think it is 64-bit safe - _chsize_s(_fileno(m_file), size) + // ector: _chsize sucks, not 64-bit safe + // F|RES: changed to _chsize_s. i think it is 64-bit safe + _chsize_s(_fileno(m_file), size) #else - // TODO: handle 64bit and growing - ftruncate(fileno(m_file), size) + // TODO: handle 64bit and growing + ftruncate(fileno(m_file), size) #endif - ) + ) m_good = false; return m_good; } -} // namespace +} // namespace FileUtil diff --git a/src/common/file_util.h b/src/common/file_util.h index 94adfcd7e..091234977 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -224,6 +224,10 @@ public: return WriteArray(&object, 1); } + size_t WriteString(const std::string& str) { + return WriteArray(str.c_str(), str.length()); + } + bool IsOpen() const { return nullptr != m_file; } @@ -253,7 +257,7 @@ private: bool m_good = true; }; -} // namespace +} // namespace FileUtil // To deal with Windows being dumb at unicode: template diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 7e58314a3..133e63ed0 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -4,16 +4,108 @@ #include #include +#include #include +#include +#include +#include #include "common/assert.h" #include "common/common_funcs.h" // snprintf compatibility define #include "common/logging/backend.h" -#include "common/logging/filter.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" +#include "common/string_util.h" +#include "common/threadsafe_queue.h" namespace Log { +/** + * Static state as a singleton. + */ +class Impl { +public: + static Impl& Instance() { + static Impl backend; + return backend; + } + + Impl(Impl const&) = delete; + const Impl& operator=(Impl const&) = delete; + + Common::MPSCQueue& GetQueue() { + return message_queue; + } + + void AddBackend(std::unique_ptr backend) { + backends.push_back(std::move(backend)); + } + + void RemoveBackend(const std::string& backend_name) { + std::remove_if(backends.begin(), backends.end(), + [&backend_name](const auto& i) { return i->GetName() == backend_name; }); + } + + const Filter& GetGlobalFilter() const { + return filter; + } + + void SetGlobalFilter(const Filter& f) { + filter = f; + } + + Backend* GetBackend(const std::string& backend_name) { + auto it = std::find_if(backends.begin(), backends.end(), [&backend_name](const auto& i) { + return i->GetName() == backend_name; + }); + if (it == backends.end()) + return nullptr; + return it->get(); + } + +private: + Impl() : running(true) { + backend_thread = std::async(std::launch::async, [&] { + using namespace std::chrono_literals; + Entry entry; + while (running) { + if (!message_queue.Pop(entry)) { + std::this_thread::sleep_for(1ms); + continue; + } + for (const auto& backend : backends) { + backend->Write(entry); + } + } + }); + } + ~Impl() { + if (running) { + running = false; + } + } + + std::atomic_bool running; + std::future backend_thread; + std::vector> backends; + Common::MPSCQueue message_queue; + Filter filter; +}; + +void ConsoleBackend::Write(const Entry& entry) { + PrintMessage(entry); +} + +void ColorConsoleBackend::Write(const Entry& entry) { + PrintColoredMessage(entry); +} + +void FileBackend::Write(const Entry& entry) { + if (!file.IsOpen()) { + return; + } + file.WriteString(FormatLogMessage(entry) + "\n"); +} + /// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. #define ALL_LOG_CLASSES() \ CLS(Log) \ @@ -113,45 +205,65 @@ const char* GetLevelName(Level log_level) { } Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const char* format, va_list args) { - using std::chrono::steady_clock; + const char* function, const std::string& message) { using std::chrono::duration_cast; + using std::chrono::steady_clock; static steady_clock::time_point time_origin = steady_clock::now(); - std::array formatting_buffer; - Entry entry; entry.timestamp = duration_cast(steady_clock::now() - time_origin); entry.log_class = log_class; entry.log_level = log_level; - - snprintf(formatting_buffer.data(), formatting_buffer.size(), "%s:%s:%u", filename, function, - line_nr); - entry.location = std::string(formatting_buffer.data()); - - vsnprintf(formatting_buffer.data(), formatting_buffer.size(), format, args); - entry.message = std::string(formatting_buffer.data()); + entry.filename = std::string(Common::TrimSourcePath(filename)); + entry.line_num = line_nr; + entry.function = std::string(function); + entry.message = message; return entry; } -static Filter* filter = nullptr; - -void SetFilter(Filter* new_filter) { - filter = new_filter; +void SetGlobalFilter(const Filter& filter) { + Impl::Instance().SetGlobalFilter(filter); } -void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const char* format, ...) { - if (filter != nullptr && !filter->CheckMessage(log_class, log_level)) - return; +void AddBackend(std::unique_ptr backend) { + Impl::Instance().AddBackend(std::move(backend)); +} +void RemoveBackend(const std::string& backend_name) { + Impl::Instance().RemoveBackend(backend_name); +} + +Backend* GetBackend(const std::string& backend_name) { + return Impl::Instance().GetBackend(backend_name); +} + +void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, const char* format, ...) { + auto filter = Impl::Instance().GetGlobalFilter(); + if (!filter.CheckMessage(log_class, log_level)) + return; + std::array formatting_buffer; va_list args; va_start(args, format); - Entry entry = CreateEntry(log_class, log_level, filename, line_nr, function, format, args); + vsnprintf(formatting_buffer.data(), formatting_buffer.size(), format, args); va_end(args); + Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, + std::string(formatting_buffer.data())); - PrintColoredMessage(entry); + Impl::Instance().GetQueue().Push(std::move(entry)); } + +void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, const char* format, fmt::ArgList args) { + auto filter = Impl::Instance().GetGlobalFilter(); + if (!filter.CheckMessage(log_class, log_level)) + return; + + Entry entry = + CreateEntry(log_class, log_level, filename, line_num, function, fmt::format(format, args)); + + Impl::Instance().GetQueue().Push(std::move(entry)); } +} // namespace Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index c4fe2acbf..93d982c9d 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -6,8 +6,11 @@ #include #include +#include #include #include +#include "common/file_util.h" +#include "common/logging/filter.h" #include "common/logging/log.h" namespace Log { @@ -22,15 +25,80 @@ struct Entry { std::chrono::microseconds timestamp; Class log_class; Level log_level; - std::string location; + std::string filename; + unsigned int line_num; + std::string function; std::string message; Entry() = default; Entry(Entry&& o) = default; Entry& operator=(Entry&& o) = default; + Entry& operator=(const Entry& o) = default; }; +/** + * Interface for logging backends. As loggers can be created and removed at runtime, this can be + * used by a frontend for adding a custom logging backend as needed + */ +class Backend { +public: + virtual ~Backend() = default; + virtual void SetFilter(const Filter& new_filter) { + filter = new_filter; + } + virtual const char* GetName() const = 0; + virtual void Write(const Entry& entry) = 0; + +private: + Filter filter; +}; + +/** + * Backend that writes to stderr without any color commands + */ +class ConsoleBackend : public Backend { +public: + const char* GetName() const override { + return "console"; + } + void Write(const Entry& entry) override; +}; + +/** + * Backend that writes to stderr and with color + */ +class ColorConsoleBackend : public Backend { +public: + const char* GetName() const override { + return "color_console"; + } + void Write(const Entry& entry) override; +}; + +/** + * Backend that writes to a file passed into the constructor + */ +class FileBackend : public Backend { +public: + FileBackend(const std::string& filename) : file(filename, "w") {} + + const char* GetName() const override { + return "file"; + } + + void Write(const Entry& entry) override; + +private: + FileUtil::IOFile file; +}; + +void AddBackend(std::unique_ptr backend); + +void RemoveBackend(const std::string& backend_name); + +Backend* GetBackend(const std::string& backend_name); + /** * Returns the name of the passed log class as a C-string. Subclasses are separated by periods * instead of underscores as in the enumeration. @@ -44,7 +112,13 @@ const char* GetLevelName(Level log_level); /// Creates a log entry by formatting the given source location, and message. Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const char* format, va_list args); + const char* function, const char* format, const std::string& message); -void SetFilter(Filter* filter); -} +/** + * The global filter will prevent any messages from even being processed if they are filtered. Each + * backend can have a filter, but if the level is lower than the global filter, the backend will + * never get the message + */ +void SetGlobalFilter(const Filter& filter); + +} // namespace Log diff --git a/src/common/logging/filter.h b/src/common/logging/filter.h index b51df61de..ccca289bd 100644 --- a/src/common/logging/filter.h +++ b/src/common/logging/filter.h @@ -19,7 +19,7 @@ namespace Log { class Filter { public: /// Initializes the filter with all classes having `default_level` as the minimum level. - Filter(Level default_level); + Filter(Level default_level = Level::Info); /// Resets the filter so that all classes have `level` as the minimum displayed level. void ResetAll(Level level); @@ -50,4 +50,4 @@ public: private: std::array class_levels; }; -} +} // namespace Log diff --git a/src/common/logging/log.h b/src/common/logging/log.h index f602c3ee6..59ba3a778 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 { @@ -98,7 +99,7 @@ enum class Class : ClassType { }; /// Logs a message to the global logger. -void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_nr, +void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, #ifdef _MSC_VER _Printf_format_string_ @@ -110,6 +111,12 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned #endif ; +/// Logs a message to the global logger, this time with 100% moar fmtlib +void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, const char* format, fmt::ArgList); + +FMT_VARIADIC(void, FmtLogMessage, Class, Level, const char*, unsigned int, const char*, const char*) + } // namespace Log #define LOG_GENERIC(log_class, log_level, ...) \ @@ -132,3 +139,28 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned LOG_GENERIC(::Log::Class::log_class, ::Log::Level::Error, __VA_ARGS__) #define LOG_CRITICAL(log_class, ...) \ LOG_GENERIC(::Log::Class::log_class, ::Log::Level::Critical, __VA_ARGS__) + +// Define the fmt lib macros +#ifdef _DEBUG +#define NGLOG_TRACE(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) +#else +#define NGLOG_TRACE(log_class, fmt, ...) (void(0)) +#endif + +#define NGLOG_DEBUG(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Debug, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) +#define NGLOG_INFO(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Info, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) +#define NGLOG_WARNING(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Warning, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) +#define NGLOG_ERROR(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Error, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) +#define NGLOG_CRITICAL(log_class, fmt, ...) \ + ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__, \ + __func__, fmt, ##__VA_ARGS__) diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index f71e748d1..8382ad5da 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -18,50 +18,26 @@ namespace Log { -// TODO(bunnei): This should be moved to a generic path manipulation library -const char* TrimSourcePath(const char* path, const char* root) { - const char* p = path; - - while (*p != '\0') { - const char* next_slash = p; - while (*next_slash != '\0' && *next_slash != '/' && *next_slash != '\\') { - ++next_slash; - } - - bool is_src = Common::ComparePartialString(p, next_slash, root); - p = next_slash; - - if (*p != '\0') { - ++p; - } - if (is_src) { - path = p; - } - } - return path; -} - -void FormatLogMessage(const Entry& entry, char* out_text, size_t text_len) { +std::string FormatLogMessage(const Entry& entry) { unsigned int time_seconds = static_cast(entry.timestamp.count() / 1000000); unsigned int time_fractional = static_cast(entry.timestamp.count() % 1000000); const char* class_name = GetLogClassName(entry.log_class); const char* level_name = GetLevelName(entry.log_level); - snprintf(out_text, text_len, "[%4u.%06u] %s <%s> %s: %s", time_seconds, time_fractional, - class_name, level_name, TrimSourcePath(entry.location.c_str()), entry.message.c_str()); + return fmt::format("[{:4d}.{:06d}] {} <{}> {}:{}:{}: {}", time_seconds, time_fractional, + class_name, level_name, entry.filename, entry.function, entry.line_num, + entry.message); } void PrintMessage(const Entry& entry) { - std::array format_buffer; - FormatLogMessage(entry, format_buffer.data(), format_buffer.size()); - fputs(format_buffer.data(), stderr); - fputc('\n', stderr); + auto str = FormatLogMessage(entry) + "\n"; + fputs(str.c_str(), stderr); } void PrintColoredMessage(const Entry& entry) { #ifdef _WIN32 - static HANDLE console_handle = GetStdHandle(STD_ERROR_HANDLE); + HANDLE console_handle = GetStdHandle(STD_ERROR_HANDLE); CONSOLE_SCREEN_BUFFER_INFO original_info = {0}; GetConsoleScreenBufferInfo(console_handle, &original_info); @@ -129,4 +105,4 @@ void PrintColoredMessage(const Entry& entry) { #undef ESC #endif } -} +} // namespace Log diff --git a/src/common/logging/text_formatter.h b/src/common/logging/text_formatter.h index 749268310..c587faefb 100644 --- a/src/common/logging/text_formatter.h +++ b/src/common/logging/text_formatter.h @@ -10,22 +10,10 @@ namespace Log { struct Entry; -/** - * Attempts to trim an arbitrary prefix from `path`, leaving only the part starting at `root`. It's - * intended to be used to strip a system-specific build directory from the `__FILE__` macro, - * leaving only the path relative to the sources root. - * - * @param path The input file path as a null-terminated string - * @param root The name of the root source directory as a null-terminated string. Path up to and - * including the last occurrence of this name will be stripped - * @return A pointer to the same string passed as `path`, but starting at the trimmed portion - */ -const char* TrimSourcePath(const char* path, const char* root = "src"); - /// Formats a log entry into the provided text buffer. -void FormatLogMessage(const Entry& entry, char* out_text, size_t text_len); +std::string FormatLogMessage(const Entry& entry); /// Formats and prints a log entry to stderr. void PrintMessage(const Entry& entry); /// Prints the same message as `PrintMessage`, but colored acoording to the severity level. void PrintColoredMessage(const Entry& entry); -} +} // namespace Log diff --git a/src/common/string_util.cpp b/src/common/string_util.cpp index 6959915fa..0b120e407 100644 --- a/src/common/string_util.cpp +++ b/src/common/string_util.cpp @@ -202,7 +202,7 @@ bool SplitPath(const std::string& full_path, std::string* _pPath, std::string* _ #ifdef _WIN32 ":" #endif - ); + ); if (std::string::npos == dir_end) dir_end = 0; else @@ -462,4 +462,26 @@ std::string StringFromFixedZeroTerminatedBuffer(const char* buffer, size_t max_l return std::string(buffer, len); } + +const char* TrimSourcePath(const char* path, const char* root) { + const char* p = path; + + while (*p != '\0') { + const char* next_slash = p; + while (*next_slash != '\0' && *next_slash != '/' && *next_slash != '\\') { + ++next_slash; + } + + bool is_src = Common::ComparePartialString(p, next_slash, root); + p = next_slash; + + if (*p != '\0') { + ++p; + } + if (is_src) { + path = p; + } + } + return path; } +} // namespace Common diff --git a/src/common/string_util.h b/src/common/string_util.h index 259360aec..ec0c31a24 100644 --- a/src/common/string_util.h +++ b/src/common/string_util.h @@ -134,4 +134,17 @@ bool ComparePartialString(InIt begin, InIt end, const char* other) { * NUL-terminated then the string ends at max_len characters. */ std::string StringFromFixedZeroTerminatedBuffer(const char* buffer, size_t max_len); -} + +/** + * Attempts to trim an arbitrary prefix from `path`, leaving only the part starting at `root`. It's + * intended to be used to strip a system-specific build directory from the `__FILE__` macro, + * leaving only the path relative to the sources root. + * + * @param path The input file path as a null-terminated string + * @param root The name of the root source directory as a null-terminated string. Path up to and + * including the last occurrence of this name will be stripped + * @return A pointer to the same string passed as `path`, but starting at the trimmed portion + */ +const char* TrimSourcePath(const char* path, const char* root = "src"); + +} // namespace Common diff --git a/src/core/settings.cpp b/src/core/settings.cpp index f457c3d9c..770b7fda7 100644 --- a/src/core/settings.cpp +++ b/src/core/settings.cpp @@ -4,14 +4,13 @@ #include "audio_core/dsp_interface.h" #include "core/core.h" +#include "core/frontend/emu_window.h" #include "core/gdbstub/gdbstub.h" #include "core/hle/service/hid/hid.h" #include "core/hle/service/ir/ir.h" #include "core/settings.h" #include "video_core/video_core.h" -#include "core/frontend/emu_window.h" - namespace Settings { Values values = {}; diff --git a/src/core/settings.h b/src/core/settings.h index 1b639cf46..d574a1578 100644 --- a/src/core/settings.h +++ b/src/core/settings.h @@ -54,9 +54,21 @@ constexpr int NUM_BUTTONS_IR = BUTTON_IR_END - BUTTON_IR_BEGIN; constexpr int NUM_BUTTONS_NS = BUTTON_NS_END - BUTTON_NS_BEGIN; static const std::array mapping = {{ - "button_a", "button_b", "button_x", "button_y", "button_up", "button_down", "button_left", - "button_right", "button_l", "button_r", "button_start", "button_select", "button_zl", - "button_zr", "button_home", + "button_a", + "button_b", + "button_x", + "button_y", + "button_up", + "button_down", + "button_left", + "button_right", + "button_l", + "button_r", + "button_start", + "button_select", + "button_zl", + "button_zr", + "button_home", }}; } // namespace NativeButton @@ -69,7 +81,8 @@ enum Values { }; static const std::array mapping = {{ - "circle_pad", "c_stick", + "circle_pad", + "c_stick", }}; } // namespace NativeAnalog @@ -116,8 +129,6 @@ struct Values { float bg_green; float bg_blue; - std::string log_filter; - // Audio std::string sink_id; bool enable_audio_stretching; @@ -130,6 +141,7 @@ struct Values { // Debugging bool use_gdbstub; u16 gdbstub_port; + std::string log_filter; // Movie std::string movie_play; From f6762f05cda389d0e646f8e222ec91f21e001a91 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Mon, 19 Feb 2018 21:26:26 -0700 Subject: [PATCH 02/18] Use the correct linker flag for mingw --- src/citra_qt/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/citra_qt/CMakeLists.txt b/src/citra_qt/CMakeLists.txt index 8ad1b3c5f..a70476998 100644 --- a/src/citra_qt/CMakeLists.txt +++ b/src/citra_qt/CMakeLists.txt @@ -143,7 +143,11 @@ if (APPLE) elseif(WIN32) # compile as a win32 gui application instead of a console application target_link_libraries(citra-qt PRIVATE Qt5::WinMain) - set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "/SUBSYSTEM:WINDOWS") + if(MSVC) + set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "/SUBSYSTEM:WINDOWS") + elseif(MINGW) + set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "-mwindows") + endif() endif() create_target_directory_groups(citra-qt) From 7b78425d6b42a7ca6ddf39cf2f43fcbdc01d85a3 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Mon, 19 Feb 2018 21:35:57 -0700 Subject: [PATCH 03/18] Address review comments --- src/common/logging/backend.cpp | 30 +++++++++++++-------------- src/common/logging/backend.h | 7 ++++--- src/common/logging/text_formatter.cpp | 5 ++++- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 133e63ed0..dbc11fb1e 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -32,8 +32,8 @@ public: Impl(Impl const&) = delete; const Impl& operator=(Impl const&) = delete; - Common::MPSCQueue& GetQueue() { - return message_queue; + void PushEntry(Entry e) { + message_queue.Push(std::move(e)); } void AddBackend(std::unique_ptr backend) { @@ -53,17 +53,17 @@ public: filter = f; } - Backend* GetBackend(const std::string& backend_name) { + boost::optional GetBackend(const std::string& backend_name) { auto it = std::find_if(backends.begin(), backends.end(), [&backend_name](const auto& i) { return i->GetName() == backend_name; }); if (it == backends.end()) - return nullptr; + return boost::none; return it->get(); } private: - Impl() : running(true) { + Impl() { backend_thread = std::async(std::launch::async, [&] { using namespace std::chrono_literals; Entry entry; @@ -79,12 +79,10 @@ private: }); } ~Impl() { - if (running) { - running = false; - } + running = false; } - std::atomic_bool running; + std::atomic_bool running{true}; std::future backend_thread; std::vector> backends; Common::MPSCQueue message_queue; @@ -103,7 +101,7 @@ void FileBackend::Write(const Entry& entry) { if (!file.IsOpen()) { return; } - file.WriteString(FormatLogMessage(entry) + "\n"); + file.WriteString(FormatLogMessage(entry) + '\n'); } /// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. @@ -205,7 +203,7 @@ const char* GetLevelName(Level log_level) { } Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const std::string& message) { + const char* function, std::string message) { using std::chrono::duration_cast; using std::chrono::steady_clock; @@ -215,9 +213,9 @@ Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsign entry.timestamp = duration_cast(steady_clock::now() - time_origin); entry.log_class = log_class; entry.log_level = log_level; - entry.filename = std::string(Common::TrimSourcePath(filename)); + entry.filename = Common::TrimSourcePath(filename); entry.line_num = line_nr; - entry.function = std::string(function); + entry.function = function; entry.message = message; return entry; @@ -235,7 +233,7 @@ void RemoveBackend(const std::string& backend_name) { Impl::Instance().RemoveBackend(backend_name); } -Backend* GetBackend(const std::string& backend_name) { +boost::optional GetBackend(const std::string& backend_name) { return Impl::Instance().GetBackend(backend_name); } @@ -252,7 +250,7 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, std::string(formatting_buffer.data())); - Impl::Instance().GetQueue().Push(std::move(entry)); + Impl::Instance().PushEntry(std::move(entry)); } void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, @@ -264,6 +262,6 @@ void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsig Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, fmt::format(format, args)); - Impl::Instance().GetQueue().Push(std::move(entry)); + Impl::Instance().PushEntry(std::move(entry)); } } // namespace Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 93d982c9d..dfa4944f4 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -9,6 +9,7 @@ #include #include #include +#include #include "common/file_util.h" #include "common/logging/filter.h" #include "common/logging/log.h" @@ -81,7 +82,7 @@ public: */ class FileBackend : public Backend { public: - FileBackend(const std::string& filename) : file(filename, "w") {} + explicit FileBackend(const std::string& filename) : file(filename, "w") {} const char* GetName() const override { return "file"; @@ -97,7 +98,7 @@ void AddBackend(std::unique_ptr backend); void RemoveBackend(const std::string& backend_name); -Backend* GetBackend(const std::string& backend_name); +boost::optional GetBackend(const std::string& backend_name); /** * Returns the name of the passed log class as a C-string. Subclasses are separated by periods @@ -112,7 +113,7 @@ const char* GetLevelName(Level log_level); /// Creates a log entry by formatting the given source location, and message. Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, const char* format, const std::string& message); + const char* function, std::string message); /** * The global filter will prevent any messages from even being processed if they are filtered. Each diff --git a/src/common/logging/text_formatter.cpp b/src/common/logging/text_formatter.cpp index 8382ad5da..8583916a8 100644 --- a/src/common/logging/text_formatter.cpp +++ b/src/common/logging/text_formatter.cpp @@ -31,13 +31,16 @@ std::string FormatLogMessage(const Entry& entry) { } void PrintMessage(const Entry& entry) { - auto str = FormatLogMessage(entry) + "\n"; + auto str = FormatLogMessage(entry) + '\n'; fputs(str.c_str(), stderr); } void PrintColoredMessage(const Entry& entry) { #ifdef _WIN32 HANDLE console_handle = GetStdHandle(STD_ERROR_HANDLE); + if (console_handle == INVALID_HANDLE_VALUE) { + return; + } CONSOLE_SCREEN_BUFFER_INFO original_info = {0}; GetConsoleScreenBufferInfo(console_handle, &original_info); From 47f0185bcd2ca0903131ddbb9b5d87e9acafa0cc Mon Sep 17 00:00:00 2001 From: James Rowe Date: Mon, 19 Feb 2018 22:17:49 -0700 Subject: [PATCH 04/18] fixup! move message --- src/common/logging/backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index dbc11fb1e..4d80fd8e2 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -216,7 +216,7 @@ Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsign entry.filename = Common::TrimSourcePath(filename); entry.line_num = line_nr; entry.function = function; - entry.message = message; + entry.message = std::move(message); return entry; } From 9fdc89a4568e340fd88889235473e1c5b7d34623 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Fri, 23 Feb 2018 00:30:43 -0700 Subject: [PATCH 05/18] SPSCQueue: Add PopWait Adds a condition var to SPSCQueue so when a new log is pushed it will wake the consumer thread that is calling PopWait. This only applies to to queues with NeedSize=true --- src/common/threadsafe_queue.h | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/common/threadsafe_queue.h b/src/common/threadsafe_queue.h index a0c731e8c..1c676d202 100644 --- a/src/common/threadsafe_queue.h +++ b/src/common/threadsafe_queue.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include "common/common_types.h" @@ -36,6 +37,10 @@ public: T& Front() const { return read_ptr->current; } + + /** + * Push data to the queue. If NeedSize=True then Push will notify the waiting consumer thread + */ template void Push(Arg&& t) { // create the element, add it to the queue @@ -45,8 +50,11 @@ public: ElementPtr* new_ptr = new ElementPtr(); write_ptr->next.store(new_ptr, std::memory_order_release); write_ptr = new_ptr; - if (NeedSize) + if (NeedSize) { + std::lock_guard lock(size_lock); size++; + size_cv.notify_one(); + } } void Pop() { @@ -75,6 +83,25 @@ public: return true; } + /** + * Waits up to timeout for data to be Pushed to the queue. Push uses a condition variable to + * signal the waiting thread, but only if NeedSize = true. Returns false if the timeout is + * triggered. If the condition variable is signalled, returns the value from Pop + * @param T In parameter to store the value if this method returns true + * @param timeout Time in milliseconds to wait for a signal from a Push + */ + bool PopWait(T& t, u64 timeout = 500) { + if (NeedSize) { + std::unique_lock lock(size_lock); + if (size_cv.wait_for(lock, std::chrono::milliseconds(timeout), + [& size = size] { return size > 0; })) { + return Pop(t); + } + return false; + } + return Pop(t); + } + // not thread-safe void Clear() { size.store(0); @@ -102,6 +129,9 @@ private: ElementPtr* write_ptr; ElementPtr* read_ptr; std::atomic size; + + std::mutex size_lock; + std::condition_variable size_cv; }; // a simple thread-safe, From 87bc5266ef14acf41f0751873cca469ef7e2e01b Mon Sep 17 00:00:00 2001 From: James Rowe Date: Fri, 23 Feb 2018 00:33:44 -0700 Subject: [PATCH 06/18] Logging: Various logging improvements * Uses PopWait to reduce the amount of busy waiting if there aren't many new logs * Opens the log file as shared on windows, letting other programs read the logs, but not write to them while citra is running * Flushes the logs to disk if a log >= error arrives --- src/common/file_util.cpp | 15 ++++++++++----- src/common/file_util.h | 5 +++-- src/common/logging/backend.cpp | 15 +++++++++++++-- src/common/logging/backend.h | 5 +++-- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 446bd4398..7b256fede 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -803,8 +803,8 @@ void SplitFilename83(const std::string& filename, std::array& short_nam IOFile::IOFile() {} -IOFile::IOFile(const std::string& filename, const char openmode[]) { - Open(filename, openmode); +IOFile::IOFile(const std::string& filename, const char openmode[], int flags) { + Open(filename, openmode, flags); } IOFile::~IOFile() { @@ -825,11 +825,16 @@ void IOFile::Swap(IOFile& other) { std::swap(m_good, other.m_good); } -bool IOFile::Open(const std::string& filename, const char openmode[]) { +bool IOFile::Open(const std::string& filename, const char openmode[], int flags) { Close(); #ifdef _WIN32 - _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), - Common::UTF8ToUTF16W(openmode).c_str()); + if (flags != 0) { + m_file = _wfsopen(Common::UTF8ToUTF16W(filename).c_str(), + Common::UTF8ToUTF16W(openmode).c_str(), flags); + } else { + _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), + Common::UTF8ToUTF16W(openmode).c_str()); + } #else m_file = fopen(filename.c_str(), openmode); #endif diff --git a/src/common/file_util.h b/src/common/file_util.h index 091234977..8674ac224 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -156,7 +156,8 @@ void SplitFilename83(const std::string& filename, std::array& short_nam class IOFile : public NonCopyable { public: IOFile(); - IOFile(const std::string& filename, const char openmode[]); + /// Opens the file. flags is for windows shared file settings and are ignored on other oses + IOFile(const std::string& filename, const char openmode[], int flags = 0); ~IOFile(); @@ -165,7 +166,7 @@ public: void Swap(IOFile& other); - bool Open(const std::string& filename, const char openmode[]); + bool Open(const std::string& filename, const char openmode[], int flags = 0); bool Close(); template diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 4d80fd8e2..cae48d9f3 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -9,6 +9,11 @@ #include #include #include +#ifdef _WIN32 +#include // For _SH_DENYWR +#else +#define _SH_DENYWR 0 +#endif #include "common/assert.h" #include "common/common_funcs.h" // snprintf compatibility define #include "common/logging/backend.h" @@ -68,8 +73,7 @@ private: using namespace std::chrono_literals; Entry entry; while (running) { - if (!message_queue.Pop(entry)) { - std::this_thread::sleep_for(1ms); + if (!message_queue.PopWait(entry)) { continue; } for (const auto& backend : backends) { @@ -97,11 +101,18 @@ void ColorConsoleBackend::Write(const Entry& entry) { PrintColoredMessage(entry); } +// _SH_DENYWR allows read only access to the file for other programs. +// It is #defined to 0 on other platforms +FileBackend::FileBackend(const std::string& filename) : file(filename, "w", _SH_DENYWR) {} + void FileBackend::Write(const Entry& entry) { if (!file.IsOpen()) { return; } file.WriteString(FormatLogMessage(entry) + '\n'); + if (entry.log_level >= Level::Error) { + file.Flush(); + } } /// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index dfa4944f4..5feef5dd4 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -78,11 +78,12 @@ public: }; /** - * Backend that writes to a file passed into the constructor + * Backend that writes to a file passed into the constructor. If a log level is error or higher, it + * will flush immediately after writing */ class FileBackend : public Backend { public: - explicit FileBackend(const std::string& filename) : file(filename, "w") {} + explicit FileBackend(const std::string& filename); const char* GetName() const override { return "file"; From ab4ba71f3e16c393b61cde313d201d42dc5c47f2 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Sat, 24 Feb 2018 17:41:15 -0700 Subject: [PATCH 07/18] fixup! Prevent crashes on closing by waiting for the impl thread --- src/common/logging/backend.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index cae48d9f3..ef530e3ee 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -84,6 +84,7 @@ private: } ~Impl() { running = false; + backend_thread.get(); } std::atomic_bool running{true}; From eee388588e1d99deae59c358d48f4f79ea3f1dd7 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Fri, 16 Mar 2018 11:48:33 +0800 Subject: [PATCH 08/18] Logging: Fix fmt errors after rebasing with master fmt was updated during the clang-format update, which breaks the previous implementation of FmtLogMessage Changes were: * Move definition of FmtLogMessage into log.h to use variadic templates as FMT_VARIADIC was removed To supplement the change above: * Move Entry and CreateEntry into log.h * Add LogEntry in backend.cpp --- src/common/logging/backend.cpp | 8 ++----- src/common/logging/backend.h | 24 --------------------- src/common/logging/log.h | 38 +++++++++++++++++++++++++++++++--- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index ef530e3ee..20b4fe5ea 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -265,15 +265,11 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned Impl::Instance().PushEntry(std::move(entry)); } -void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, - const char* function, const char* format, fmt::ArgList args) { +void LogEntry(Entry& entry) { auto filter = Impl::Instance().GetGlobalFilter(); - if (!filter.CheckMessage(log_class, log_level)) + if (!filter.CheckMessage(entry.log_class, entry.log_level)) return; - Entry entry = - CreateEntry(log_class, log_level, filename, line_num, function, fmt::format(format, args)); - Impl::Instance().PushEntry(std::move(entry)); } } // namespace Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 5feef5dd4..0bf9b8fd7 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -18,26 +18,6 @@ namespace Log { class Filter; -/** - * A log entry. Log entries are store in a structured format to permit more varied output - * formatting on different frontends, as well as facilitating filtering and aggregation. - */ -struct Entry { - std::chrono::microseconds timestamp; - Class log_class; - Level log_level; - std::string filename; - unsigned int line_num; - std::string function; - std::string message; - - Entry() = default; - Entry(Entry&& o) = default; - - Entry& operator=(Entry&& o) = default; - Entry& operator=(const Entry& o) = default; -}; - /** * Interface for logging backends. As loggers can be created and removed at runtime, this can be * used by a frontend for adding a custom logging backend as needed @@ -112,10 +92,6 @@ const char* GetLogClassName(Class log_class); */ const char* GetLevelName(Level log_level); -/// Creates a log entry by formatting the given source location, and message. -Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, std::string message); - /** * The global filter will prevent any messages from even being processed if they are filtered. Each * backend can have a filter, but if the level is lower than the global filter, the backend will diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 59ba3a778..ba3831ea6 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include "common/common_types.h" @@ -98,6 +99,33 @@ enum class Class : ClassType { Count ///< Total number of logging classes }; +/** + * A log entry. Log entries are store in a structured format to permit more varied output + * formatting on different frontends, as well as facilitating filtering and aggregation. + */ +struct Entry { + std::chrono::microseconds timestamp; + Class log_class; + Level log_level; + std::string filename; + unsigned int line_num; + std::string function; + std::string message; + + Entry() = default; + Entry(Entry&& o) = default; + + Entry& operator=(Entry&& o) = default; + Entry& operator=(const Entry& o) = default; +}; + +/// Creates a log entry by formatting the given source location, and message. +Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, + const char* function, std::string message); + +// Logs an Entry +void LogEntry(Entry& entry); + /// Logs a message to the global logger. void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, @@ -112,10 +140,14 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned ; /// Logs a message to the global logger, this time with 100% moar fmtlib +template void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, - const char* function, const char* format, fmt::ArgList); + const char* function, const char* format, const Args & ... args) { + Entry entry = + CreateEntry(log_class, log_level, filename, line_num, function, fmt::format(format, args...)); -FMT_VARIADIC(void, FmtLogMessage, Class, Level, const char*, unsigned int, const char*, const char*) + LogEntry(entry); +} } // namespace Log @@ -163,4 +195,4 @@ FMT_VARIADIC(void, FmtLogMessage, Class, Level, const char*, unsigned int, const __func__, fmt, ##__VA_ARGS__) #define NGLOG_CRITICAL(log_class, fmt, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) +__func__, fmt, ##__VA_ARGS__) From 373efd3158392ef105287f675daf235d27537bab Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Fri, 16 Mar 2018 11:56:40 +0800 Subject: [PATCH 09/18] Logging: Remove customizable logging backends Separate code for customizing logging backends from this branch --- src/citra/citra.cpp | 7 +- .../configuration/configure_debug.cpp | 2 +- src/citra_qt/main.cpp | 7 +- src/common/logging/backend.cpp | 129 ++---------------- src/common/logging/backend.h | 75 +--------- 5 files changed, 12 insertions(+), 208 deletions(-) diff --git a/src/citra/citra.cpp b/src/citra/citra.cpp index eb7a7d181..d8209fec9 100644 --- a/src/citra/citra.cpp +++ b/src/citra/citra.cpp @@ -239,12 +239,7 @@ int main(int argc, char** argv) { Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter); - Log::SetGlobalFilter(log_filter); - - Log::AddBackend(std::make_unique()); - FileUtil::CreateFullPath(FileUtil::GetUserPath(D_LOGS_IDX)); - Log::AddBackend( - std::make_unique(FileUtil::GetUserPath(D_LOGS_IDX) + LOG_FILE)); + Log::SetFilter(&log_filter); // Apply the command line arguments Settings::values.gdbstub_port = gdb_port; diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index 9744ac6c0..b9eb0e3d1 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -44,7 +44,7 @@ void ConfigureDebug::applyConfiguration() { Debugger::ToggleConsole(); Log::Filter filter; filter.ParseFilterString(Settings::values.log_filter); - Log::SetGlobalFilter(filter); + Log::SetFilter(&filter); Settings::Apply(); } diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index d60d8f511..c164eeb81 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -1298,8 +1298,6 @@ void GMainWindow::SyncMenuUISettings() { #endif int main(int argc, char* argv[]) { - Log::AddBackend(std::make_unique()); - MicroProfileOnThreadCreate("Frontend"); SCOPE_EXIT({ MicroProfileShutdown(); }); @@ -1318,10 +1316,7 @@ int main(int argc, char* argv[]) { // After settings have been loaded by GMainWindow, apply the filter Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter); - Log::SetGlobalFilter(log_filter); - FileUtil::CreateFullPath(FileUtil::GetUserPath(D_LOGS_IDX)); - Log::AddBackend( - std::make_unique(FileUtil::GetUserPath(D_LOGS_IDX) + LOG_FILE)); + Log::SetFilter(&log_filter); main_window.show(); return app.exec(); diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 20b4fe5ea..3c0361713 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -4,118 +4,17 @@ #include #include -#include #include -#include -#include -#include -#ifdef _WIN32 -#include // For _SH_DENYWR -#else -#define _SH_DENYWR 0 -#endif #include "common/assert.h" #include "common/common_funcs.h" // snprintf compatibility define #include "common/logging/backend.h" +#include "common/logging/filter.h" #include "common/logging/log.h" #include "common/logging/text_formatter.h" #include "common/string_util.h" -#include "common/threadsafe_queue.h" namespace Log { -/** - * Static state as a singleton. - */ -class Impl { -public: - static Impl& Instance() { - static Impl backend; - return backend; - } - - Impl(Impl const&) = delete; - const Impl& operator=(Impl const&) = delete; - - void PushEntry(Entry e) { - message_queue.Push(std::move(e)); - } - - void AddBackend(std::unique_ptr backend) { - backends.push_back(std::move(backend)); - } - - void RemoveBackend(const std::string& backend_name) { - std::remove_if(backends.begin(), backends.end(), - [&backend_name](const auto& i) { return i->GetName() == backend_name; }); - } - - const Filter& GetGlobalFilter() const { - return filter; - } - - void SetGlobalFilter(const Filter& f) { - filter = f; - } - - boost::optional GetBackend(const std::string& backend_name) { - auto it = std::find_if(backends.begin(), backends.end(), [&backend_name](const auto& i) { - return i->GetName() == backend_name; - }); - if (it == backends.end()) - return boost::none; - return it->get(); - } - -private: - Impl() { - backend_thread = std::async(std::launch::async, [&] { - using namespace std::chrono_literals; - Entry entry; - while (running) { - if (!message_queue.PopWait(entry)) { - continue; - } - for (const auto& backend : backends) { - backend->Write(entry); - } - } - }); - } - ~Impl() { - running = false; - backend_thread.get(); - } - - std::atomic_bool running{true}; - std::future backend_thread; - std::vector> backends; - Common::MPSCQueue message_queue; - Filter filter; -}; - -void ConsoleBackend::Write(const Entry& entry) { - PrintMessage(entry); -} - -void ColorConsoleBackend::Write(const Entry& entry) { - PrintColoredMessage(entry); -} - -// _SH_DENYWR allows read only access to the file for other programs. -// It is #defined to 0 on other platforms -FileBackend::FileBackend(const std::string& filename) : file(filename, "w", _SH_DENYWR) {} - -void FileBackend::Write(const Entry& entry) { - if (!file.IsOpen()) { - return; - } - file.WriteString(FormatLogMessage(entry) + '\n'); - if (entry.log_level >= Level::Error) { - file.Flush(); - } -} - /// Macro listing all log classes. Code should define CLS and SUB as desired before invoking this. #define ALL_LOG_CLASSES() \ CLS(Log) \ @@ -233,26 +132,15 @@ Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsign return entry; } -void SetGlobalFilter(const Filter& filter) { - Impl::Instance().SetGlobalFilter(filter); -} +static Filter* filter = nullptr; -void AddBackend(std::unique_ptr backend) { - Impl::Instance().AddBackend(std::move(backend)); -} - -void RemoveBackend(const std::string& backend_name) { - Impl::Instance().RemoveBackend(backend_name); -} - -boost::optional GetBackend(const std::string& backend_name) { - return Impl::Instance().GetBackend(backend_name); +void SetFilter(Filter* new_filter) { + filter = new_filter; } void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, ...) { - auto filter = Impl::Instance().GetGlobalFilter(); - if (!filter.CheckMessage(log_class, log_level)) + if (!filter->CheckMessage(log_class, log_level)) return; std::array formatting_buffer; va_list args; @@ -262,14 +150,13 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, std::string(formatting_buffer.data())); - Impl::Instance().PushEntry(std::move(entry)); + PrintColoredMessage(entry); } void LogEntry(Entry& entry) { - auto filter = Impl::Instance().GetGlobalFilter(); - if (!filter.CheckMessage(entry.log_class, entry.log_level)) + if (!filter->CheckMessage(entry.log_class, entry.log_level)) return; - Impl::Instance().PushEntry(std::move(entry)); + PrintColoredMessage(entry); } } // namespace Log diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index 0bf9b8fd7..d5b9834e1 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -6,81 +6,14 @@ #include #include -#include #include #include -#include -#include "common/file_util.h" -#include "common/logging/filter.h" #include "common/logging/log.h" namespace Log { class Filter; -/** - * Interface for logging backends. As loggers can be created and removed at runtime, this can be - * used by a frontend for adding a custom logging backend as needed - */ -class Backend { -public: - virtual ~Backend() = default; - virtual void SetFilter(const Filter& new_filter) { - filter = new_filter; - } - virtual const char* GetName() const = 0; - virtual void Write(const Entry& entry) = 0; - -private: - Filter filter; -}; - -/** - * Backend that writes to stderr without any color commands - */ -class ConsoleBackend : public Backend { -public: - const char* GetName() const override { - return "console"; - } - void Write(const Entry& entry) override; -}; - -/** - * Backend that writes to stderr and with color - */ -class ColorConsoleBackend : public Backend { -public: - const char* GetName() const override { - return "color_console"; - } - void Write(const Entry& entry) override; -}; - -/** - * Backend that writes to a file passed into the constructor. If a log level is error or higher, it - * will flush immediately after writing - */ -class FileBackend : public Backend { -public: - explicit FileBackend(const std::string& filename); - - const char* GetName() const override { - return "file"; - } - - void Write(const Entry& entry) override; - -private: - FileUtil::IOFile file; -}; - -void AddBackend(std::unique_ptr backend); - -void RemoveBackend(const std::string& backend_name); - -boost::optional GetBackend(const std::string& backend_name); - /** * Returns the name of the passed log class as a C-string. Subclasses are separated by periods * instead of underscores as in the enumeration. @@ -92,11 +25,5 @@ const char* GetLogClassName(Class log_class); */ const char* GetLevelName(Level log_level); -/** - * The global filter will prevent any messages from even being processed if they are filtered. Each - * backend can have a filter, but if the level is lower than the global filter, the backend will - * never get the message - */ -void SetGlobalFilter(const Filter& filter); - +void SetFilter(Filter* filter); } // namespace Log From 98e669cf00f5e8ddb8d61a35b1c15170be6cccf3 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Fri, 16 Mar 2018 12:17:14 +0800 Subject: [PATCH 10/18] Remove all edits not in the scope of this PR --- src/citra_qt/CMakeLists.txt | 10 ----- src/citra_qt/configuration/config.cpp | 2 - .../configuration/configure_debug.cpp | 23 ----------- src/citra_qt/configuration/configure_debug.ui | 41 ------------------- src/citra_qt/debugger/console.cpp | 34 --------------- src/citra_qt/debugger/console.h | 14 ------- src/citra_qt/ui_settings.h | 2 - src/common/common_paths.h | 4 -- src/common/file_util.cpp | 17 +++----- src/common/file_util.h | 9 +--- src/common/string_util.cpp | 1 + src/common/threadsafe_queue.h | 32 +-------------- src/core/settings.cpp | 3 +- src/core/settings.h | 3 +- 14 files changed, 13 insertions(+), 182 deletions(-) delete mode 100644 src/citra_qt/debugger/console.cpp delete mode 100644 src/citra_qt/debugger/console.h diff --git a/src/citra_qt/CMakeLists.txt b/src/citra_qt/CMakeLists.txt index a70476998..b0fc1acca 100644 --- a/src/citra_qt/CMakeLists.txt +++ b/src/citra_qt/CMakeLists.txt @@ -28,8 +28,6 @@ add_executable(citra-qt configuration/configure_system.h configuration/configure_web.cpp configuration/configure_web.h - debugger/console.h - debugger/console.cpp debugger/graphics/graphics.cpp debugger/graphics/graphics.h debugger/graphics/graphics_breakpoint_observer.cpp @@ -140,14 +138,6 @@ if (APPLE) target_sources(citra-qt PRIVATE ${MACOSX_ICON}) set_target_properties(citra-qt PROPERTIES MACOSX_BUNDLE TRUE) set_target_properties(citra-qt PROPERTIES MACOSX_BUNDLE_INFO_PLIST ${CMAKE_CURRENT_SOURCE_DIR}/Info.plist) -elseif(WIN32) - # compile as a win32 gui application instead of a console application - target_link_libraries(citra-qt PRIVATE Qt5::WinMain) - if(MSVC) - set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "/SUBSYSTEM:WINDOWS") - elseif(MINGW) - set_target_properties(citra-qt PROPERTIES LINK_FLAGS_RELEASE "-mwindows") - endif() endif() create_target_directory_groups(citra-qt) diff --git a/src/citra_qt/configuration/config.cpp b/src/citra_qt/configuration/config.cpp index c18422e3f..a0dc0cd84 100644 --- a/src/citra_qt/configuration/config.cpp +++ b/src/citra_qt/configuration/config.cpp @@ -224,7 +224,6 @@ void Config::ReadValues() { UISettings::values.confirm_before_closing = qt_config->value("confirmClose", true).toBool(); UISettings::values.first_start = qt_config->value("firstStart", true).toBool(); UISettings::values.callout_flags = qt_config->value("calloutFlags", 0).toUInt(); - UISettings::values.show_console = qt_config->value("showConsole", false).toBool(); qt_config->endGroup(); } @@ -366,7 +365,6 @@ void Config::SaveValues() { qt_config->setValue("confirmClose", UISettings::values.confirm_before_closing); qt_config->setValue("firstStart", UISettings::values.first_start); qt_config->setValue("calloutFlags", UISettings::values.callout_flags); - qt_config->setValue("showConsole", UISettings::values.show_console); qt_config->endGroup(); } diff --git a/src/citra_qt/configuration/configure_debug.cpp b/src/citra_qt/configuration/configure_debug.cpp index b9eb0e3d1..48f57739e 100644 --- a/src/citra_qt/configuration/configure_debug.cpp +++ b/src/citra_qt/configuration/configure_debug.cpp @@ -2,27 +2,13 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. -#include -#include - #include "citra_qt/configuration/configure_debug.h" -#include "citra_qt/debugger/console.h" -#include "citra_qt/ui_settings.h" -#include "common/file_util.h" -#include "common/logging/backend.h" -#include "common/logging/filter.h" -#include "common/logging/log.h" -#include "core/core.h" #include "core/settings.h" #include "ui_configure_debug.h" ConfigureDebug::ConfigureDebug(QWidget* parent) : QWidget(parent), ui(new Ui::ConfigureDebug) { ui->setupUi(this); this->setConfiguration(); - connect(ui->open_log_button, &QPushButton::pressed, []() { - QString path = QString::fromStdString(FileUtil::GetUserPath(D_LOGS_IDX)); - QDesktopServices::openUrl(QUrl::fromLocalFile(path)); - }); } ConfigureDebug::~ConfigureDebug() {} @@ -31,20 +17,11 @@ void ConfigureDebug::setConfiguration() { ui->toggle_gdbstub->setChecked(Settings::values.use_gdbstub); ui->gdbport_spinbox->setEnabled(Settings::values.use_gdbstub); ui->gdbport_spinbox->setValue(Settings::values.gdbstub_port); - ui->toggle_console->setEnabled(!Core::System::GetInstance().IsPoweredOn()); - ui->toggle_console->setChecked(UISettings::values.show_console); - ui->log_filter_edit->setText(QString::fromStdString(Settings::values.log_filter)); } void ConfigureDebug::applyConfiguration() { Settings::values.use_gdbstub = ui->toggle_gdbstub->isChecked(); Settings::values.gdbstub_port = ui->gdbport_spinbox->value(); - UISettings::values.show_console = ui->toggle_console->isChecked(); - Settings::values.log_filter = ui->log_filter_edit->text().toStdString(); - Debugger::ToggleConsole(); - Log::Filter filter; - filter.ParseFilterString(Settings::values.log_filter); - Log::SetFilter(&filter); Settings::Apply(); } diff --git a/src/citra_qt/configuration/configure_debug.ui b/src/citra_qt/configuration/configure_debug.ui index 118e91cf1..a10bea2f4 100644 --- a/src/citra_qt/configuration/configure_debug.ui +++ b/src/citra_qt/configuration/configure_debug.ui @@ -72,47 +72,6 @@ - - - - Logging - - - - - - - - Global Log Filter - - - - - - - - - - - - - - Show Log Console (Windows Only) - - - - - - - Open Log Location - - - - - - - - diff --git a/src/citra_qt/debugger/console.cpp b/src/citra_qt/debugger/console.cpp deleted file mode 100644 index 9d80d108e..000000000 --- a/src/citra_qt/debugger/console.cpp +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2018 Citra Emulator Project -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. - -#ifdef _WIN32 -#include - -#include -#endif - -#include "citra_qt/debugger/console.h" -#include "citra_qt/ui_settings.h" - -namespace Debugger { -void ToggleConsole() { -#ifdef _WIN32 - if (UISettings::values.show_console) { - if (AllocConsole()) { - freopen_s((FILE**)stdin, "CONIN$", "r", stdin); - freopen_s((FILE**)stdout, "CONOUT$", "w", stdout); - freopen_s((FILE**)stderr, "CONOUT$", "w", stderr); - } - } else { - if (FreeConsole()) { - // In order to close the console, we have to also detach the streams on it. - // Just redirect them to NUL if there is no console window - freopen_s((FILE**)stdin, "NUL", "r", stdin); - freopen_s((FILE**)stdout, "NUL", "w", stdout); - freopen_s((FILE**)stderr, "NUL", "w", stderr); - } - } -#endif -} -} // namespace Debugger diff --git a/src/citra_qt/debugger/console.h b/src/citra_qt/debugger/console.h deleted file mode 100644 index 3baf0fdd4..000000000 --- a/src/citra_qt/debugger/console.h +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2018 Citra Emulator Project -// Licensed under GPLv2 or any later version -// Refer to the license.txt file included. - -#pragma once - -namespace Debugger { - -/** - * Uses the WINAPI to hide or show the stderr console. This function is a placeholder until we can - * get a real qt logging window which would work for all platforms. - */ -void ToggleConsole(); -} // namespace Debugger \ No newline at end of file diff --git a/src/citra_qt/ui_settings.h b/src/citra_qt/ui_settings.h index d994cd5c5..caf6aea6a 100644 --- a/src/citra_qt/ui_settings.h +++ b/src/citra_qt/ui_settings.h @@ -56,8 +56,6 @@ struct Values { std::vector shortcuts; uint32_t callout_flags; - - bool show_console; }; extern Values values; diff --git a/src/common/common_paths.h b/src/common/common_paths.h index f6d9ea303..d5b510cdb 100644 --- a/src/common/common_paths.h +++ b/src/common/common_paths.h @@ -36,12 +36,8 @@ #define SDMC_DIR "sdmc" #define NAND_DIR "nand" #define SYSDATA_DIR "sysdata" -#define LOG_DIR "log" // Filenames -// Files in the directory returned by GetUserPath(D_LOGS_IDX) -#define LOG_FILE "citra_log.txt" - // Files in the directory returned by GetUserPath(D_CONFIG_IDX) #define EMU_CONFIG "emu.ini" #define DEBUGGER_CONFIG "debugger.ini" diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 7b256fede..4e1d702f7 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -715,8 +715,6 @@ const std::string& GetUserPath(const unsigned int DirIDX, const std::string& new paths[D_SDMC_IDX] = paths[D_USER_IDX] + SDMC_DIR DIR_SEP; paths[D_NAND_IDX] = paths[D_USER_IDX] + NAND_DIR DIR_SEP; paths[D_SYSDATA_IDX] = paths[D_USER_IDX] + SYSDATA_DIR DIR_SEP; - // TODO: Put the logs in a better location for each OS - paths[D_LOGS_IDX] = paths[D_USER_IDX] + LOG_DIR DIR_SEP; } if (!newPath.empty()) { @@ -803,8 +801,8 @@ void SplitFilename83(const std::string& filename, std::array& short_nam IOFile::IOFile() {} -IOFile::IOFile(const std::string& filename, const char openmode[], int flags) { - Open(filename, openmode, flags); +IOFile::IOFile(const std::string& filename, const char openmode[]) { + Open(filename, openmode); } IOFile::~IOFile() { @@ -825,16 +823,11 @@ void IOFile::Swap(IOFile& other) { std::swap(m_good, other.m_good); } -bool IOFile::Open(const std::string& filename, const char openmode[], int flags) { +bool IOFile::Open(const std::string& filename, const char openmode[]) { Close(); #ifdef _WIN32 - if (flags != 0) { - m_file = _wfsopen(Common::UTF8ToUTF16W(filename).c_str(), - Common::UTF8ToUTF16W(openmode).c_str(), flags); - } else { - _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), - Common::UTF8ToUTF16W(openmode).c_str()); - } + _wfopen_s(&m_file, Common::UTF8ToUTF16W(filename).c_str(), + Common::UTF8ToUTF16W(openmode).c_str()); #else m_file = fopen(filename.c_str(), openmode); #endif diff --git a/src/common/file_util.h b/src/common/file_util.h index 8674ac224..630232a25 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -156,8 +156,7 @@ void SplitFilename83(const std::string& filename, std::array& short_nam class IOFile : public NonCopyable { public: IOFile(); - /// Opens the file. flags is for windows shared file settings and are ignored on other oses - IOFile(const std::string& filename, const char openmode[], int flags = 0); + IOFile(const std::string& filename, const char openmode[]); ~IOFile(); @@ -166,7 +165,7 @@ public: void Swap(IOFile& other); - bool Open(const std::string& filename, const char openmode[], int flags = 0); + bool Open(const std::string& filename, const char openmode[]); bool Close(); template @@ -225,10 +224,6 @@ public: return WriteArray(&object, 1); } - size_t WriteString(const std::string& str) { - return WriteArray(str.c_str(), str.length()); - } - bool IsOpen() const { return nullptr != m_file; } diff --git a/src/common/string_util.cpp b/src/common/string_util.cpp index 0b120e407..124a8937f 100644 --- a/src/common/string_util.cpp +++ b/src/common/string_util.cpp @@ -484,4 +484,5 @@ const char* TrimSourcePath(const char* path, const char* root) { } return path; } + } // namespace Common diff --git a/src/common/threadsafe_queue.h b/src/common/threadsafe_queue.h index 1c676d202..a0c731e8c 100644 --- a/src/common/threadsafe_queue.h +++ b/src/common/threadsafe_queue.h @@ -9,7 +9,6 @@ #include #include -#include #include #include #include "common/common_types.h" @@ -37,10 +36,6 @@ public: T& Front() const { return read_ptr->current; } - - /** - * Push data to the queue. If NeedSize=True then Push will notify the waiting consumer thread - */ template void Push(Arg&& t) { // create the element, add it to the queue @@ -50,11 +45,8 @@ public: ElementPtr* new_ptr = new ElementPtr(); write_ptr->next.store(new_ptr, std::memory_order_release); write_ptr = new_ptr; - if (NeedSize) { - std::lock_guard lock(size_lock); + if (NeedSize) size++; - size_cv.notify_one(); - } } void Pop() { @@ -83,25 +75,6 @@ public: return true; } - /** - * Waits up to timeout for data to be Pushed to the queue. Push uses a condition variable to - * signal the waiting thread, but only if NeedSize = true. Returns false if the timeout is - * triggered. If the condition variable is signalled, returns the value from Pop - * @param T In parameter to store the value if this method returns true - * @param timeout Time in milliseconds to wait for a signal from a Push - */ - bool PopWait(T& t, u64 timeout = 500) { - if (NeedSize) { - std::unique_lock lock(size_lock); - if (size_cv.wait_for(lock, std::chrono::milliseconds(timeout), - [& size = size] { return size > 0; })) { - return Pop(t); - } - return false; - } - return Pop(t); - } - // not thread-safe void Clear() { size.store(0); @@ -129,9 +102,6 @@ private: ElementPtr* write_ptr; ElementPtr* read_ptr; std::atomic size; - - std::mutex size_lock; - std::condition_variable size_cv; }; // a simple thread-safe, diff --git a/src/core/settings.cpp b/src/core/settings.cpp index 770b7fda7..f457c3d9c 100644 --- a/src/core/settings.cpp +++ b/src/core/settings.cpp @@ -4,13 +4,14 @@ #include "audio_core/dsp_interface.h" #include "core/core.h" -#include "core/frontend/emu_window.h" #include "core/gdbstub/gdbstub.h" #include "core/hle/service/hid/hid.h" #include "core/hle/service/ir/ir.h" #include "core/settings.h" #include "video_core/video_core.h" +#include "core/frontend/emu_window.h" + namespace Settings { Values values = {}; diff --git a/src/core/settings.h b/src/core/settings.h index d574a1578..15751720a 100644 --- a/src/core/settings.h +++ b/src/core/settings.h @@ -129,6 +129,8 @@ struct Values { float bg_green; float bg_blue; + std::string log_filter; + // Audio std::string sink_id; bool enable_audio_stretching; @@ -141,7 +143,6 @@ struct Values { // Debugging bool use_gdbstub; u16 gdbstub_port; - std::string log_filter; // Movie std::string movie_play; From 327901e145fbda038477cbd7606b633a88c0e0b9 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Fri, 16 Mar 2018 12:24:16 +0800 Subject: [PATCH 11/18] citra_qt: Remove reference to debugger/console --- src/citra_qt/main.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index c164eeb81..2c54c1539 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -20,7 +20,6 @@ #include "citra_qt/compatdb.h" #include "citra_qt/configuration/config.h" #include "citra_qt/configuration/configure_dialog.h" -#include "citra_qt/debugger/console.h" #include "citra_qt/debugger/graphics/graphics.h" #include "citra_qt/debugger/graphics/graphics_breakpoints.h" #include "citra_qt/debugger/graphics/graphics_cmdlists.h" @@ -35,7 +34,6 @@ #include "citra_qt/main.h" #include "citra_qt/ui_settings.h" #include "citra_qt/updater/updater.h" -#include "common/common_paths.h" #include "common/logging/backend.h" #include "common/logging/filter.h" #include "common/logging/log.h" @@ -371,7 +369,6 @@ void GMainWindow::RestoreUIState() { ui.action_Show_Status_Bar->setChecked(UISettings::values.show_status_bar); statusBar()->setVisible(ui.action_Show_Status_Bar->isChecked()); - Debugger::ToggleConsole(); } void GMainWindow::ConnectWidgetEvents() { @@ -1298,6 +1295,9 @@ void GMainWindow::SyncMenuUISettings() { #endif int main(int argc, char* argv[]) { + Log::Filter log_filter(Log::Level::Info); + Log::SetFilter(&log_filter); + MicroProfileOnThreadCreate("Frontend"); SCOPE_EXIT({ MicroProfileShutdown(); }); @@ -1314,9 +1314,7 @@ int main(int argc, char* argv[]) { GMainWindow main_window; // After settings have been loaded by GMainWindow, apply the filter - Log::Filter log_filter; log_filter.ParseFilterString(Settings::values.log_filter); - Log::SetFilter(&log_filter); main_window.show(); return app.exec(); From c4f98c1a2e78c7e0aeea7d4fb065ac372b0368b9 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Fri, 16 Mar 2018 21:18:45 +0800 Subject: [PATCH 12/18] Logging: Fix clang-format --- src/common/logging/log.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/logging/log.h b/src/common/logging/log.h index ba3831ea6..b272a8611 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -142,9 +142,9 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned /// Logs a message to the global logger, this time with 100% moar fmtlib template void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, - const char* function, const char* format, const Args & ... args) { - Entry entry = - CreateEntry(log_class, log_level, filename, line_num, function, fmt::format(format, args...)); + const char* function, const char* format, const Args&... args) { + Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, + fmt::format(format, args...)); LogEntry(entry); } @@ -195,4 +195,4 @@ void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsig __func__, fmt, ##__VA_ARGS__) #define NGLOG_CRITICAL(log_class, fmt, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__, \ -__func__, fmt, ##__VA_ARGS__) + __func__, fmt, ##__VA_ARGS__) From 2b4998a122de12850509b4c12430266368430a00 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Sat, 17 Mar 2018 22:55:57 +0800 Subject: [PATCH 13/18] Add check if filter is not a nullptr Fix segfault and pass tests --- src/common/logging/backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 3c0361713..0400a3282 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -140,7 +140,7 @@ void SetFilter(Filter* new_filter) { void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, ...) { - if (!filter->CheckMessage(log_class, log_level)) + if (filter != nullptr && !filter->CheckMessage(log_class, log_level)) return; std::array formatting_buffer; va_list args; From ceeb2810fe831fba7ad93d0b99dc4ed10a67ad9d Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Tue, 20 Mar 2018 14:15:48 +0800 Subject: [PATCH 14/18] Add nullptr check to LogEntry Also remove explicit comparison with nullptr to make code shorter. --- src/common/logging/backend.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index 0400a3282..fdd0480c9 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -140,7 +140,7 @@ void SetFilter(Filter* new_filter) { void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, ...) { - if (filter != nullptr && !filter->CheckMessage(log_class, log_level)) + if (filter && !filter->CheckMessage(log_class, log_level)) return; std::array formatting_buffer; va_list args; @@ -154,7 +154,7 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned } void LogEntry(Entry& entry) { - if (!filter->CheckMessage(entry.log_class, entry.log_level)) + if (filter && !filter->CheckMessage(entry.log_class, entry.log_level)) return; PrintColoredMessage(entry); From c892cea02936e509b73ea9ce8ae9302a0bdc2b4b Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Wed, 21 Mar 2018 23:36:45 +0800 Subject: [PATCH 15/18] Place FmtLogMessage's definition in backend.cpp I decided to overload LogMessage because I don't see a reason to come up with a new function name just for this, but if you guys want me to overload FmtLogMessage instead I'm fine with that. --- src/common/logging/backend.cpp | 7 +++++-- src/common/logging/backend.h | 24 ++++++++++++++++++++++ src/common/logging/log.h | 37 +++++----------------------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/common/logging/backend.cpp b/src/common/logging/backend.cpp index fdd0480c9..768f3f22f 100644 --- a/src/common/logging/backend.cpp +++ b/src/common/logging/backend.cpp @@ -153,9 +153,12 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned PrintColoredMessage(entry); } -void LogEntry(Entry& entry) { - if (filter && !filter->CheckMessage(entry.log_class, entry.log_level)) +void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, const char* format, const fmt::format_args& args) { + if (filter && !filter->CheckMessage(log_class, log_level)) return; + Entry entry = + CreateEntry(log_class, log_level, filename, line_num, function, fmt::vformat(format, args)); PrintColoredMessage(entry); } diff --git a/src/common/logging/backend.h b/src/common/logging/backend.h index d5b9834e1..7e81efb23 100644 --- a/src/common/logging/backend.h +++ b/src/common/logging/backend.h @@ -14,6 +14,26 @@ namespace Log { class Filter; +/** + * A log entry. Log entries are store in a structured format to permit more varied output + * formatting on different frontends, as well as facilitating filtering and aggregation. + */ +struct Entry { + std::chrono::microseconds timestamp; + Class log_class; + Level log_level; + std::string filename; + unsigned int line_num; + std::string function; + std::string message; + + Entry() = default; + Entry(Entry&& o) = default; + + Entry& operator=(Entry&& o) = default; + Entry& operator=(const Entry& o) = default; +}; + /** * Returns the name of the passed log class as a C-string. Subclasses are separated by periods * instead of underscores as in the enumeration. @@ -25,5 +45,9 @@ const char* GetLogClassName(Class log_class); */ const char* GetLevelName(Level log_level); +/// Creates a log entry by formatting the given source location, and message. +Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, + const char* function, std::string message); + void SetFilter(Filter* filter); } // namespace Log diff --git a/src/common/logging/log.h b/src/common/logging/log.h index b272a8611..cfaa59487 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -99,33 +99,6 @@ enum class Class : ClassType { Count ///< Total number of logging classes }; -/** - * A log entry. Log entries are store in a structured format to permit more varied output - * formatting on different frontends, as well as facilitating filtering and aggregation. - */ -struct Entry { - std::chrono::microseconds timestamp; - Class log_class; - Level log_level; - std::string filename; - unsigned int line_num; - std::string function; - std::string message; - - Entry() = default; - Entry(Entry&& o) = default; - - Entry& operator=(Entry&& o) = default; - Entry& operator=(const Entry& o) = default; -}; - -/// Creates a log entry by formatting the given source location, and message. -Entry CreateEntry(Class log_class, Level log_level, const char* filename, unsigned int line_nr, - const char* function, std::string message); - -// Logs an Entry -void LogEntry(Entry& entry); - /// Logs a message to the global logger. void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, @@ -139,14 +112,14 @@ void LogMessage(Class log_class, Level log_level, const char* filename, unsigned #endif ; -/// Logs a message to the global logger, this time with 100% moar fmtlib +/// Logs a message to the global logger, using fmt +void LogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, + const char* function, const char* format, const fmt::format_args& args); + template void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsigned int line_num, const char* function, const char* format, const Args&... args) { - Entry entry = CreateEntry(log_class, log_level, filename, line_num, function, - fmt::format(format, args...)); - - LogEntry(entry); + LogMessage(log_class, log_level, filename, line_num, function, format, fmt::make_args(args...)); } } // namespace Log From e6ee1020e16dc00929519743fb71b522a9deb6bf Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Thu, 22 Mar 2018 00:40:01 +0800 Subject: [PATCH 16/18] Replace ##__VA_ARGS__ with __VA_ARGS__ --- src/common/logging/log.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/common/logging/log.h b/src/common/logging/log.h index cfaa59487..66dd736f5 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -147,25 +147,25 @@ void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsig // Define the fmt lib macros #ifdef _DEBUG -#define NGLOG_TRACE(log_class, fmt, ...) \ +#define NGLOG_TRACE(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) + __func__, __VA_ARGS__) #else #define NGLOG_TRACE(log_class, fmt, ...) (void(0)) #endif -#define NGLOG_DEBUG(log_class, fmt, ...) \ +#define NGLOG_DEBUG(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Debug, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) -#define NGLOG_INFO(log_class, fmt, ...) \ + __func__, __VA_ARGS__) +#define NGLOG_INFO(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Info, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) -#define NGLOG_WARNING(log_class, fmt, ...) \ + __func__, __VA_ARGS__) +#define NGLOG_WARNING(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Warning, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) -#define NGLOG_ERROR(log_class, fmt, ...) \ + __func__, __VA_ARGS__) +#define NGLOG_ERROR(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Error, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) -#define NGLOG_CRITICAL(log_class, fmt, ...) \ + __func__, __VA_ARGS__) +#define NGLOG_CRITICAL(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__, \ - __func__, fmt, ##__VA_ARGS__) + __func__, __VA_ARGS__) From 4adbf29b0c72cab8517646fd5303932efcba9fc8 Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Thu, 22 Mar 2018 00:45:01 +0800 Subject: [PATCH 17/18] Fix clang-format Oops forgot to check... --- src/common/logging/log.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/common/logging/log.h b/src/common/logging/log.h index 66dd736f5..de83a9c19 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -147,25 +147,25 @@ void FmtLogMessage(Class log_class, Level log_level, const char* filename, unsig // Define the fmt lib macros #ifdef _DEBUG -#define NGLOG_TRACE(log_class, ...) \ +#define NGLOG_TRACE(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Trace, __FILE__, __LINE__, \ __func__, __VA_ARGS__) #else #define NGLOG_TRACE(log_class, fmt, ...) (void(0)) #endif -#define NGLOG_DEBUG(log_class, ...) \ +#define NGLOG_DEBUG(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Debug, __FILE__, __LINE__, \ __func__, __VA_ARGS__) -#define NGLOG_INFO(log_class, ...) \ +#define NGLOG_INFO(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Info, __FILE__, __LINE__, \ __func__, __VA_ARGS__) -#define NGLOG_WARNING(log_class, ...) \ +#define NGLOG_WARNING(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Warning, __FILE__, __LINE__, \ __func__, __VA_ARGS__) -#define NGLOG_ERROR(log_class, ...) \ +#define NGLOG_ERROR(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Error, __FILE__, __LINE__, \ __func__, __VA_ARGS__) -#define NGLOG_CRITICAL(log_class, ...) \ +#define NGLOG_CRITICAL(log_class, ...) \ ::Log::FmtLogMessage(::Log::Class::log_class, ::Log::Level::Critical, __FILE__, __LINE__, \ __func__, __VA_ARGS__) From 11ce0f91abcaa66bcbdc8423ef322b2fbd614d0b Mon Sep 17 00:00:00 2001 From: Daniel Lim Wee Soong Date: Thu, 22 Mar 2018 21:42:02 +0800 Subject: [PATCH 18/18] Remove dependency chrono Was included last time due to adding Entry into log.h Now it is no longer needed but I forgot to remove it last time --- src/common/logging/log.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/common/logging/log.h b/src/common/logging/log.h index de83a9c19..c72f925f5 100644 --- a/src/common/logging/log.h +++ b/src/common/logging/log.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include "common/common_types.h"