From ba0fbe3eecc7ee519c4e94b7ab8c39bbc32310e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 5 Oct 2024 17:20:00 +0200 Subject: [PATCH] Platform: make Sdl2Application::setMinimalLoopPeriod() unit-safe. I wanted to add this for GlfwApplication, only to realize the timer there is a double, in seconds, so accepting *integral* milliseconds there felt very weird. Let's use the fancy new time types instead. Also updated the docs to (hopefully) clarify that setSwapInterval() has to be called in order for the interaction between the two to work properly. As usual, the old variant taking untyped milliseconds is a deprecated alias to this one. --- doc/changelog.dox | 3 ++ src/Magnum/Platform/Sdl2Application.cpp | 39 ++++++++++++------ src/Magnum/Platform/Sdl2Application.h | 40 ++++++++++++++----- .../Platform/Test/Sdl2ApplicationTest.cpp | 2 +- 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 24328e0e5..dafddb84a 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1364,6 +1364,9 @@ See also: performance and a default, use @relativeref{Platform::EmscriptenApplication::GLConfiguration,Flag::PowerPreferenceLowPower} or @relativeref{Platform::EmscriptenApplication::GLConfiguration,Flag::PowerPreferenceHighPerformance} instead +- @cpp Platform::Sdl2Application::setMinimalLoopPeriod(UnsignedInt) @ce + taking an untyped millisecond value is deprecated in favor of + @relativeref{Platform::Sdl2Application,setMinimalLoopPeriod(Nanoseconds)} - @cpp Shaders::DistanceFieldVector @ce, @cpp Shaders::Flat @ce, @cpp Shaders::Generic @ce, @cpp Shaders::MeshVisualizer2D @ce, @cpp Shaders::MeshVisualizer3D @ce, @cpp Shaders::Phong @ce, diff --git a/src/Magnum/Platform/Sdl2Application.cpp b/src/Magnum/Platform/Sdl2Application.cpp index 06e299da3..7e2ddb234 100644 --- a/src/Magnum/Platform/Sdl2Application.cpp +++ b/src/Magnum/Platform/Sdl2Application.cpp @@ -52,6 +52,9 @@ #include "Magnum/Math/ConfigurationValue.h" #include "Magnum/Math/FunctionsBatch.h" #include "Magnum/Math/Range.h" +#ifndef CORRADE_TARGET_EMSCRIPTEN +#include "Magnum/Math/Time.h" +#endif #include "Magnum/Platform/ScreenedApplication.hpp" #include "Magnum/Platform/Implementation/DpiScaling.h" @@ -78,6 +81,7 @@ extern "C" { namespace Magnum { namespace Platform { using namespace Containers::Literals; +using namespace Math::Literals; namespace { @@ -126,9 +130,6 @@ Sdl2Application::Sdl2Application(const Arguments& arguments, const Configuration #endif Sdl2Application::Sdl2Application(const Arguments& arguments, NoCreateT): - #ifndef CORRADE_TARGET_EMSCRIPTEN - _minimalLoopPeriod{0}, - #endif _flags{Flag::Redraw} { Utility::Arguments args{Implementation::windowScalingArguments()}; @@ -826,6 +827,20 @@ bool Sdl2Application::setSwapInterval(const Int interval) { return true; } +#ifndef CORRADE_TARGET_EMSCRIPTEN +void Sdl2Application::setMinimalLoopPeriod(const Nanoseconds time) { + CORRADE_ASSERT(time >= 0_nsec, + "Platform::Sdl2Application::setMinimalLoopPeriod(): expected non-negative time, got" << time, ); + _minimalLoopPeriodMilliseconds = Long(time)/1000000; +} + +#ifdef MAGNUM_BUILD_DEPRECATED +void Sdl2Application::setMinimalLoopPeriod(const UnsignedInt milliseconds) { + _minimalLoopPeriodMilliseconds = milliseconds; +} +#endif +#endif + void Sdl2Application::redraw() { _flags |= Flag::Redraw; } Sdl2Application::~Sdl2Application() { @@ -890,7 +905,7 @@ bool Sdl2Application::mainLoopIteration() { #endif #ifndef CORRADE_TARGET_EMSCRIPTEN - const UnsignedInt timeBefore = _minimalLoopPeriod ? SDL_GetTicks() : 0; + const Nanoseconds timeBefore = _minimalLoopPeriodMilliseconds ? SDL_GetTicks()*1.0_msec : Nanoseconds{}; #endif #ifdef CORRADE_TARGET_EMSCRIPTEN @@ -1032,10 +1047,10 @@ bool Sdl2Application::mainLoopIteration() { #ifndef CORRADE_TARGET_EMSCRIPTEN /* If VSync is not enabled, delay to prevent CPU hogging (if set) */ - if(!(_flags & Flag::VSyncEnabled) && _minimalLoopPeriod) { - const UnsignedInt loopTime = SDL_GetTicks() - timeBefore; - if(loopTime < _minimalLoopPeriod) - SDL_Delay(_minimalLoopPeriod - loopTime); + if(!(_flags & Flag::VSyncEnabled) && _minimalLoopPeriodMilliseconds) { + const Nanoseconds loopTime = SDL_GetTicks()*1.0_msec - timeBefore; + if(loopTime < _minimalLoopPeriodMilliseconds*1.0_msec) + SDL_Delay(_minimalLoopPeriodMilliseconds - loopTime/1.0_msec); } #endif @@ -1044,10 +1059,10 @@ bool Sdl2Application::mainLoopIteration() { #ifndef CORRADE_TARGET_EMSCRIPTEN /* If not drawing anything, delay to prevent CPU hogging (if set) */ - if(_minimalLoopPeriod) { - const UnsignedInt loopTime = SDL_GetTicks() - timeBefore; - if(loopTime < _minimalLoopPeriod) - SDL_Delay(_minimalLoopPeriod - loopTime); + if(_minimalLoopPeriodMilliseconds) { + const Nanoseconds loopTime = SDL_GetTicks()*1.0_msec - timeBefore; + if(loopTime < _minimalLoopPeriodMilliseconds*1.0_msec) + SDL_Delay(_minimalLoopPeriodMilliseconds - loopTime/1.0_msec); } /* Then, if the tick event doesn't need to be called periodically, wait diff --git a/src/Magnum/Platform/Sdl2Application.h b/src/Magnum/Platform/Sdl2Application.h index 9de903728..b73699d39 100644 --- a/src/Magnum/Platform/Sdl2Application.h +++ b/src/Magnum/Platform/Sdl2Application.h @@ -897,18 +897,35 @@ class Sdl2Application { #ifndef CORRADE_TARGET_EMSCRIPTEN /** * @brief Set minimal loop period - * - * This setting reduces the main loop frequency in case VSync is - * not/cannot be enabled or no drawing is done. Default is @cpp 0 @ce - * (i.e. looping at maximum frequency). If the application is drawing - * on the screen and VSync is enabled, this setting is ignored. + * @m_since_latest + * + * This setting reduces the main loop frequency in case + * @ref setSwapInterval() wasn't called at all, was called with + * @cpp 0 @ce, the call failed, or no drawing is done and just + * @ref tickEvent() is being executed. The @p time is expected to be + * non-negative, default is @cpp 0_nsec @ce (i.e., looping at maximum + * frequency). If the application is drawing on the screen and VSync + * was enabled by calling @ref setSwapInterval(), this setting is + * ignored. + * + * Note that SDL timer resolution is just milliseconds, so anything + * below @cpp 1.0_msec @ce will behave the same as @cpp 0_nsec @ce. As + * the VSync default is driver-dependent, @ref setSwapInterval() has to + * be explicitly called to make the interaction between the two work + * correctly. * @note Not available in @ref CORRADE_TARGET_EMSCRIPTEN "Emscripten", * the browser is managing the frequency instead. - * @see @ref setSwapInterval() */ - void setMinimalLoopPeriod(UnsignedInt milliseconds) { - _minimalLoopPeriod = milliseconds; - } + void setMinimalLoopPeriod(Nanoseconds time); + + #ifdef MAGNUM_BUILD_DEPRECATED + /** + * @brief @copybrief setMinimalLoopPeriod(Nanoseconds) + * @m_deprecated_since_latest Use @ref setMinimalLoopPeriod(Nanoseconds), + * which prevents unit mismatch errors, instead. + */ + CORRADE_DEPRECATED("use setMinimalLoopPeriod(Nanoseconds) instead") void setMinimalLoopPeriod(UnsignedInt milliseconds); + #endif #endif /** @@ -1204,7 +1221,7 @@ class Sdl2Application { * If implemented, this function is called periodically after * processing all input events and before draw event even though there * might be no input events and redraw is not requested. Useful e.g. - * for asynchronous task polling. Use @ref setMinimalLoopPeriod()/ + * for asynchronous task polling. Use @ref setMinimalLoopPeriod() / * @ref setSwapInterval() to control main loop frequency. * * If this implementation gets called from its @cpp override @ce, it @@ -1253,7 +1270,8 @@ class Sdl2Application { #ifndef CORRADE_TARGET_EMSCRIPTEN SDL_Window* _window{}; - UnsignedInt _minimalLoopPeriod; + /* Not using Nanoseconds as that would require including Time.h */ + UnsignedInt _minimalLoopPeriodMilliseconds{}; #else SDL_Surface* _surface{}; Vector2i _lastKnownCanvasSize; diff --git a/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp b/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp index 40e449fbc..3ae58db5a 100644 --- a/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp +++ b/src/Magnum/Platform/Test/Sdl2ApplicationTest.cpp @@ -327,7 +327,7 @@ struct Sdl2ApplicationTest: Platform::Application { when redrawing constantly. */ #if 0 void tickEvent() override { - setMinimalLoopPeriod(250); + setMinimalLoopPeriod(250.0_msec); Debug{} << "tick event:" << Seconds{SDL_GetTicks()*1.0_msec}; } #endif