From 50b240fcbd3822e3d7aa9d5fa2c86274393de14a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 28 Jul 2019 22:42:42 -0400 Subject: [PATCH 1/5] VideoCommon/OnScreenDisplay: Default initialize all Message members Provides a deterministic initial state in the case of the default constructor. --- Source/Core/VideoCommon/OnScreenDisplay.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/VideoCommon/OnScreenDisplay.cpp b/Source/Core/VideoCommon/OnScreenDisplay.cpp index a38dc35318..24d8987d99 100644 --- a/Source/Core/VideoCommon/OnScreenDisplay.cpp +++ b/Source/Core/VideoCommon/OnScreenDisplay.cpp @@ -26,14 +26,14 @@ constexpr float WINDOW_PADDING = 4.0f; // Pixels between subsequent OSD message struct Message { - Message() {} + Message() = default; Message(const std::string& text_, u32 timestamp_, u32 color_) : text(text_), timestamp(timestamp_), color(color_) { } std::string text; - u32 timestamp; - u32 color; + u32 timestamp = 0; + u32 color = 0; }; static std::multimap s_messages; static std::mutex s_messages_mutex; From c212310fbe0b205b62c4d3ac997060cf4fc26e2f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 28 Jul 2019 22:46:08 -0400 Subject: [PATCH 2/5] VideoCommon/OnScreenDisplay: Take Message's std::string parameter by value Allows callers to std::move strings into the functions (or automatically assume the move constructor/move assignment operator for rvalue references, potentially avoiding copies altogether. --- Source/Core/Core/Core.cpp | 4 ++-- Source/Core/Core/Core.h | 2 +- Source/Core/Core/DSP/DSPHost.h | 2 +- Source/Core/Core/HW/DSPLLE/DSPHost.cpp | 4 ++-- Source/Core/VideoCommon/OnScreenDisplay.cpp | 12 ++++++------ Source/Core/VideoCommon/OnScreenDisplay.h | 9 +++++---- Source/DSPTool/DSPTool.cpp | 2 +- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index f1e3161220..5398d8347b 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -150,7 +150,7 @@ std::string StopMessage(bool main_thread, const std::string& message) Common::CurrentThreadId(), message.c_str()); } -void DisplayMessage(const std::string& message, int time_in_ms) +void DisplayMessage(std::string message, int time_in_ms) { if (!IsRunning()) return; @@ -162,8 +162,8 @@ void DisplayMessage(const std::string& message, int time_in_ms) return; } - OSD::AddMessage(message, time_in_ms); Host_UpdateTitle(message); + OSD::AddMessage(std::move(message), time_in_ms); } bool IsRunning() diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index 26e30a2d8a..f76823d0e6 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -63,7 +63,7 @@ void SaveScreenShot(const std::string& name, bool wait_for_completion = false); void Callback_WiimoteInterruptChannel(int number, u16 channel_id, const u8* data, u32 size); // This displays messages in a user-visible way. -void DisplayMessage(const std::string& message, int time_in_ms); +void DisplayMessage(std::string message, int time_in_ms); void FrameUpdateOnCPUThread(); void OnFrameEnd(); diff --git a/Source/Core/Core/DSP/DSPHost.h b/Source/Core/Core/DSP/DSPHost.h index b0a424806a..d771fd9b48 100644 --- a/Source/Core/Core/DSP/DSPHost.h +++ b/Source/Core/Core/DSP/DSPHost.h @@ -17,7 +17,7 @@ namespace DSP::Host { u8 ReadHostMemory(u32 addr); void WriteHostMemory(u8 value, u32 addr); -void OSD_AddMessage(const std::string& str, u32 ms); +void OSD_AddMessage(std::string str, u32 ms); bool OnThread(); bool IsWiiHost(); void InterruptRequest(); diff --git a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp index b8f42f7e41..ab88e457d2 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp @@ -36,9 +36,9 @@ void WriteHostMemory(u8 value, u32 addr) DSP::WriteARAM(value, addr); } -void OSD_AddMessage(const std::string& str, u32 ms) +void OSD_AddMessage(std::string str, u32 ms) { - OSD::AddMessage(str, ms); + OSD::AddMessage(std::move(str), ms); } bool OnThread() diff --git a/Source/Core/VideoCommon/OnScreenDisplay.cpp b/Source/Core/VideoCommon/OnScreenDisplay.cpp index 24d8987d99..b52de17cbf 100644 --- a/Source/Core/VideoCommon/OnScreenDisplay.cpp +++ b/Source/Core/VideoCommon/OnScreenDisplay.cpp @@ -27,8 +27,8 @@ constexpr float WINDOW_PADDING = 4.0f; // Pixels between subsequent OSD message struct Message { Message() = default; - Message(const std::string& text_, u32 timestamp_, u32 color_) - : text(text_), timestamp(timestamp_), color(color_) + Message(std::string text_, u32 timestamp_, u32 color_) + : text(std::move(text_)), timestamp(timestamp_), color(color_) { } std::string text; @@ -79,18 +79,18 @@ static float DrawMessage(int index, const Message& msg, const ImVec2& position, return window_height; } -void AddTypedMessage(MessageType type, const std::string& message, u32 ms, u32 rgba) +void AddTypedMessage(MessageType type, std::string message, u32 ms, u32 rgba) { std::lock_guard lock(s_messages_mutex); s_messages.erase(type); - s_messages.emplace(type, Message(message, Common::Timer::GetTimeMs() + ms, rgba)); + s_messages.emplace(type, Message(std::move(message), Common::Timer::GetTimeMs() + ms, rgba)); } -void AddMessage(const std::string& message, u32 ms, u32 rgba) +void AddMessage(std::string message, u32 ms, u32 rgba) { std::lock_guard lock(s_messages_mutex); s_messages.emplace(MessageType::Typeless, - Message(message, Common::Timer::GetTimeMs() + ms, rgba)); + Message(std::move(message), Common::Timer::GetTimeMs() + ms, rgba)); } void DrawMessages() diff --git a/Source/Core/VideoCommon/OnScreenDisplay.h b/Source/Core/VideoCommon/OnScreenDisplay.h index 20d563d7ba..7179b9f4c2 100644 --- a/Source/Core/VideoCommon/OnScreenDisplay.h +++ b/Source/Core/VideoCommon/OnScreenDisplay.h @@ -37,10 +37,11 @@ constexpr u32 VERY_LONG = 10000; }; // namespace Duration // On-screen message display (colored yellow by default) -void AddMessage(const std::string& message, u32 ms = Duration::SHORT, u32 rgba = Color::YELLOW); -void AddTypedMessage(MessageType type, const std::string& message, u32 ms = Duration::SHORT, +void AddMessage(std::string message, u32 ms = Duration::SHORT, u32 rgba = Color::YELLOW); +void AddTypedMessage(MessageType type, std::string message, u32 ms = Duration::SHORT, u32 rgba = Color::YELLOW); -void DrawMessages(); // draw the current messages on the screen. Only call once - // per frame. + +// Draw the current messages on the screen. Only call once per frame. +void DrawMessages(); void ClearMessages(); } // namespace OSD diff --git a/Source/DSPTool/DSPTool.cpp b/Source/DSPTool/DSPTool.cpp index f1434fd03e..bd3335189d 100644 --- a/Source/DSPTool/DSPTool.cpp +++ b/Source/DSPTool/DSPTool.cpp @@ -22,7 +22,7 @@ u8 DSP::Host::ReadHostMemory(u32 addr) void DSP::Host::WriteHostMemory(u8 value, u32 addr) { } -void DSP::Host::OSD_AddMessage(const std::string& str, u32 ms) +void DSP::Host::OSD_AddMessage(std::string str, u32 ms) { } bool DSP::Host::OnThread() From a565e41cb8d1c695581482ad4cb1cb580b084f34 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 28 Jul 2019 23:04:50 -0400 Subject: [PATCH 3/5] VideoCommon/OnScreenDisplay: Remove unused headers While we're at it, fix up the imgui include to use the convention we use for referencing external library headers. --- Source/Core/VideoCommon/OnScreenDisplay.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/Core/VideoCommon/OnScreenDisplay.cpp b/Source/Core/VideoCommon/OnScreenDisplay.cpp index b52de17cbf..8868585120 100644 --- a/Source/Core/VideoCommon/OnScreenDisplay.cpp +++ b/Source/Core/VideoCommon/OnScreenDisplay.cpp @@ -3,12 +3,11 @@ // Refer to the license.txt file included. #include -#include #include #include #include -#include "imgui.h" +#include #include "Common/CommonTypes.h" #include "Common/StringUtil.h" From 3f947f086fdc29765594bcaea8ed7fb528e8edbc Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 28 Jul 2019 23:08:18 -0400 Subject: [PATCH 4/5] VideoCommon/OnScreenDisplay: Use deduction guides for std::lock_guard Same behavior without hardcoding the type of the mutex within the lock guards. This means the type of the mutex would be able to be changed without needing to also change all occurrences lock guards are used. --- Source/Core/VideoCommon/OnScreenDisplay.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Core/VideoCommon/OnScreenDisplay.cpp b/Source/Core/VideoCommon/OnScreenDisplay.cpp index 8868585120..0fb4a55a18 100644 --- a/Source/Core/VideoCommon/OnScreenDisplay.cpp +++ b/Source/Core/VideoCommon/OnScreenDisplay.cpp @@ -80,14 +80,14 @@ static float DrawMessage(int index, const Message& msg, const ImVec2& position, void AddTypedMessage(MessageType type, std::string message, u32 ms, u32 rgba) { - std::lock_guard lock(s_messages_mutex); + std::lock_guard lock{s_messages_mutex}; s_messages.erase(type); s_messages.emplace(type, Message(std::move(message), Common::Timer::GetTimeMs() + ms, rgba)); } void AddMessage(std::string message, u32 ms, u32 rgba) { - std::lock_guard lock(s_messages_mutex); + std::lock_guard lock{s_messages_mutex}; s_messages.emplace(MessageType::Typeless, Message(std::move(message), Common::Timer::GetTimeMs() + ms, rgba)); } @@ -98,7 +98,7 @@ void DrawMessages() return; { - std::lock_guard lock(s_messages_mutex); + std::lock_guard lock{s_messages_mutex}; const u32 now = Common::Timer::GetTimeMs(); float current_x = LEFT_MARGIN * ImGui::GetIO().DisplayFramebufferScale.x; @@ -122,7 +122,7 @@ void DrawMessages() void ClearMessages() { - std::lock_guard lock(s_messages_mutex); + std::lock_guard lock{s_messages_mutex}; s_messages.clear(); } } // namespace OSD From c93fffaed62b6c64bfd6ce306aa85765e9df961f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 28 Jul 2019 23:16:20 -0400 Subject: [PATCH 5/5] DolphinQt/HotkeyScheduler: Correct string within Run() AddMessage() by itself doesn't perform string formatting facilities, so this message was actually using the EFB scale as a duration value, not a format argument. This corrects that. --- Source/Core/DolphinQt/HotkeyScheduler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/HotkeyScheduler.cpp b/Source/Core/DolphinQt/HotkeyScheduler.cpp index e43d66c803..1fa4919ec7 100644 --- a/Source/Core/DolphinQt/HotkeyScheduler.cpp +++ b/Source/Core/DolphinQt/HotkeyScheduler.cpp @@ -337,7 +337,7 @@ void HotkeyScheduler::Run() OSD::AddMessage("Internal Resolution: Native"); break; default: - OSD::AddMessage("Internal Resolution: %dx", g_Config.iEFBScale); + OSD::AddMessage(StringFromFormat("Internal Resolution: %dx", g_Config.iEFBScale)); break; } };