From 6d02bb730457f70b6afe770ee679c35b2f661c1e Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Tue, 28 Nov 2023 11:06:35 +0100 Subject: [PATCH 1/7] Make emote completion a lot smarter (#4987) --- CHANGELOG.md | 1 + src/CMakeLists.txt | 4 +- .../completion/TabCompletionModel.cpp | 8 + .../strategies/SmartEmoteStrategy.cpp | 204 ++++++++++++++++++ .../strategies/SmartEmoteStrategy.hpp | 22 ++ src/singletons/Settings.hpp | 4 + src/widgets/settingspages/GeneralPage.cpp | 2 + src/widgets/splits/InputCompletionPopup.cpp | 9 + 8 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 src/controllers/completion/strategies/SmartEmoteStrategy.cpp create mode 100644 src/controllers/completion/strategies/SmartEmoteStrategy.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 57fd2f21619..845d117b446 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Minor: Add menu actions to reply directly to a message or the original thread root. (#4923) - Minor: The `/reply` command now replies to the latest message of the user. (#4919) - Minor: All sound capabilities can now be disabled by setting your "Sound backend" setting to "Null" and restarting Chatterino. (#4978) +- Minor: Add an option to use new experimental smarter emote completion. (#4987) - Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840) - Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848) - Bugfix: Trimmed custom streamlink paths on all platforms making sure you don't accidentally add spaces at the beginning or end of its path. (#4834) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9b75d7e9022..86c24c15b40 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -123,13 +123,15 @@ set(SOURCE_FILES controllers/completion/sources/UnifiedSource.hpp controllers/completion/sources/UserSource.cpp controllers/completion/sources/UserSource.hpp - controllers/completion/strategies/Strategy.hpp controllers/completion/strategies/ClassicEmoteStrategy.cpp controllers/completion/strategies/ClassicEmoteStrategy.hpp controllers/completion/strategies/ClassicUserStrategy.cpp controllers/completion/strategies/ClassicUserStrategy.hpp controllers/completion/strategies/CommandStrategy.cpp controllers/completion/strategies/CommandStrategy.hpp + controllers/completion/strategies/SmartEmoteStrategy.cpp + controllers/completion/strategies/SmartEmoteStrategy.cpp + controllers/completion/strategies/Strategy.hpp controllers/completion/TabCompletionModel.cpp controllers/completion/TabCompletionModel.hpp diff --git a/src/controllers/completion/TabCompletionModel.cpp b/src/controllers/completion/TabCompletionModel.cpp index 159e89f3d33..74e82652900 100644 --- a/src/controllers/completion/TabCompletionModel.cpp +++ b/src/controllers/completion/TabCompletionModel.cpp @@ -8,6 +8,7 @@ #include "controllers/completion/strategies/ClassicEmoteStrategy.hpp" #include "controllers/completion/strategies/ClassicUserStrategy.hpp" #include "controllers/completion/strategies/CommandStrategy.hpp" +#include "controllers/completion/strategies/SmartEmoteStrategy.hpp" #include "singletons/Settings.hpp" namespace chatterino { @@ -123,6 +124,13 @@ std::unique_ptr TabCompletionModel::buildSource( std::unique_ptr TabCompletionModel::buildEmoteSource() const { + if (getSettings()->useSmartEmoteCompletion) + { + return std::make_unique( + &this->channel_, + std::make_unique()); + } + return std::make_unique( &this->channel_, std::make_unique()); diff --git a/src/controllers/completion/strategies/SmartEmoteStrategy.cpp b/src/controllers/completion/strategies/SmartEmoteStrategy.cpp new file mode 100644 index 00000000000..aa6e43127ef --- /dev/null +++ b/src/controllers/completion/strategies/SmartEmoteStrategy.cpp @@ -0,0 +1,204 @@ +#include "controllers/completion/strategies/SmartEmoteStrategy.hpp" + +#include "common/QLogging.hpp" +#include "controllers/completion/sources/EmoteSource.hpp" +#include "singletons/Settings.hpp" +#include "util/Helpers.hpp" + +#include + +#include + +namespace chatterino::completion { +namespace { + /** + * @brief This function calculates the "cost" of the changes that need to + * be done to the query to make it the value. + * + * By default an emote with more differences in character casing from the + * query will get a higher cost, each additional letter also increases cost. + * + * @param prioritizeUpper If set, then differences in casing don't matter, but + * instead the more lowercase letters an emote contains, the higher cost it + * will get. Additional letters also increase the cost in this mode. + * + * @return How different the emote is from query. Values in the range [-10, + * \infty]. + */ + int costOfEmote(const QString &query, const QString &emote, + bool prioritizeUpper) + { + int score = 0; + + if (prioritizeUpper) + { + // We are in case 3, push 'more uppercase' emotes to the top + for (const auto i : emote) + { + score += int(!i.isUpper()); + } + } + else + { + // Push more matching emotes to the top + int len = std::min(emote.size(), query.size()); + for (int i = 0; i < len; i++) + { + // Different casing gets a higher cost score + score += query.at(i).isUpper() ^ emote.at(i).isUpper(); + } + } + // No case differences, put this at the top + if (score == 0) + { + score = -10; + } + + auto diff = emote.size() - query.size(); + if (diff > 0) + { + // Case changes are way less changes to the user compared to adding characters + score += diff * 100; + } + return score; + }; + + // This contains the brains of emote tab completion. Updates output to sorted completions. + // Ensure that the query string is already normalized, that is doesn't have a leading ':' + // matchingFunction is used for testing if the emote should be included in the search. + void completeEmotes( + const std::vector &items, std::vector &output, + const QString &query, bool ignoreColonForCost, + const std::function + &matchingFunction) + { + // Given these emotes: pajaW, PAJAW + // There are a few cases of input: + // 1. "pajaw" expect {pajaW, PAJAW} - no uppercase characters, do regular case insensitive search + // 2. "PA" expect {PAJAW} - uppercase characters, case sensitive search gives results + // 3. "Pajaw" expect {PAJAW, pajaW} - case sensitive search doesn't give results, need to use sorting + // 4. "NOTHING" expect {} - no results + // 5. "nothing" expect {} - same as 4 but first search is case insensitive + + // Check if the query contains any uppercase characters + // This tells us if we're in case 1 or 5 vs all others + bool haveUpper = + std::any_of(query.begin(), query.end(), [](const QChar &c) { + return c.isUpper(); + }); + + // First search, for case 1 it will be case insensitive, + // for cases 2, 3 and 4 it will be case sensitive + for (const auto &item : items) + { + if (matchingFunction( + item, query, + haveUpper ? Qt::CaseSensitive : Qt::CaseInsensitive)) + { + output.push_back(item); + } + } + + // if case 3: then true; false otherwise + bool prioritizeUpper = false; + + // No results from search + if (output.empty()) + { + if (!haveUpper) + { + // Optimisation: First search was case insensitive, but we found nothing + // There is nothing to be found: case 5. + return; + } + // Case sensitive search from case 2 found nothing, therefore we can + // only be in case 3 or 4. + + prioritizeUpper = true; + // Run the search again but this time without case sensitivity + for (const auto &item : items) + { + if (matchingFunction(item, query, Qt::CaseInsensitive)) + { + output.push_back(item); + } + } + if (output.empty()) + { + // The second search found nothing, so don't even try to sort: case 4 + return; + } + } + + std::sort(output.begin(), output.end(), + [query, prioritizeUpper, ignoreColonForCost]( + const EmoteItem &a, const EmoteItem &b) -> bool { + auto tempA = a.searchName; + auto tempB = b.searchName; + if (ignoreColonForCost && tempA.startsWith(":")) + { + tempA = tempA.mid(1); + } + if (ignoreColonForCost && tempB.startsWith(":")) + { + tempB = tempB.mid(1); + } + + auto costA = costOfEmote(query, tempA, prioritizeUpper); + auto costB = costOfEmote(query, tempB, prioritizeUpper); + if (costA == costB) + { + // Case difference and length came up tied for (a, b), break the tie + return QString::compare(tempA, tempB, + Qt::CaseInsensitive) < 0; + } + + return costA < costB; + }); + } +} // namespace + +void SmartEmoteStrategy::apply(const std::vector &items, + std::vector &output, + const QString &query) const +{ + QString normalizedQuery = query; + bool ignoreColonForCost = false; + if (normalizedQuery.startsWith(':')) + { + normalizedQuery = normalizedQuery.mid(1); + ignoreColonForCost = true; + } + completeEmotes(items, output, normalizedQuery, ignoreColonForCost, + [](const EmoteItem &left, const QString &right, + Qt::CaseSensitivity caseHandling) { + return left.searchName.contains(right, caseHandling); + }); +} + +void SmartTabEmoteStrategy::apply(const std::vector &items, + std::vector &output, + const QString &query) const +{ + bool emojiOnly = false; + QString normalizedQuery = query; + if (normalizedQuery.startsWith(':')) + { + normalizedQuery = normalizedQuery.mid(1); + // tab completion with : prefix should do emojis only + emojiOnly = true; + } + completeEmotes(items, output, normalizedQuery, false, + [emojiOnly](const EmoteItem &left, const QString &right, + Qt::CaseSensitivity caseHandling) -> bool { + if (emojiOnly ^ left.isEmoji) + { + return false; + } + return startsWithOrContains( + left.searchName, right, caseHandling, + getSettings()->prefixOnlyEmoteCompletion); + }); +} + +} // namespace chatterino::completion diff --git a/src/controllers/completion/strategies/SmartEmoteStrategy.hpp b/src/controllers/completion/strategies/SmartEmoteStrategy.hpp new file mode 100644 index 00000000000..365e106b033 --- /dev/null +++ b/src/controllers/completion/strategies/SmartEmoteStrategy.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include "controllers/completion/sources/EmoteSource.hpp" +#include "controllers/completion/strategies/Strategy.hpp" + +namespace chatterino::completion { + +class SmartEmoteStrategy : public Strategy +{ + void apply(const std::vector &items, + std::vector &output, + const QString &query) const override; +}; + +class SmartTabEmoteStrategy : public Strategy +{ + void apply(const std::vector &items, + std::vector &output, + const QString &query) const override; +}; + +} // namespace chatterino::completion diff --git a/src/singletons/Settings.hpp b/src/singletons/Settings.hpp index 40d7be51e8e..8e650de873d 100644 --- a/src/singletons/Settings.hpp +++ b/src/singletons/Settings.hpp @@ -218,6 +218,10 @@ class Settings "/behaviour/autocompletion/emoteCompletionWithColon", true}; BoolSetting showUsernameCompletionMenu = { "/behaviour/autocompletion/showUsernameCompletionMenu", true}; + BoolSetting useSmartEmoteCompletion = { + "/experiments/useSmartEmoteCompletion", + false, + }; FloatSetting pauseOnHoverDuration = {"/behaviour/pauseOnHoverDuration", 0}; EnumSetting pauseChatModifier = { diff --git a/src/widgets/settingspages/GeneralPage.cpp b/src/widgets/settingspages/GeneralPage.cpp index 48ecdb486fd..d2b5c990892 100644 --- a/src/widgets/settingspages/GeneralPage.cpp +++ b/src/widgets/settingspages/GeneralPage.cpp @@ -484,6 +484,8 @@ void GeneralPage::initLayout(GeneralPageView &layout) "cvMask and 7TV's RainTime, will appear as normal emotes."); layout.addCheckbox("Enable emote auto-completion by typing :", s.emoteCompletionWithColon); + layout.addCheckbox("Use experimental smarter emote completion.", + s.useSmartEmoteCompletion); layout.addDropdown( "Size", {"0.5x", "0.75x", "Default", "1.25x", "1.5x", "2x"}, s.emoteScale, diff --git a/src/widgets/splits/InputCompletionPopup.cpp b/src/widgets/splits/InputCompletionPopup.cpp index 2828551ea0b..c103774db82 100644 --- a/src/widgets/splits/InputCompletionPopup.cpp +++ b/src/widgets/splits/InputCompletionPopup.cpp @@ -3,6 +3,8 @@ #include "controllers/completion/sources/UserSource.hpp" #include "controllers/completion/strategies/ClassicEmoteStrategy.hpp" #include "controllers/completion/strategies/ClassicUserStrategy.hpp" +#include "controllers/completion/strategies/SmartEmoteStrategy.hpp" +#include "singletons/Settings.hpp" #include "singletons/Theme.hpp" #include "util/LayoutCreator.hpp" #include "widgets/splits/InputCompletionItem.hpp" @@ -60,6 +62,13 @@ std::unique_ptr InputCompletionPopup::getSource() const switch (*this->currentKind_) { case CompletionKind::Emote: + if (getSettings()->useSmartEmoteCompletion) + { + return std::make_unique( + this->currentChannel_.get(), + std::make_unique(), + this->callback_); + } return std::make_unique( this->currentChannel_.get(), std::make_unique(), From e327ed416685c4de067dbf7e4498e63a8c67a447 Mon Sep 17 00:00:00 2001 From: pajlada Date: Fri, 1 Dec 2023 15:03:04 +0100 Subject: [PATCH 2/7] Update magic_enum from v0.9.3 to v0.9.5 (#4992) * Fix include path for magic enum * Update .clang-format to ensure magic enum is caught as a third party library --- CHANGELOG.md | 1 + benchmarks/.clang-format | 9 +++------ cmake/FindMagicEnum.cmake | 2 +- lib/magic_enum | 2 +- mocks/.clang-format | 9 +++------ src/.clang-format | 9 +++------ src/common/ChatterinoSetting.hpp | 2 +- src/controllers/plugins/LuaUtilities.hpp | 2 +- src/controllers/plugins/Plugin.cpp | 2 +- src/providers/seventv/SeventvCosmetics.hpp | 2 +- src/providers/seventv/eventapi/Message.hpp | 2 +- src/providers/seventv/eventapi/Subscription.hpp | 2 +- src/providers/twitch/api/Helix.cpp | 2 +- src/providers/twitch/pubsubmessages/AutoMod.hpp | 2 +- src/providers/twitch/pubsubmessages/Base.hpp | 2 +- src/providers/twitch/pubsubmessages/ChannelPoints.hpp | 2 +- .../twitch/pubsubmessages/ChatModeratorAction.hpp | 2 +- src/providers/twitch/pubsubmessages/Whisper.hpp | 2 +- src/widgets/settingspages/GeneralPage.cpp | 2 +- tests/.clang-format | 9 +++------ 20 files changed, 28 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 845d117b446..5cf1e866223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ - Dev: Add a compile-time flag `CHATTERINO_UPDATER` which can be turned off to disable update checks. (#4854) - Dev: Add a compile-time flag `USE_SYSTEM_MINIAUDIO` which can be turned on to use the system miniaudio. (#4867) - Dev: Update vcpkg to use Qt6. (#4872) +- Dev: Update `magic_enum` to v0.9.5. (#4992) - Dev: Replace `boost::optional` with `std::optional`. (#4877) - Dev: Improve performance of selecting text. (#4889, #4911) - Dev: Removed direct dependency on Qt 5 compatibility module. (#4906) diff --git a/benchmarks/.clang-format b/benchmarks/.clang-format index 7bae09f2ce3..0feaad9dc10 100644 --- a/benchmarks/.clang-format +++ b/benchmarks/.clang-format @@ -32,9 +32,6 @@ IncludeCategories: # Project includes - Regex: '^"[a-zA-Z\._-]+(/[a-zA-Z0-9\._-]+)*"$' Priority: 1 - # Third party library includes - - Regex: '<[[:alnum:].]+/[a-zA-Z0-9\._\/-]+>' - Priority: 3 # Qt includes - Regex: '^$' Priority: 3 @@ -42,12 +39,12 @@ IncludeCategories: # LibCommuni includes - Regex: "^$" Priority: 3 - # Misc libraries - - Regex: '^<[a-zA-Z_0-9]+\.h(pp)?>$' - Priority: 3 # Standard library includes - Regex: "^<[a-zA-Z_]+>$" Priority: 4 + # Third party library includes + - Regex: "^<([a-zA-Z_0-9-]+/)*[a-zA-Z_0-9-]+.h(pp)?>$" + Priority: 3 NamespaceIndentation: Inner PointerBindsToType: false SpacesBeforeTrailingComments: 2 diff --git a/cmake/FindMagicEnum.cmake b/cmake/FindMagicEnum.cmake index 0a77bd279fc..b595075cab5 100644 --- a/cmake/FindMagicEnum.cmake +++ b/cmake/FindMagicEnum.cmake @@ -1,6 +1,6 @@ include(FindPackageHandleStandardArgs) -find_path(MagicEnum_INCLUDE_DIR magic_enum.hpp HINTS ${CMAKE_SOURCE_DIR}/lib/magic_enum/include) +find_path(MagicEnum_INCLUDE_DIR magic_enum/magic_enum.hpp HINTS ${CMAKE_SOURCE_DIR}/lib/magic_enum/include) find_package_handle_standard_args(MagicEnum DEFAULT_MSG MagicEnum_INCLUDE_DIR) diff --git a/lib/magic_enum b/lib/magic_enum index e1a68e9dd3d..e55b9b54d5c 160000 --- a/lib/magic_enum +++ b/lib/magic_enum @@ -1 +1 @@ -Subproject commit e1a68e9dd3d2e9180b04c8aeacd4975db745e6b8 +Subproject commit e55b9b54d5cf61f8e117cafb17846d7d742dd3b4 diff --git a/mocks/.clang-format b/mocks/.clang-format index 7bae09f2ce3..0feaad9dc10 100644 --- a/mocks/.clang-format +++ b/mocks/.clang-format @@ -32,9 +32,6 @@ IncludeCategories: # Project includes - Regex: '^"[a-zA-Z\._-]+(/[a-zA-Z0-9\._-]+)*"$' Priority: 1 - # Third party library includes - - Regex: '<[[:alnum:].]+/[a-zA-Z0-9\._\/-]+>' - Priority: 3 # Qt includes - Regex: '^$' Priority: 3 @@ -42,12 +39,12 @@ IncludeCategories: # LibCommuni includes - Regex: "^$" Priority: 3 - # Misc libraries - - Regex: '^<[a-zA-Z_0-9]+\.h(pp)?>$' - Priority: 3 # Standard library includes - Regex: "^<[a-zA-Z_]+>$" Priority: 4 + # Third party library includes + - Regex: "^<([a-zA-Z_0-9-]+/)*[a-zA-Z_0-9-]+.h(pp)?>$" + Priority: 3 NamespaceIndentation: Inner PointerBindsToType: false SpacesBeforeTrailingComments: 2 diff --git a/src/.clang-format b/src/.clang-format index 7bae09f2ce3..0feaad9dc10 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -32,9 +32,6 @@ IncludeCategories: # Project includes - Regex: '^"[a-zA-Z\._-]+(/[a-zA-Z0-9\._-]+)*"$' Priority: 1 - # Third party library includes - - Regex: '<[[:alnum:].]+/[a-zA-Z0-9\._\/-]+>' - Priority: 3 # Qt includes - Regex: '^$' Priority: 3 @@ -42,12 +39,12 @@ IncludeCategories: # LibCommuni includes - Regex: "^$" Priority: 3 - # Misc libraries - - Regex: '^<[a-zA-Z_0-9]+\.h(pp)?>$' - Priority: 3 # Standard library includes - Regex: "^<[a-zA-Z_]+>$" Priority: 4 + # Third party library includes + - Regex: "^<([a-zA-Z_0-9-]+/)*[a-zA-Z_0-9-]+.h(pp)?>$" + Priority: 3 NamespaceIndentation: Inner PointerBindsToType: false SpacesBeforeTrailingComments: 2 diff --git a/src/common/ChatterinoSetting.hpp b/src/common/ChatterinoSetting.hpp index 6c9d8ec4728..2f5a0cac436 100644 --- a/src/common/ChatterinoSetting.hpp +++ b/src/common/ChatterinoSetting.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include diff --git a/src/controllers/plugins/LuaUtilities.hpp b/src/controllers/plugins/LuaUtilities.hpp index 6a75b774a35..f88f2a92836 100644 --- a/src/controllers/plugins/LuaUtilities.hpp +++ b/src/controllers/plugins/LuaUtilities.hpp @@ -4,7 +4,7 @@ # include # include -# include +# include # include # include diff --git a/src/controllers/plugins/Plugin.cpp b/src/controllers/plugins/Plugin.cpp index 3fdc8e4dca8..0e9a20d631f 100644 --- a/src/controllers/plugins/Plugin.cpp +++ b/src/controllers/plugins/Plugin.cpp @@ -4,7 +4,7 @@ # include "controllers/commands/CommandController.hpp" # include -# include +# include # include # include diff --git a/src/providers/seventv/SeventvCosmetics.hpp b/src/providers/seventv/SeventvCosmetics.hpp index 0d521ac03a8..6302c51f45d 100644 --- a/src/providers/seventv/SeventvCosmetics.hpp +++ b/src/providers/seventv/SeventvCosmetics.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include namespace chatterino::seventv { diff --git a/src/providers/seventv/eventapi/Message.hpp b/src/providers/seventv/eventapi/Message.hpp index 5dbc848b756..4227f683990 100644 --- a/src/providers/seventv/eventapi/Message.hpp +++ b/src/providers/seventv/eventapi/Message.hpp @@ -2,7 +2,7 @@ #include "providers/seventv/eventapi/Subscription.hpp" -#include +#include #include #include #include diff --git a/src/providers/seventv/eventapi/Subscription.hpp b/src/providers/seventv/eventapi/Subscription.hpp index 1a36811a56b..65cf0354443 100644 --- a/src/providers/seventv/eventapi/Subscription.hpp +++ b/src/providers/seventv/eventapi/Subscription.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/providers/twitch/api/Helix.cpp b/src/providers/twitch/api/Helix.cpp index 758efe9ff12..8e4c73fc5f3 100644 --- a/src/providers/twitch/api/Helix.cpp +++ b/src/providers/twitch/api/Helix.cpp @@ -6,7 +6,7 @@ #include "common/QLogging.hpp" #include "util/CancellationToken.hpp" -#include +#include #include namespace { diff --git a/src/providers/twitch/pubsubmessages/AutoMod.hpp b/src/providers/twitch/pubsubmessages/AutoMod.hpp index 400169ac7be..9f40d39da67 100644 --- a/src/providers/twitch/pubsubmessages/AutoMod.hpp +++ b/src/providers/twitch/pubsubmessages/AutoMod.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/providers/twitch/pubsubmessages/Base.hpp b/src/providers/twitch/pubsubmessages/Base.hpp index ce190ee065d..fed0112e7e0 100644 --- a/src/providers/twitch/pubsubmessages/Base.hpp +++ b/src/providers/twitch/pubsubmessages/Base.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/providers/twitch/pubsubmessages/ChannelPoints.hpp b/src/providers/twitch/pubsubmessages/ChannelPoints.hpp index c5a3ffe88d5..be8d1bd68af 100644 --- a/src/providers/twitch/pubsubmessages/ChannelPoints.hpp +++ b/src/providers/twitch/pubsubmessages/ChannelPoints.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include diff --git a/src/providers/twitch/pubsubmessages/ChatModeratorAction.hpp b/src/providers/twitch/pubsubmessages/ChatModeratorAction.hpp index 5f29673e3d3..e04019cb7cf 100644 --- a/src/providers/twitch/pubsubmessages/ChatModeratorAction.hpp +++ b/src/providers/twitch/pubsubmessages/ChatModeratorAction.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include diff --git a/src/providers/twitch/pubsubmessages/Whisper.hpp b/src/providers/twitch/pubsubmessages/Whisper.hpp index ef96cd62b35..979cb6a1ebf 100644 --- a/src/providers/twitch/pubsubmessages/Whisper.hpp +++ b/src/providers/twitch/pubsubmessages/Whisper.hpp @@ -1,6 +1,6 @@ #pragma once -#include +#include #include #include #include diff --git a/src/widgets/settingspages/GeneralPage.cpp b/src/widgets/settingspages/GeneralPage.cpp index d2b5c990892..3382199763b 100644 --- a/src/widgets/settingspages/GeneralPage.cpp +++ b/src/widgets/settingspages/GeneralPage.cpp @@ -21,7 +21,7 @@ #include "widgets/settingspages/GeneralPageView.hpp" #include "widgets/splits/SplitInput.hpp" -#include +#include #include #include #include diff --git a/tests/.clang-format b/tests/.clang-format index 7bae09f2ce3..0feaad9dc10 100644 --- a/tests/.clang-format +++ b/tests/.clang-format @@ -32,9 +32,6 @@ IncludeCategories: # Project includes - Regex: '^"[a-zA-Z\._-]+(/[a-zA-Z0-9\._-]+)*"$' Priority: 1 - # Third party library includes - - Regex: '<[[:alnum:].]+/[a-zA-Z0-9\._\/-]+>' - Priority: 3 # Qt includes - Regex: '^$' Priority: 3 @@ -42,12 +39,12 @@ IncludeCategories: # LibCommuni includes - Regex: "^$" Priority: 3 - # Misc libraries - - Regex: '^<[a-zA-Z_0-9]+\.h(pp)?>$' - Priority: 3 # Standard library includes - Regex: "^<[a-zA-Z_]+>$" Priority: 4 + # Third party library includes + - Regex: "^<([a-zA-Z_0-9-]+/)*[a-zA-Z_0-9-]+.h(pp)?>$" + Priority: 3 NamespaceIndentation: Inner PointerBindsToType: false SpacesBeforeTrailingComments: 2 From c4c94473ae4f7b3d01c4852105412f11f2575187 Mon Sep 17 00:00:00 2001 From: nerix Date: Sat, 2 Dec 2023 12:56:03 +0100 Subject: [PATCH 3/7] Do bounds-checking on more windows (#4797) Co-authored-by: pajlada --- CHANGELOG.md | 1 + src/singletons/WindowManager.cpp | 3 +-- src/util/InitUpdateButton.cpp | 2 +- src/util/WidgetHelpers.cpp | 13 +++++++++++ src/util/WidgetHelpers.hpp | 10 ++++++++ src/widgets/BaseWindow.cpp | 11 +++++++++ src/widgets/BaseWindow.hpp | 23 ++++++++++++------- src/widgets/dialogs/ColorPickerDialog.cpp | 9 ++++++-- src/widgets/dialogs/QualityPopup.cpp | 8 +++++-- src/widgets/dialogs/SelectChannelDialog.cpp | 11 ++++++--- src/widgets/dialogs/SettingsDialog.cpp | 15 ++++++++---- .../dialogs/switcher/QuickSwitcherPopup.cpp | 11 ++++++--- src/widgets/helper/ChannelView.cpp | 4 ++-- src/widgets/helper/SearchPopup.cpp | 10 ++++++-- src/widgets/settingspages/AboutPage.cpp | 10 +++++--- src/widgets/settingspages/FiltersPage.cpp | 5 ++-- src/widgets/splits/Split.cpp | 12 +++++++--- 17 files changed, 120 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cf1e866223..1ee1403f8c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ - Bugfix: Fixed lookahead/-behind not working in _Ignores_. (#4965) - Bugfix: Fixed Image Uploader accidentally deleting images with some hosts when link resolver was enabled. (#4971) - Bugfix: Fixed rare crash with Image Uploader when closing a split right after starting an upload. (#4971) +- Bugfix: Fixed some windows appearing between screens. (#4797) - Dev: Run miniaudio in a separate thread, and simplify it to not manage the device ourselves. There's a chance the simplification is a bad idea. (#4978) - Dev: Change clang-format from v14 to v16. (#4929) - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791) diff --git a/src/singletons/WindowManager.cpp b/src/singletons/WindowManager.cpp index a86fdbc5ac1..be352588c6f 100644 --- a/src/singletons/WindowManager.cpp +++ b/src/singletons/WindowManager.cpp @@ -87,8 +87,7 @@ void WindowManager::showAccountSelectPopup(QPoint point) w->refresh(); - QPoint buttonPos = point; - w->move(buttonPos.x() - 30, buttonPos.y()); + w->moveTo(point - QPoint(30, 0), widgets::BoundsChecking::CursorPosition); w->show(); w->setFocus(); } diff --git a/src/util/InitUpdateButton.cpp b/src/util/InitUpdateButton.cpp index 7d687b66732..b381af01ed8 100644 --- a/src/util/InitUpdateButton.cpp +++ b/src/util/InitUpdateButton.cpp @@ -24,7 +24,7 @@ void initUpdateButton(Button &button, globalPoint.setX(0); } - dialog->move(globalPoint); + dialog->moveTo(globalPoint, widgets::BoundsChecking::DesiredPosition); dialog->show(); dialog->raise(); diff --git a/src/util/WidgetHelpers.cpp b/src/util/WidgetHelpers.cpp index 7e68727603e..b5e6fa9a303 100644 --- a/src/util/WidgetHelpers.cpp +++ b/src/util/WidgetHelpers.cpp @@ -79,4 +79,17 @@ void moveWindowTo(QWidget *window, QPoint position, BoundsChecking mode) } } +void showAndMoveWindowTo(QWidget *window, QPoint position, BoundsChecking mode) +{ +#ifdef Q_OS_WINDOWS + window->show(); + + moveWindowTo(window, position, mode); +#else + moveWindowTo(window, position, mode); + + window->show(); +#endif +} + } // namespace chatterino::widgets diff --git a/src/util/WidgetHelpers.hpp b/src/util/WidgetHelpers.hpp index 7f57ad39383..b09e93d0b95 100644 --- a/src/util/WidgetHelpers.hpp +++ b/src/util/WidgetHelpers.hpp @@ -26,4 +26,14 @@ enum class BoundsChecking { void moveWindowTo(QWidget *window, QPoint position, BoundsChecking mode = BoundsChecking::DesiredPosition); +/// Moves the `window` to the (global) `position` +/// while doing bounds-checking according to `mode` to ensure the window stays on one screen. +/// Will also call show on the `window`, order is dependant on platform. +/// +/// @param window The window to move. +/// @param position The global position to move the window to. +/// @param mode The desired bounds checking. +void showAndMoveWindowTo(QWidget *window, QPoint position, + BoundsChecking mode = BoundsChecking::DesiredPosition); + } // namespace chatterino::widgets diff --git a/src/widgets/BaseWindow.cpp b/src/widgets/BaseWindow.cpp index ad12fe185b3..a98ab8da8c4 100644 --- a/src/widgets/BaseWindow.cpp +++ b/src/widgets/BaseWindow.cpp @@ -508,6 +508,11 @@ void BaseWindow::moveTo(QPoint point, widgets::BoundsChecking mode) widgets::moveWindowTo(this, point, mode); } +void BaseWindow::showAndMoveTo(QPoint point, widgets::BoundsChecking mode) +{ + widgets::showAndMoveWindowTo(this, point, mode); +} + void BaseWindow::resizeEvent(QResizeEvent *) { // Queue up save because: Window resized @@ -559,6 +564,12 @@ void BaseWindow::closeEvent(QCloseEvent *) void BaseWindow::showEvent(QShowEvent *) { +#ifdef Q_OS_WIN + if (this->flags_.has(BoundsCheckOnShow)) + { + this->moveTo(this->pos(), widgets::BoundsChecking::CursorPosition); + } +#endif } #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) diff --git a/src/widgets/BaseWindow.hpp b/src/widgets/BaseWindow.hpp index 197618657ae..3b46aea6bc5 100644 --- a/src/widgets/BaseWindow.hpp +++ b/src/widgets/BaseWindow.hpp @@ -27,14 +27,15 @@ class BaseWindow : public BaseWidget public: enum Flags { None = 0, - EnableCustomFrame = 1, - Frameless = 2, - TopMost = 4, - DisableCustomScaling = 8, - FramelessDraggable = 16, - DontFocus = 32, - Dialog = 64, - DisableLayoutSave = 128, + EnableCustomFrame = 1 << 0, + Frameless = 1 << 1, + TopMost = 1 << 2, + DisableCustomScaling = 1 << 3, + FramelessDraggable = 1 << 4, + DontFocus = 1 << 5, + Dialog = 1 << 6, + DisableLayoutSave = 1 << 7, + BoundsCheckOnShow = 1 << 8, }; enum ActionOnFocusLoss { Nothing, Delete, Close, Hide }; @@ -57,6 +58,12 @@ class BaseWindow : public BaseWidget void moveTo(QPoint point, widgets::BoundsChecking mode); + /** + * Moves the window to the given point and does bounds checking according to `mode` + * Depending on the platform, either the move or the show will take place first + **/ + void showAndMoveTo(QPoint point, widgets::BoundsChecking mode); + float scale() const override; float qtFontScale() const; diff --git a/src/widgets/dialogs/ColorPickerDialog.cpp b/src/widgets/dialogs/ColorPickerDialog.cpp index c15e4b16a18..5d1096d815e 100644 --- a/src/widgets/dialogs/ColorPickerDialog.cpp +++ b/src/widgets/dialogs/ColorPickerDialog.cpp @@ -13,8 +13,13 @@ namespace chatterino { ColorPickerDialog::ColorPickerDialog(const QColor &initial, QWidget *parent) - : BasePopup({BaseWindow::EnableCustomFrame, BaseWindow::DisableLayoutSave}, - parent) + : BasePopup( + { + BaseWindow::EnableCustomFrame, + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent) , color_() , dialogConfirmed_(false) { diff --git a/src/widgets/dialogs/QualityPopup.cpp b/src/widgets/dialogs/QualityPopup.cpp index 9c7d519db73..f090bee3d80 100644 --- a/src/widgets/dialogs/QualityPopup.cpp +++ b/src/widgets/dialogs/QualityPopup.cpp @@ -9,8 +9,12 @@ namespace chatterino { QualityPopup::QualityPopup(const QString &channelURL, QStringList options) - : BasePopup({BaseWindow::DisableLayoutSave}, - static_cast(&(getApp()->windows->getMainWindow()))) + : BasePopup( + { + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + static_cast(&(getApp()->windows->getMainWindow()))) , channelURL_(channelURL) { this->ui_.selector = new QComboBox(this); diff --git a/src/widgets/dialogs/SelectChannelDialog.cpp b/src/widgets/dialogs/SelectChannelDialog.cpp index 4c1dba08510..c74bfcc2c8a 100644 --- a/src/widgets/dialogs/SelectChannelDialog.cpp +++ b/src/widgets/dialogs/SelectChannelDialog.cpp @@ -31,9 +31,14 @@ namespace chatterino { SelectChannelDialog::SelectChannelDialog(QWidget *parent) - : BaseWindow({BaseWindow::Flags::EnableCustomFrame, - BaseWindow::Flags::Dialog, BaseWindow::DisableLayoutSave}, - parent) + : BaseWindow( + { + BaseWindow::Flags::EnableCustomFrame, + BaseWindow::Flags::Dialog, + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent) , selectedChannel_(Channel::getEmpty()) { this->setWindowTitle("Select a channel to join"); diff --git a/src/widgets/dialogs/SettingsDialog.cpp b/src/widgets/dialogs/SettingsDialog.cpp index 1ee94b4cfea..2fcfa31665d 100644 --- a/src/widgets/dialogs/SettingsDialog.cpp +++ b/src/widgets/dialogs/SettingsDialog.cpp @@ -6,6 +6,7 @@ #include "controllers/hotkeys/HotkeyController.hpp" #include "singletons/Settings.hpp" #include "util/LayoutCreator.hpp" +#include "widgets/BaseWindow.hpp" #include "widgets/helper/Button.hpp" #include "widgets/helper/SettingsDialogTab.hpp" #include "widgets/settingspages/AboutPage.hpp" @@ -29,9 +30,14 @@ namespace chatterino { SettingsDialog::SettingsDialog(QWidget *parent) - : BaseWindow({BaseWindow::Flags::DisableCustomScaling, - BaseWindow::Flags::Dialog, BaseWindow::DisableLayoutSave}, - parent) + : BaseWindow( + { + BaseWindow::Flags::DisableCustomScaling, + BaseWindow::Flags::Dialog, + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent) { this->setObjectName("SettingsDialog"); this->setWindowTitle("Chatterino Settings"); @@ -380,9 +386,10 @@ void SettingsDialog::themeChangedEvent() this->setPalette(palette); } -void SettingsDialog::showEvent(QShowEvent *) +void SettingsDialog::showEvent(QShowEvent *e) { this->ui_.search->setText(""); + BaseWindow::showEvent(e); } ///// Widget creation helpers diff --git a/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp b/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp index 5110b51e394..7841d06b4e4 100644 --- a/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp +++ b/src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp @@ -37,9 +37,14 @@ QList openPages(Window *window) namespace chatterino { QuickSwitcherPopup::QuickSwitcherPopup(Window *parent) - : BasePopup({BaseWindow::Flags::Frameless, BaseWindow::Flags::TopMost, - BaseWindow::DisableLayoutSave}, - parent) + : BasePopup( + { + BaseWindow::Flags::Frameless, + BaseWindow::Flags::TopMost, + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent) , switcherModel_(this) , window(parent) { diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index ff180f01c6b..c0f1f521278 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/src/widgets/helper/ChannelView.cpp @@ -2914,8 +2914,8 @@ void ChannelView::showReplyThreadPopup(const MessagePtr &message) popup->setThread(message->replyThread); QPoint offset(int(150 * this->scale()), int(70 * this->scale())); - popup->move(QCursor::pos() - offset); - popup->show(); + popup->showAndMoveTo(QCursor::pos() - offset, + widgets::BoundsChecking::CursorPosition); popup->giveFocus(Qt::MouseFocusReason); } diff --git a/src/widgets/helper/SearchPopup.cpp b/src/widgets/helper/SearchPopup.cpp index f768bdd1e67..329dff06802 100644 --- a/src/widgets/helper/SearchPopup.cpp +++ b/src/widgets/helper/SearchPopup.cpp @@ -63,7 +63,12 @@ ChannelPtr SearchPopup::filter(const QString &text, const QString &channelName, } SearchPopup::SearchPopup(QWidget *parent, Split *split) - : BasePopup({BaseWindow::DisableLayoutSave}, parent) + : BasePopup( + { + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent) , split_(split) { this->initLayout(); @@ -180,9 +185,10 @@ void SearchPopup::updateWindowTitle() this->setWindowTitle("Searching in " + historyName + " history"); } -void SearchPopup::showEvent(QShowEvent *) +void SearchPopup::showEvent(QShowEvent *e) { this->search(); + BaseWindow::showEvent(e); } bool SearchPopup::eventFilter(QObject *object, QEvent *event) diff --git a/src/widgets/settingspages/AboutPage.cpp b/src/widgets/settingspages/AboutPage.cpp index affea98cda2..87c5caa7e1f 100644 --- a/src/widgets/settingspages/AboutPage.cpp +++ b/src/widgets/settingspages/AboutPage.cpp @@ -227,9 +227,13 @@ void AboutPage::addLicense(QFormLayout *form, const QString &name, auto *b = new QLabel("show license"); QObject::connect( b, &QLabel::linkActivated, [parent = this, name, licenseLink] { - auto window = new BasePopup({BaseWindow::Flags::EnableCustomFrame, - BaseWindow::DisableLayoutSave}, - parent); + auto *window = new BasePopup( + { + BaseWindow::EnableCustomFrame, + BaseWindow::DisableLayoutSave, + BaseWindow::BoundsCheckOnShow, + }, + parent); window->setWindowTitle("Chatterino - License for " + name); window->setAttribute(Qt::WA_DeleteOnClose); auto layout = new QVBoxLayout(); diff --git a/src/widgets/settingspages/FiltersPage.cpp b/src/widgets/settingspages/FiltersPage.cpp index d367a3e3233..c66b139bee5 100644 --- a/src/widgets/settingspages/FiltersPage.cpp +++ b/src/widgets/settingspages/FiltersPage.cpp @@ -45,9 +45,8 @@ FiltersPage::FiltersPage() }); // We can safely ignore this signal connection since we own the view - std::ignore = view->addButtonPressed.connect([] { - ChannelFilterEditorDialog d( - static_cast(&(getApp()->windows->getMainWindow()))); + std::ignore = view->addButtonPressed.connect([this] { + ChannelFilterEditorDialog d(this->window()); if (d.exec() == QDialog::Accepted) { getSettings()->filterRecords.append( diff --git a/src/widgets/splits/Split.cpp b/src/widgets/splits/Split.cpp index 7d1a2b91e0d..046b905ae8d 100644 --- a/src/widgets/splits/Split.cpp +++ b/src/widgets/splits/Split.cpp @@ -192,8 +192,12 @@ namespace { void showTutorialVideo(QWidget *parent, const QString &source, const QString &title, const QString &description) { - auto window = - new BasePopup(BaseWindow::Flags::EnableCustomFrame, parent); + auto *window = new BasePopup( + { + BaseWindow::EnableCustomFrame, + BaseWindow::BoundsCheckOnShow, + }, + parent); window->setWindowTitle("Chatterino - " + title); window->setAttribute(Qt::WA_DeleteOnClose); auto layout = new QVBoxLayout(); @@ -1393,7 +1397,9 @@ void Split::showChatterList() multiWidget->setLayout(dockVbox); chatterDock->setWidget(multiWidget); chatterDock->setFloating(true); - chatterDock->show(); + widgets::showAndMoveWindowTo( + chatterDock, this->mapToGlobal(QPoint{0, this->header_->height()}), + widgets::BoundsChecking::CursorPosition); chatterDock->activateWindow(); } From 0fd4e8ffd60a969d6f3ae2d77ca95c21138f0141 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 2 Dec 2023 13:35:55 +0100 Subject: [PATCH 4/7] fix: typo in debug log for hotkeys (#4997) --- src/controllers/hotkeys/HotkeyController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/hotkeys/HotkeyController.cpp b/src/controllers/hotkeys/HotkeyController.cpp index 0fc85f97e79..ea618d99aa1 100644 --- a/src/controllers/hotkeys/HotkeyController.cpp +++ b/src/controllers/hotkeys/HotkeyController.cpp @@ -56,7 +56,7 @@ std::vector HotkeyController::shortcutsForCategory( { qCDebug(chatterinoHotkeys) << qPrintable(parent->objectName()) - << "Unimplemeneted hotkey action:" << hotkey->action() << "in " + << "Unimplemented hotkey action:" << hotkey->action() << "in" << hotkey->getCategory(); continue; } From 584a7c86fcd8f9f35354b320ef58293af3beb5e6 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 2 Dec 2023 14:04:43 +0100 Subject: [PATCH 5/7] Move clang-tidy to its own CI job (#4996) * Only run the `post-clang-tidy-review` if the `clang-tidy` build succeeded --- .github/workflows/build.yml | 38 ----- .github/workflows/clang-tidy.yml | 148 +++++++++++++++++++ .github/workflows/post-clang-tidy-review.yml | 4 +- CHANGELOG.md | 1 + 4 files changed, 152 insertions(+), 39 deletions(-) create mode 100644 .github/workflows/clang-tidy.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 30870797852..70a952134d5 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -37,7 +37,6 @@ jobs: plugins: false skip-artifact: false skip-crashpad: false - clang-tidy-review: false # Ubuntu 22.04, Qt 5.15 - os: ubuntu-22.04 qt-version: 5.15.2 @@ -45,7 +44,6 @@ jobs: plugins: false skip-artifact: false skip-crashpad: false - clang-tidy-review: true # Ubuntu 22.04, Qt 6.2.4 - tests LTO & plugins - os: ubuntu-22.04 qt-version: 6.2.4 @@ -53,7 +51,6 @@ jobs: plugins: true skip-artifact: false skip-crashpad: false - clang-tidy-review: false # macOS - os: macos-latest qt-version: 5.15.2 @@ -61,7 +58,6 @@ jobs: plugins: false skip-artifact: false skip-crashpad: false - clang-tidy-review: false # Windows - os: windows-latest qt-version: 6.5.0 @@ -69,7 +65,6 @@ jobs: plugins: false skip-artifact: false skip-crashpad: false - clang-tidy-review: false # Windows 7/8 - os: windows-latest qt-version: 5.15.2 @@ -77,7 +72,6 @@ jobs: plugins: false skip-artifact: false skip-crashpad: true - clang-tidy-review: false fail-fast: false @@ -329,38 +323,6 @@ jobs: make -j"$(nproc)" shell: bash - - name: clang-tidy review - if: matrix.clang-tidy-review && github.event_name == 'pull_request' - timeout-minutes: 10 - uses: ZedThree/clang-tidy-review@v0.14.0 - with: - build_dir: build-clang-tidy - config_file: ".clang-tidy" - split_workflow: true - exclude: "lib/*" - cmake_command: >- - cmake -S. -Bbuild-clang-tidy - -DCMAKE_BUILD_TYPE=Release - -DPAJLADA_SETTINGS_USE_BOOST_FILESYSTEM=On - -DUSE_PRECOMPILED_HEADERS=OFF - -DCMAKE_EXPORT_COMPILE_COMMANDS=On - -DCHATTERINO_LTO=Off - -DCHATTERINO_PLUGINS=On - -DBUILD_WITH_QT6=Off - -DBUILD_TESTS=On - -DBUILD_BENCHMARKS=On - apt_packages: >- - qttools5-dev, qt5-image-formats-plugins, libqt5svg5-dev, - libsecret-1-dev, - libboost-dev, libboost-system-dev, libboost-filesystem-dev, - libssl-dev, - rapidjson-dev, - libbenchmark-dev - - - name: clang-tidy-review upload - if: matrix.clang-tidy-review && github.event_name == 'pull_request' - uses: ZedThree/clang-tidy-review/upload@v0.14.0 - - name: Package - AppImage (Ubuntu) if: startsWith(matrix.os, 'ubuntu-20.04') && !matrix.skip-artifact run: | diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml new file mode 100644 index 00000000000..96468c137af --- /dev/null +++ b/.github/workflows/clang-tidy.yml @@ -0,0 +1,148 @@ +--- +name: clang-tidy + +on: + pull_request: + +concurrency: + group: clang-tidy-${{ github.ref }} + cancel-in-progress: true + +env: + CHATTERINO_REQUIRE_CLEAN_GIT: On + C2_BUILD_WITH_QT6: Off + +jobs: + build: + name: "clang-tidy ${{ matrix.os }}, Qt ${{ matrix.qt-version }})" + runs-on: ${{ matrix.os }} + strategy: + matrix: + include: + # Ubuntu 22.04, Qt 5.15 + - os: ubuntu-22.04 + qt-version: 5.15.2 + plugins: false + + fail-fast: false + + steps: + - name: Enable plugin support + if: matrix.plugins + run: | + echo "C2_PLUGINS=ON" >> "$GITHUB_ENV" + shell: bash + + - name: Set BUILD_WITH_QT6 + if: startsWith(matrix.qt-version, '6.') + run: | + echo "C2_BUILD_WITH_QT6=ON" >> "$GITHUB_ENV" + shell: bash + + - uses: actions/checkout@v4 + with: + submodules: recursive + fetch-depth: 0 # allows for tags access + + - name: Install Qt5 + if: startsWith(matrix.qt-version, '5.') + uses: jurplel/install-qt-action@v3.3.0 + with: + cache: true + cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2 + version: ${{ matrix.qt-version }} + + - name: Install Qt 6.5.3 imageformats + if: startsWith(matrix.qt-version, '6.') + uses: jurplel/install-qt-action@v3.3.0 + with: + cache: false + modules: qtimageformats + set-env: false + version: 6.5.3 + extra: --noarchives + + - name: Install Qt6 + if: startsWith(matrix.qt-version, '6.') + uses: jurplel/install-qt-action@v3.3.0 + with: + cache: true + cache-key-prefix: ${{ runner.os }}-QtCache-${{ matrix.qt-version }}-v2 + modules: qt5compat qtimageformats + version: ${{ matrix.qt-version }} + + # LINUX + - name: Install dependencies + run: | + sudo apt-get update + sudo apt-get -y install \ + cmake \ + virtualenv \ + rapidjson-dev \ + libfuse2 \ + libssl-dev \ + libboost-dev \ + libxcb-randr0-dev \ + libboost-system-dev \ + libboost-filesystem-dev \ + libpulse-dev \ + libxkbcommon-x11-0 \ + build-essential \ + libgl1-mesa-dev \ + libxcb-icccm4 \ + libxcb-image0 \ + libxcb-keysyms1 \ + libxcb-render-util0 \ + libxcb-xinerama0 + + - name: Apply Qt5 patches + if: startsWith(matrix.qt-version, '5.') + run: | + patch "$Qt5_DIR/include/QtConcurrent/qtconcurrentthreadengine.h" .patches/qt5-on-newer-gcc.patch + shell: bash + + - name: Build + run: | + mkdir build + cd build + CXXFLAGS=-fno-sized-deallocation cmake \ + -DCMAKE_INSTALL_PREFIX=appdir/usr/ \ + -DCMAKE_BUILD_TYPE=Release \ + -DPAJLADA_SETTINGS_USE_BOOST_FILESYSTEM=On \ + -DUSE_PRECOMPILED_HEADERS=OFF \ + -DCMAKE_EXPORT_COMPILE_COMMANDS=On \ + -DCHATTERINO_LTO="$C2_ENABLE_LTO" \ + -DCHATTERINO_PLUGINS="$C2_PLUGINS" \ + -DBUILD_WITH_QT6="$C2_BUILD_WITH_QT6" \ + .. + shell: bash + + - name: clang-tidy review + timeout-minutes: 20 + uses: ZedThree/clang-tidy-review@v0.14.0 + with: + build_dir: build-clang-tidy + config_file: ".clang-tidy" + split_workflow: true + exclude: "lib/*" + cmake_command: >- + cmake -S. -Bbuild-clang-tidy + -DCMAKE_BUILD_TYPE=Release + -DPAJLADA_SETTINGS_USE_BOOST_FILESYSTEM=On + -DUSE_PRECOMPILED_HEADERS=OFF + -DCMAKE_EXPORT_COMPILE_COMMANDS=On + -DCHATTERINO_LTO=Off + -DCHATTERINO_PLUGINS=On + -DBUILD_WITH_QT6=Off + -DBUILD_TESTS=On + -DBUILD_BENCHMARKS=On + apt_packages: >- + qttools5-dev, qt5-image-formats-plugins, libqt5svg5-dev, + libsecret-1-dev, + libboost-dev, libboost-system-dev, libboost-filesystem-dev, + libssl-dev, + rapidjson-dev, + libbenchmark-dev + + - name: clang-tidy-review upload + uses: ZedThree/clang-tidy-review/upload@v0.14.0 diff --git a/.github/workflows/post-clang-tidy-review.yml b/.github/workflows/post-clang-tidy-review.yml index e611a24f086..92ef0820cdb 100644 --- a/.github/workflows/post-clang-tidy-review.yml +++ b/.github/workflows/post-clang-tidy-review.yml @@ -3,13 +3,15 @@ name: Post clang-tidy review comments on: workflow_run: - workflows: ["Build"] + workflows: ["clang-tidy"] types: - completed jobs: build: runs-on: ubuntu-latest + # Only when a build succeeds + if: ${{ github.event.workflow_run.conclusion == 'success' }} steps: - uses: ZedThree/clang-tidy-review/post@v0.14.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ee1403f8c9..89bd4622954 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ - Dev: `Details` file properties tab is now populated on Windows. (#4912) - Dev: Removed `Outcome` from network requests. (#4959) - Dev: Added Tests for Windows and MacOS in CI. (#4970) +- Dev: Move `clang-tidy` checker to its own CI job. (#4996) - Dev: Refactored the Image Uploader feature. (#4971) - Dev: Fixed deadlock and use-after-free in tests. (#4981) From 812186dc4c4d7447104ca43802de65ab4c69ea34 Mon Sep 17 00:00:00 2001 From: nerix Date: Sun, 3 Dec 2023 14:41:33 +0100 Subject: [PATCH 6/7] Return correct hit-test values for title bar buttons on Windows (#4994) Co-authored-by: Rasmus Karlsson --- CHANGELOG.md | 1 + src/CMakeLists.txt | 2 + src/widgets/BaseWindow.cpp | 172 ++++++++++++++++++++----- src/widgets/BaseWindow.hpp | 6 +- src/widgets/helper/TitlebarButton.cpp | 35 +++++ src/widgets/helper/TitlebarButton.hpp | 18 +++ src/widgets/helper/TitlebarButtons.cpp | 116 +++++++++++++++++ src/widgets/helper/TitlebarButtons.hpp | 69 ++++++++++ 8 files changed, 381 insertions(+), 38 deletions(-) create mode 100644 src/widgets/helper/TitlebarButtons.cpp create mode 100644 src/widgets/helper/TitlebarButtons.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 89bd4622954..00c765cd7aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ - Bugfix: Fixed lookahead/-behind not working in _Ignores_. (#4965) - Bugfix: Fixed Image Uploader accidentally deleting images with some hosts when link resolver was enabled. (#4971) - Bugfix: Fixed rare crash with Image Uploader when closing a split right after starting an upload. (#4971) +- Bugfix: Fixed support for Windows 11 Snap layouts. (#4994) - Bugfix: Fixed some windows appearing between screens. (#4797) - Dev: Run miniaudio in a separate thread, and simplify it to not manage the device ourselves. There's a chance the simplification is a bad idea. (#4978) - Dev: Change clang-format from v14 to v16. (#4929) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 86c24c15b40..1c6c3c0a0f3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -626,6 +626,8 @@ set(SOURCE_FILES widgets/helper/SignalLabel.hpp widgets/helper/TitlebarButton.cpp widgets/helper/TitlebarButton.hpp + widgets/helper/TitlebarButtons.cpp + widgets/helper/TitlebarButtons.hpp widgets/listview/GenericItemDelegate.cpp widgets/listview/GenericItemDelegate.hpp diff --git a/src/widgets/BaseWindow.cpp b/src/widgets/BaseWindow.cpp index a98ab8da8c4..59abe0a4f4f 100644 --- a/src/widgets/BaseWindow.cpp +++ b/src/widgets/BaseWindow.cpp @@ -8,6 +8,7 @@ #include "util/PostToThread.hpp" #include "util/WindowsHelper.hpp" #include "widgets/helper/EffectLabel.hpp" +#include "widgets/helper/TitlebarButtons.hpp" #include "widgets/Label.hpp" #include "widgets/TooltipWidget.hpp" #include "widgets/Window.hpp" @@ -180,9 +181,8 @@ void BaseWindow::init() this->close(); }); - this->ui_.minButton = _minButton; - this->ui_.maxButton = _maxButton; - this->ui_.exitButton = _exitButton; + this->ui_.titlebarButtons = new TitleBarButtons( + this, _minButton, _maxButton, _exitButton); this->ui_.buttons.push_back(_minButton); this->ui_.buttons.push_back(_maxButton); @@ -474,12 +474,9 @@ void BaseWindow::changeEvent(QEvent *) } #ifdef USEWINSDK - if (this->ui_.maxButton) + if (this->ui_.titlebarButtons) { - this->ui_.maxButton->setButtonStyle( - this->windowState() & Qt::WindowMaximized - ? TitleBarButtonStyle::Unmaximize - : TitleBarButtonStyle::Maximize); + this->ui_.titlebarButtons->updateMaxButton(); } if (this->isVisible() && this->hasCustomWindowFrame()) @@ -585,6 +582,11 @@ bool BaseWindow::nativeEvent(const QByteArray &eventType, void *message, bool returnValue = false; + auto isHoveringTitlebarButton = [&]() { + auto ht = msg->wParam; + return ht == HTMAXBUTTON || ht == HTMINBUTTON || ht == HTCLOSE; + }; + switch (msg->message) { case WM_DPICHANGED: @@ -612,6 +614,91 @@ bool BaseWindow::nativeEvent(const QByteArray &eventType, void *message, returnValue = this->handleNCHITTEST(msg, result); break; + case WM_NCMOUSEHOVER: + case WM_NCMOUSEMOVE: { + // WM_NCMOUSEMOVE/WM_NCMOUSEHOVER gets sent when the mouse is + // moving/hovering in the non-client area + // - (mostly) the edges and the titlebar. + // We only need to handle the event for the titlebar buttons, + // as Qt doesn't create mouse events for these events. + if (!this->ui_.titlebarButtons) + { + // we don't consume the event if we don't have custom buttons + break; + } + + if (isHoveringTitlebarButton()) + { + *result = 0; + returnValue = true; + long x = GET_X_LPARAM(msg->lParam); + long y = GET_Y_LPARAM(msg->lParam); + + RECT winrect; + GetWindowRect(HWND(winId()), &winrect); + QPoint globalPos(x, y); + this->ui_.titlebarButtons->hover(msg->wParam, globalPos); + this->lastEventWasNcMouseMove_ = true; + } + else + { + this->ui_.titlebarButtons->leave(); + } + } + break; + + case WM_MOUSEMOVE: { + if (!this->lastEventWasNcMouseMove_) + { + break; + } + this->lastEventWasNcMouseMove_ = false; + // Windows doesn't send WM_NCMOUSELEAVE in some cases, + // so the buttons show as hovered even though they're not hovered. + [[fallthrough]]; + } + case WM_NCMOUSELEAVE: { + // WM_NCMOUSELEAVE gets sent when the mouse leaves any + // non-client area. In case we have titlebar buttons, + // we want to ensure they're deselected. + if (this->ui_.titlebarButtons) + { + this->ui_.titlebarButtons->leave(); + } + } + break; + + case WM_NCLBUTTONDOWN: + case WM_NCLBUTTONUP: { + // WM_NCLBUTTON{DOWN, UP} gets called when the left mouse button + // was pressed in a non-client area. + // We simulate a mouse down/up event for the titlebar buttons + // as Qt doesn't create an event in that case. + if (!this->ui_.titlebarButtons || !isHoveringTitlebarButton()) + { + break; + } + returnValue = true; + *result = 0; + + auto ht = msg->wParam; + long x = GET_X_LPARAM(msg->lParam); + long y = GET_Y_LPARAM(msg->lParam); + + RECT winrect; + GetWindowRect(HWND(winId()), &winrect); + QPoint globalPos(x, y); + if (msg->message == WM_NCLBUTTONDOWN) + { + this->ui_.titlebarButtons->mousePress(ht, globalPos); + } + else + { + this->ui_.titlebarButtons->mouseRelease(ht, globalPos); + } + } + break; + default: return QWidget::nativeEvent(eventType, message, result); } @@ -668,29 +755,21 @@ void BaseWindow::calcButtonsSizes() return; } - if (this->frameless_) + if (this->frameless_ || !this->ui_.titlebarButtons) { return; } - if ((this->width() / this->scale()) < 300) +#ifdef USEWINSDK + if ((static_cast(this->width()) / this->scale()) < 300) { - if (this->ui_.minButton) - this->ui_.minButton->setScaleIndependantSize(30, 30); - if (this->ui_.maxButton) - this->ui_.maxButton->setScaleIndependantSize(30, 30); - if (this->ui_.exitButton) - this->ui_.exitButton->setScaleIndependantSize(30, 30); + this->ui_.titlebarButtons->setSmallSize(); } else { - if (this->ui_.minButton) - this->ui_.minButton->setScaleIndependantSize(46, 30); - if (this->ui_.maxButton) - this->ui_.maxButton->setScaleIndependantSize(46, 30); - if (this->ui_.exitButton) - this->ui_.exitButton->setScaleIndependantSize(46, 30); + this->ui_.titlebarButtons->setRegularSize(); } +#endif } void BaseWindow::drawCustomWindowFrame(QPainter &painter) @@ -943,32 +1022,55 @@ bool BaseWindow::handleNCHITTEST(MSG *msg, long *result) if (*result == 0) { - bool client = false; - // Check the main layout first, as it's the largest area if (this->ui_.layoutBase->geometry().contains(point)) { - client = true; + *result = HTCLIENT; } // Check the titlebar buttons - if (!client && this->ui_.titlebarBox->geometry().contains(point)) + if (*result == 0 && + this->ui_.titlebarBox->geometry().contains(point)) { - for (QWidget *widget : this->ui_.buttons) + for (const auto *widget : this->ui_.buttons) { - if (widget->isVisible() && - widget->geometry().contains(point)) + if (!widget->isVisible() || + !widget->geometry().contains(point)) { - client = true; + continue; } + + if (const auto *btn = + dynamic_cast(widget)) + { + switch (btn->getButtonStyle()) + { + case TitleBarButtonStyle::Minimize: { + *result = HTMINBUTTON; + break; + } + case TitleBarButtonStyle::Unmaximize: + case TitleBarButtonStyle::Maximize: { + *result = HTMAXBUTTON; + break; + } + case TitleBarButtonStyle::Close: { + *result = HTCLOSE; + break; + } + default: { + *result = HTCLIENT; + break; + } + } + break; + } + *result = HTCLIENT; + break; } } - if (client) - { - *result = HTCLIENT; - } - else + if (*result == 0) { *result = HTCAPTION; } diff --git a/src/widgets/BaseWindow.hpp b/src/widgets/BaseWindow.hpp index 3b46aea6bc5..d55b5bd6d16 100644 --- a/src/widgets/BaseWindow.hpp +++ b/src/widgets/BaseWindow.hpp @@ -18,6 +18,7 @@ namespace chatterino { class Button; class EffectLabel; class TitleBarButton; +class TitleBarButtons; enum class TitleBarButtonStyle; class BaseWindow : public BaseWidget @@ -135,9 +136,7 @@ class BaseWindow : public BaseWidget QLayout *windowLayout = nullptr; QHBoxLayout *titlebarBox = nullptr; QWidget *titleLabel = nullptr; - TitleBarButton *minButton = nullptr; - TitleBarButton *maxButton = nullptr; - TitleBarButton *exitButton = nullptr; + TitleBarButtons *titlebarButtons = nullptr; QWidget *layoutBase = nullptr; std::vector