From 90798b008a17f5807644d27935db796d1e9b51cb Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Wed, 5 Jun 2024 18:33:38 +0530 Subject: [PATCH 1/9] MacOS: Add support for timers to the platform integrations Task-Id:SRNTY-40 --- src/KDFoundation/CMakeLists.txt | 2 + .../macos/macos_platform_event_loop.h | 6 + .../macos/macos_platform_event_loop.mm | 16 +-- .../platform/macos/macos_platform_timer.h | 39 ++++++ .../platform/macos/macos_platform_timer.mm | 116 ++++++++++++++++++ .../core_application/tst_core_application.cpp | 2 - 6 files changed, 172 insertions(+), 9 deletions(-) create mode 100644 src/KDFoundation/platform/macos/macos_platform_timer.h create mode 100644 src/KDFoundation/platform/macos/macos_platform_timer.mm diff --git a/src/KDFoundation/CMakeLists.txt b/src/KDFoundation/CMakeLists.txt index 240c868..672d9d9 100644 --- a/src/KDFoundation/CMakeLists.txt +++ b/src/KDFoundation/CMakeLists.txt @@ -81,6 +81,8 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") platform/macos/macos_platform_event_loop.h platform/macos/macos_platform_integration.mm platform/macos/macos_platform_integration.h + platform/macos/macos_platform_timer.mm + platform/macos/macos_platform_timer.h ) endif() diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.h b/src/KDFoundation/platform/macos/macos_platform_event_loop.h index cbbe50f..1369490 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.h +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.h @@ -13,9 +13,13 @@ #include #include +#include namespace KDFoundation { +class MacOSPlatformTimer; +class NSTimerWrapper; + class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop { public: @@ -35,6 +39,8 @@ class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop static void postEmptyEvent(); + std::unordered_map timerMap; + private: std::unique_ptr createPlatformTimerImpl(Timer *timer) override; }; diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm index b14b944..ab00b4f 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm @@ -10,15 +10,17 @@ */ #include "macos_platform_event_loop.h" - +#include "macos_platform_timer.h" +#import +#include +#include #import #include - -using namespace KDFoundation; - constexpr auto KDFoundationCocoaEventSubTypeWakeup = std::numeric_limits::max(); +namespace KDFoundation { + MacOSPlatformEventLoop::MacOSPlatformEventLoop() { @autoreleasepool { @@ -78,9 +80,9 @@ // TODO return false; } - std::unique_ptr MacOSPlatformEventLoop::createPlatformTimerImpl(Timer *timer) { - // TODO - return {}; + return std::make_unique(timer); } + +} // namespace KDFoundation diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.h b/src/KDFoundation/platform/macos/macos_platform_timer.h new file mode 100644 index 0000000..39f7096 --- /dev/null +++ b/src/KDFoundation/platform/macos/macos_platform_timer.h @@ -0,0 +1,39 @@ +/* + This file is part of KDUtils. + + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company + Author: Paul Lemire + + SPDX-License-Identifier: MIT + + Contact KDAB at for commercial licensing options. +*/ + +#pragma once + +#include + +#include + +#include +#include + +namespace KDFoundation { + +class Timer; + +class KDFOUNDATION_API MacOSPlatformTimer : public AbstractPlatformTimer +{ +public: + MacOSPlatformTimer(Timer *timer); + ~MacOSPlatformTimer() override; + +private: + void arm(std::chrono::microseconds us); + void disarm(); + static void timerFired(void *context); + Timer *m_handler; + dispatch_source_t highResTimer; +}; + +} // namespace KDFoundation diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.mm b/src/KDFoundation/platform/macos/macos_platform_timer.mm new file mode 100644 index 0000000..af80d09 --- /dev/null +++ b/src/KDFoundation/platform/macos/macos_platform_timer.mm @@ -0,0 +1,116 @@ +/* + This file is part of KDUtils. + + SPDX-FileCopyrightText: 2023 Klarälvdalens Datakonsult AB, a KDAB Group company + Author: Paul Lemire + + SPDX-License-Identifier: MIT + + Contact KDAB at for commercial licensing options. +*/ + +#include "macos_platform_timer.h" +#include "macos_platform_event_loop.h" + +#include +#include +#include +#include +#include +#include +#import + +#include "KDFoundation/core_application.h" +#include "KDFoundation/timer.h" +#include "KDFoundation/platform/macos/macos_platform_event_loop.h" + +namespace KDFoundation { + +class NSTimerWrapper +{ + NSTimer *timer; +}; +} // namespace KDFoundation + +using namespace KDFoundation; + +inline MacOSPlatformEventLoop *eventLoop() +{ + return static_cast(CoreApplication::instance()->eventLoop()); +} + +MacOSPlatformTimer::MacOSPlatformTimer(Timer *timer) + : m_handler{ timer }, highResTimer{ nullptr } +{ + + @autoreleasepool { + // make sure there's a NSApp + [NSApplication sharedApplication]; + } + + timer->running.valueChanged().connect([this, timer](bool running) { + if (running) { + arm(timer->interval.get()); + } else { + disarm(); + } + }); + timer->interval.valueChanged().connect([this, timer]() { + if (timer->running.get()) { + arm(timer->interval.get()); + } + }); +} + +MacOSPlatformTimer::~MacOSPlatformTimer() +{ + disarm(); +} + +void MacOSPlatformTimer::timerFired(void *context) +{ + @autoreleasepool { + MacOSPlatformEventLoop *ev = eventLoop(); + dispatch_source_t caller = (dispatch_source_t)context; + if (auto it = ev->timerMap.find(caller); it != ev->timerMap.end()) { + it->second->m_handler->timeout.emit(); + } + } +} + +void MacOSPlatformTimer::arm(std::chrono::microseconds us) +{ + @autoreleasepool { + + if (highResTimer) { + disarm(); + } + const auto interval = std::chrono::duration_cast(us).count(); + dispatch_queue_main_t mainQueue = dispatch_get_main_queue(); + highResTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, mainQueue); + dispatch_source_set_timer(highResTimer, + dispatch_time(DISPATCH_TIME_NOW, interval), + interval, + 0); + dispatch_source_set_event_handler_f(highResTimer, timerFired); + dispatch_set_context(highResTimer, highResTimer); + dispatch_resume(highResTimer); + + if (highResTimer) { + void *key = reinterpret_cast(highResTimer); + eventLoop()->timerMap[key] = this; + } + } +} + +void MacOSPlatformTimer::disarm() +{ + @autoreleasepool { + if (highResTimer) { + void *key = reinterpret_cast(highResTimer); + eventLoop()->timerMap.erase(key); + dispatch_source_cancel(highResTimer); + highResTimer = nullptr; + } + } +} diff --git a/tests/auto/foundation/core_application/tst_core_application.cpp b/tests/auto/foundation/core_application/tst_core_application.cpp index 9fd06f5..f0e1567 100644 --- a/tests/auto/foundation/core_application/tst_core_application.cpp +++ b/tests/auto/foundation/core_application/tst_core_application.cpp @@ -188,7 +188,6 @@ TEST_CASE("Event handling") } } -#ifndef KD_PLATFORM_MACOS TEST_CASE("Timer handling") { SUBCASE("timer fires correctly") @@ -261,7 +260,6 @@ TEST_CASE("Timer handling") REQUIRE(fired == true); } } -#endif TEST_CASE("Main event loop") { From 739e506ae4989c902c4a14385b7b4d502fed376a Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Thu, 13 Jun 2024 18:47:41 +0530 Subject: [PATCH 2/9] remove unused forward declarations --- src/KDFoundation/platform/macos/macos_platform_event_loop.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.h b/src/KDFoundation/platform/macos/macos_platform_event_loop.h index 1369490..3087398 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.h +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.h @@ -18,7 +18,6 @@ namespace KDFoundation { class MacOSPlatformTimer; -class NSTimerWrapper; class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop { From 1df46c6c1706db6a39fb4d59332bf66d07fb6d24 Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Tue, 18 Jun 2024 15:21:50 +0530 Subject: [PATCH 3/9] move to CFTimer API --- .../platform/macos/macos_platform_timer.h | 6 +- .../platform/macos/macos_platform_timer.mm | 83 ++++++++----------- 2 files changed, 39 insertions(+), 50 deletions(-) diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.h b/src/KDFoundation/platform/macos/macos_platform_timer.h index 39f7096..2fee21a 100644 --- a/src/KDFoundation/platform/macos/macos_platform_timer.h +++ b/src/KDFoundation/platform/macos/macos_platform_timer.h @@ -11,7 +11,7 @@ #pragma once -#include +#include #include @@ -31,9 +31,9 @@ class KDFOUNDATION_API MacOSPlatformTimer : public AbstractPlatformTimer private: void arm(std::chrono::microseconds us); void disarm(); - static void timerFired(void *context); + static void timerFired(CFRunLoopTimerRef timer, void *info); Timer *m_handler; - dispatch_source_t highResTimer; + CFRunLoopTimerRef cfTimer; }; } // namespace KDFoundation diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.mm b/src/KDFoundation/platform/macos/macos_platform_timer.mm index af80d09..47949fe 100644 --- a/src/KDFoundation/platform/macos/macos_platform_timer.mm +++ b/src/KDFoundation/platform/macos/macos_platform_timer.mm @@ -12,26 +12,19 @@ #include "macos_platform_timer.h" #include "macos_platform_event_loop.h" +#include #include #include #include #include +#include #include #include -#import #include "KDFoundation/core_application.h" #include "KDFoundation/timer.h" #include "KDFoundation/platform/macos/macos_platform_event_loop.h" -namespace KDFoundation { - -class NSTimerWrapper -{ - NSTimer *timer; -}; -} // namespace KDFoundation - using namespace KDFoundation; inline MacOSPlatformEventLoop *eventLoop() @@ -40,14 +33,11 @@ } MacOSPlatformTimer::MacOSPlatformTimer(Timer *timer) - : m_handler{ timer }, highResTimer{ nullptr } + : m_handler{ timer }, cfTimer{ nullptr } { - @autoreleasepool { - // make sure there's a NSApp [NSApplication sharedApplication]; } - timer->running.valueChanged().connect([this, timer](bool running) { if (running) { arm(timer->interval.get()); @@ -67,50 +57,49 @@ disarm(); } -void MacOSPlatformTimer::timerFired(void *context) +void MacOSPlatformTimer::timerFired(CFRunLoopTimerRef timer, void *info) { - @autoreleasepool { - MacOSPlatformEventLoop *ev = eventLoop(); - dispatch_source_t caller = (dispatch_source_t)context; - if (auto it = ev->timerMap.find(caller); it != ev->timerMap.end()) { - it->second->m_handler->timeout.emit(); - } + MacOSPlatformEventLoop *ev = eventLoop(); + void *key = timer; + if (auto it = ev->timerMap.find(key); it != ev->timerMap.end()) { + SPDLOG_DEBUG("fired ->"); + it->second->m_handler->timeout.emit(); } } void MacOSPlatformTimer::arm(std::chrono::microseconds us) { - @autoreleasepool { - - if (highResTimer) { - disarm(); - } - const auto interval = std::chrono::duration_cast(us).count(); - dispatch_queue_main_t mainQueue = dispatch_get_main_queue(); - highResTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, mainQueue); - dispatch_source_set_timer(highResTimer, - dispatch_time(DISPATCH_TIME_NOW, interval), - interval, - 0); - dispatch_source_set_event_handler_f(highResTimer, timerFired); - dispatch_set_context(highResTimer, highResTimer); - dispatch_resume(highResTimer); + if (cfTimer) { + disarm(); + } - if (highResTimer) { - void *key = reinterpret_cast(highResTimer); - eventLoop()->timerMap[key] = this; - } - } + CFTimeInterval interval = std::chrono::duration_cast>(us).count(); + CFRunLoopTimerContext timerContext = { 0, NULL, NULL, NULL, NULL }; + CFAbsoluteTime fireDate = CFAbsoluteTimeGetCurrent() + interval; + // Create the timer + cfTimer = CFRunLoopTimerCreate( + kCFAllocatorDefault, // Allocator + fireDate, // Fire time + interval, // Interval + 0, // Flags + 0, // Order + timerFired, // Callback function + &timerContext // Timer context + ); + CFRunLoopAddTimer(CFRunLoopGetCurrent(), cfTimer, kCFRunLoopCommonModes); + + if (cfTimer) { + void *key = reinterpret_cast(cfTimer); + eventLoop()->timerMap[key] = this; + } } void MacOSPlatformTimer::disarm() { - @autoreleasepool { - if (highResTimer) { - void *key = reinterpret_cast(highResTimer); - eventLoop()->timerMap.erase(key); - dispatch_source_cancel(highResTimer); - highResTimer = nullptr; - } + if (cfTimer) { + void *key = reinterpret_cast(cfTimer); + eventLoop()->timerMap.erase(key); + CFRelease(cfTimer); + cfTimer = nullptr; } } From 009f7f4f04b04aeb05e8a3acc9663784756ebe2b Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Tue, 18 Jun 2024 15:27:47 +0530 Subject: [PATCH 4/9] remove debug statements --- src/KDFoundation/platform/macos/macos_platform_timer.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.mm b/src/KDFoundation/platform/macos/macos_platform_timer.mm index 47949fe..9966fa8 100644 --- a/src/KDFoundation/platform/macos/macos_platform_timer.mm +++ b/src/KDFoundation/platform/macos/macos_platform_timer.mm @@ -62,7 +62,6 @@ MacOSPlatformEventLoop *ev = eventLoop(); void *key = timer; if (auto it = ev->timerMap.find(key); it != ev->timerMap.end()) { - SPDLOG_DEBUG("fired ->"); it->second->m_handler->timeout.emit(); } } From 79058bb69a490b2ce4e443aed397a0cb9f667844 Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Tue, 18 Jun 2024 17:03:13 +0530 Subject: [PATCH 5/9] addressing comments --- .../platform/macos/macos_platform_event_loop.mm | 5 ++--- src/KDFoundation/platform/macos/macos_platform_timer.h | 2 +- src/KDFoundation/platform/macos/macos_platform_timer.mm | 6 ------ 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm index ab00b4f..29f37cb 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.mm +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.mm @@ -12,11 +12,10 @@ #include "macos_platform_event_loop.h" #include "macos_platform_timer.h" #import -#include -#include #import - #include +#include + constexpr auto KDFoundationCocoaEventSubTypeWakeup = std::numeric_limits::max(); namespace KDFoundation { diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.h b/src/KDFoundation/platform/macos/macos_platform_timer.h index 2fee21a..60165ad 100644 --- a/src/KDFoundation/platform/macos/macos_platform_timer.h +++ b/src/KDFoundation/platform/macos/macos_platform_timer.h @@ -25,7 +25,7 @@ class Timer; class KDFOUNDATION_API MacOSPlatformTimer : public AbstractPlatformTimer { public: - MacOSPlatformTimer(Timer *timer); + explicit MacOSPlatformTimer(Timer *timer); ~MacOSPlatformTimer() override; private: diff --git a/src/KDFoundation/platform/macos/macos_platform_timer.mm b/src/KDFoundation/platform/macos/macos_platform_timer.mm index 9966fa8..a704117 100644 --- a/src/KDFoundation/platform/macos/macos_platform_timer.mm +++ b/src/KDFoundation/platform/macos/macos_platform_timer.mm @@ -11,8 +11,6 @@ #include "macos_platform_timer.h" #include "macos_platform_event_loop.h" - -#include #include #include #include @@ -20,7 +18,6 @@ #include #include #include - #include "KDFoundation/core_application.h" #include "KDFoundation/timer.h" #include "KDFoundation/platform/macos/macos_platform_event_loop.h" @@ -35,9 +32,6 @@ MacOSPlatformTimer::MacOSPlatformTimer(Timer *timer) : m_handler{ timer }, cfTimer{ nullptr } { - @autoreleasepool { - [NSApplication sharedApplication]; - } timer->running.valueChanged().connect([this, timer](bool running) { if (running) { arm(timer->interval.get()); From 6a2bbd8078b385e747cbb1505dd78c629f93d2ad Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Tue, 18 Jun 2024 17:19:58 +0530 Subject: [PATCH 6/9] make the timer map private --- src/KDFoundation/platform/macos/macos_platform_event_loop.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/KDFoundation/platform/macos/macos_platform_event_loop.h b/src/KDFoundation/platform/macos/macos_platform_event_loop.h index 3087398..7574345 100644 --- a/src/KDFoundation/platform/macos/macos_platform_event_loop.h +++ b/src/KDFoundation/platform/macos/macos_platform_event_loop.h @@ -38,10 +38,11 @@ class KDFOUNDATION_API MacOSPlatformEventLoop : public AbstractPlatformEventLoop static void postEmptyEvent(); - std::unordered_map timerMap; - private: std::unique_ptr createPlatformTimerImpl(Timer *timer) override; + + std::unordered_map timerMap; + friend class MacOSPlatformTimer; }; } // namespace KDFoundation From 824a0f8f0af568bb955afc004dacf332d282fa5d Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Wed, 19 Jun 2024 01:14:54 +0530 Subject: [PATCH 7/9] tune timer to fix CI test runs --- tests/auto/foundation/core_application/tst_core_application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auto/foundation/core_application/tst_core_application.cpp b/tests/auto/foundation/core_application/tst_core_application.cpp index f0e1567..3dc083f 100644 --- a/tests/auto/foundation/core_application/tst_core_application.cpp +++ b/tests/auto/foundation/core_application/tst_core_application.cpp @@ -205,7 +205,7 @@ TEST_CASE("Timer handling") timer.timeout.connect([&]() { const auto endTime = std::chrono::steady_clock::now(); REQUIRE(endTime - time > 50ms); - REQUIRE(endTime - time < 150ms); + REQUIRE(endTime - time < 200ms); time = endTime; timeout++; }); From bde5a92cb9398f33a4e7f8c38fe0e7bdf8bca911 Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Wed, 19 Jun 2024 01:26:03 +0530 Subject: [PATCH 8/9] add info statements for elapsed time --- .../auto/foundation/core_application/tst_core_application.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/auto/foundation/core_application/tst_core_application.cpp b/tests/auto/foundation/core_application/tst_core_application.cpp index 3dc083f..a2ced20 100644 --- a/tests/auto/foundation/core_application/tst_core_application.cpp +++ b/tests/auto/foundation/core_application/tst_core_application.cpp @@ -204,6 +204,8 @@ TEST_CASE("Timer handling") auto time = startTime; timer.timeout.connect([&]() { const auto endTime = std::chrono::steady_clock::now(); + const auto elapsedTime = std::chrono::duration_cast(endTime - time).count(); + SPDLOG_INFO("elapsedTime = {}", elapsedTime); REQUIRE(endTime - time > 50ms); REQUIRE(endTime - time < 200ms); time = endTime; @@ -213,6 +215,7 @@ TEST_CASE("Timer handling") while (std::chrono::steady_clock::now() - startTime < 500ms) { app.processEvents(500); } + SPDLOG_INFO("timeout = {}", timeout); REQUIRE(timeout > 3); REQUIRE(timeout < 8); From 03400780c1861415ff67de47b54a2cc060dbb10a Mon Sep 17 00:00:00 2001 From: Abhishek Sudhakaran Date: Wed, 19 Jun 2024 01:42:42 +0530 Subject: [PATCH 9/9] increase test case timeout --- tests/auto/foundation/core_application/tst_core_application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/auto/foundation/core_application/tst_core_application.cpp b/tests/auto/foundation/core_application/tst_core_application.cpp index a2ced20..48a1984 100644 --- a/tests/auto/foundation/core_application/tst_core_application.cpp +++ b/tests/auto/foundation/core_application/tst_core_application.cpp @@ -188,7 +188,7 @@ TEST_CASE("Event handling") } } -TEST_CASE("Timer handling") +TEST_CASE("Timer handling" * doctest::timeout(120)) { SUBCASE("timer fires correctly") {