From 70e8a88274c93f4db1b5cd9804de97a79d445e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 12 Aug 2026 13:20:43 +0200 Subject: [PATCH] Platform: go back to caching DPI scaling values. In 6ab321a38c40933ec57b87ff4dc3300e05e679f0 (2023) I made a change where the DPI scaling value is queried every time it's accessed and nothing is cached, matching what's done with window and framebuffer size. However, a consequence of that is that (on Linux at least) the whole complexity of querying and calling X11 symbols is then re-run each time a DPI scaling value is used, which in certain use cases may be several times per frame. And if one is running Wayland, or hitting any of the other corner cases that produce a warning, it causes endless log spam in the output. Sorry, that wasn't a good idea. Now the value is cached again, and explicitly re-queried on each window size change, with log output suppressed to not repeatedly spam the same warnings. Which should solve the original issue why the changes in 6ab321a38c40933ec57b87ff4dc3300e05e679f0 were done at all, but without the downsides. The documentation of dpiScaling() was also still saying that the value is only available once a window is created (which wasn't true until now), this commit reverts back to that. Furthermore, in case of Emscripten, the DPI scaling is only depending on configuration and/or URL arguments specified upon startup, thus the value can be simply just cached forever without having to recalculate it. What changes there instead is the device pixel ratio, which gets re-queried each (browser) window size change. --- doc/changelog.dox | 17 +++-- src/Magnum/Platform/EmscriptenApplication.cpp | 57 +++++++++-------- src/Magnum/Platform/EmscriptenApplication.h | 14 ++-- src/Magnum/Platform/GlfwApplication.cpp | 39 +++++------ src/Magnum/Platform/GlfwApplication.h | 18 ++++-- .../Platform/Implementation/DpiScaling.cpp | 6 +- .../Platform/Implementation/DpiScaling.h | 6 +- src/Magnum/Platform/Sdl2Application.cpp | 64 +++++++++++-------- src/Magnum/Platform/Sdl2Application.h | 18 ++++-- 9 files changed, 141 insertions(+), 98 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 094dba107..66e043d44 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -910,15 +910,14 @@ See also: changes current working directory to `Resources/` in the app bundle on Apple platforms. - @ref Platform::EmscriptenApplication, @ref Platform::GlfwApplication and - @ref Platform::Sdl2Application now explicitly query DPI scaling values each - time they're accessed, either directly or via the viewport event, instead - of always returning the initially queried / calculated value. This makes - the behavior consistent with framebuffer and window sizes that are also - queried every time, and fixes a case where changing global UI scaling - would trigger a viewport event but the event would still have the previous - UI scale. However note that this does not yet properly handle DPI change - events themselves, which happen for example when moving windows across - displays with different DPI. + @ref Platform::Sdl2Application now re-query DPI scaling values on each + window or canvas resize to fix a case where changing global UI scalimg + would trigget a viewport event but the event would still have the previous + UI scale. Other than that, a cached DPI scaling value is returned each time + it's accessed to reduce the query overhead (and possible output log spam) + on certain platforms. Note also that this does not yet properly handle DPI + change events themselves, which happen for example when moving windows + across displays with different DPI. - Added more keys to @ref Platform::AbstractXApplication::KeyEvent::Key and implemented @ref Platform::AbstractXApplication::scrollEvent() for better consistency with other application implementations diff --git a/src/Magnum/Platform/EmscriptenApplication.cpp b/src/Magnum/Platform/EmscriptenApplication.cpp index a021db754..1540d3fc0 100644 --- a/src/Magnum/Platform/EmscriptenApplication.cpp +++ b/src/Magnum/Platform/EmscriptenApplication.cpp @@ -290,20 +290,16 @@ void EmscriptenApplication::create(const Configuration& configuration, const GLC #endif Vector2 EmscriptenApplication::dpiScaling(const Configuration& configuration) const { - return dpiScalingInternal(configuration.dpiScaling()); -} - -Vector2 EmscriptenApplication::dpiScalingInternal(const Vector2& configurationDpiScaling) const { - std::ostream* verbose = _verboseLog ? Debug::output() : nullptr; + std::ostream* const verbose = _verboseLog ? Debug::output() : nullptr; /* Use values from the configuration only if not overridden on command line. In any case explicit scaling has a precedence before the policy. */ if(!_commandLineDpiScaling.isZero()) { Debug{verbose} << "Platform::EmscriptenApplication: user-defined DPI scaling" << _commandLineDpiScaling; return _commandLineDpiScaling; - } else if(!configurationDpiScaling.isZero()) { - Debug{verbose} << "Platform::EmscriptenApplication: app-defined DPI scaling" << configurationDpiScaling; - return configurationDpiScaling; + } else if(!configuration.dpiScaling().isZero()) { + Debug{verbose} << "Platform::EmscriptenApplication: app-defined DPI scaling" << configuration.dpiScaling(); + return configuration.dpiScaling(); } /* Unlike Sdl2Application, not taking device pixel ratio into account @@ -334,6 +330,10 @@ bool EmscriptenApplication::tryCreate(const Configuration& configuration) { _lastKnownCanvasSize = windowSize(); _lastKnownDevicePixelRatio = devicePixelRatio(); + /* Device pixel ratio together with DPI scaling (which is 1.0 by default) + defines framebuffer size. See class docs for why it's done like that. */ + Debug{verbose} << "Platform::EmscriptenApplication: device pixel ratio" << _lastKnownDevicePixelRatio.x(); + /* By default Emscripten creates a 300x150 canvas. That's so freaking random I'm getting mad. Use the real (CSS pixels) canvas size instead, if the size is not hardcoded from the configuration. This is then @@ -349,12 +349,12 @@ bool EmscriptenApplication::tryCreate(const Configuration& configuration) { Debug{verbose} << "Platform::EmscriptenApplication::tryCreate(): autodetected canvas size" << canvasSize; } - /* Save DPI scaling value from configuration for future use. Device pixel - ratio together with DPI scaling (which is 1.0 by default) defines - framebuffer size. See class docs for why it's done like that. */ - _configurationDpiScaling = configuration.dpiScaling(); - Debug{verbose} << "Platform::EmscriptenApplication: device pixel ratio" << _lastKnownDevicePixelRatio.x(); - const Vector2i scaledCanvasSize = canvasSize*dpiScaling(configuration)*_lastKnownDevicePixelRatio; + /* Save DPI scaling value from configuration for future use. Unlike with + device pixel ratio, the DPI scaling value is only based on values set on + startup so unlike SDL or GLFW we don't need to remember the inputs and + also never need to update it afterwards. */ + _dpiScaling = dpiScaling(configuration); + const Vector2i scaledCanvasSize = canvasSize*_dpiScaling*_lastKnownDevicePixelRatio; emscripten_set_canvas_element_size(_canvasTarget.data(), scaledCanvasSize.x(), scaledCanvasSize.y()); setupCallbacks(!!(configuration.windowFlags() & Configuration::WindowFlag::Resizable)); @@ -414,6 +414,10 @@ bool EmscriptenApplication::tryCreate(const Configuration& configuration, const _lastKnownCanvasSize = windowSize(); _lastKnownDevicePixelRatio = devicePixelRatio(); + /* Device pixel ratio together with DPI scaling (which is 1.0 by default) + defines framebuffer size. See class docs for why it's done like that. */ + Debug{verbose} << "Platform::EmscriptenApplication: device pixel ratio" << _lastKnownDevicePixelRatio.x(); + /* By default Emscripten creates a 300x150 canvas. That's so freaking random I'm getting mad. Use the real (CSS pixels) canvas size instead, if the size is not hardcoded from the configuration. This is then @@ -429,12 +433,12 @@ bool EmscriptenApplication::tryCreate(const Configuration& configuration, const Debug{verbose} << "Platform::EmscriptenApplication::tryCreate(): autodetected canvas size" << canvasSize; } - /* Save DPI scaling value from configuration for future use. Device pixel - ratio together with DPI scaling (which is 1.0 by default) defines - framebuffer size. See class docs for why it's done like that. */ - _configurationDpiScaling = configuration.dpiScaling(); - Debug{verbose} << "Platform::EmscriptenApplication: device pixel ratio" << _lastKnownDevicePixelRatio.x(); - const Vector2i scaledCanvasSize = canvasSize*dpiScaling(configuration)*_lastKnownDevicePixelRatio; + /* Save DPI scaling value from configuration for future use. Unlike with + device pixel ratio, the DPI scaling value is only based on values set on + startup so unlike SDL or GLFW we don't need to remember the inputs and + also never need to update it afterwards. */ + _dpiScaling = dpiScaling(configuration); + const Vector2i scaledCanvasSize = canvasSize*_dpiScaling*_lastKnownDevicePixelRatio; emscripten_set_canvas_element_size(_canvasTarget.data(), scaledCanvasSize.x(), scaledCanvasSize.y()); /* Create WebGL context */ @@ -472,10 +476,6 @@ Vector2i EmscriptenApplication::framebufferSize() const { } #endif -Vector2 EmscriptenApplication::dpiScaling() const { - return dpiScalingInternal(_configurationDpiScaling); -} - Vector2 EmscriptenApplication::devicePixelRatio() const { return Vector2{Float(emscripten_get_device_pixel_ratio())}; } @@ -504,14 +504,17 @@ void EmscriptenApplication::handleCanvasResize(const EmscriptenUiEvent* event) { if(canvasSize != _lastKnownCanvasSize || devicePixelRatio != _lastKnownDevicePixelRatio) { _lastKnownCanvasSize = canvasSize; _lastKnownDevicePixelRatio = devicePixelRatio; - const Vector2 dpiScaling = this->dpiScaling(); - const Vector2i size = canvasSize*dpiScaling*devicePixelRatio; + /* Compared to SDL2 or GLFW, here we're *not* refreshing the DPI + scaling value, as it's hardcoded on startup and thus cannot change + in response to window size change. Only the device pixel ratio can + change. */ + const Vector2i size = canvasSize*_dpiScaling*_lastKnownDevicePixelRatio; emscripten_set_canvas_element_size(_canvasTarget.data(), size.x(), size.y()); ViewportEvent e{event, canvasSize, #ifdef MAGNUM_TARGET_GL framebufferSize(), #endif - dpiScaling, devicePixelRatio}; + _dpiScaling, _lastKnownDevicePixelRatio}; viewportEvent(e); /* Can't say just _flags | Flag::Redraw because in case the diff --git a/src/Magnum/Platform/EmscriptenApplication.h b/src/Magnum/Platform/EmscriptenApplication.h index 503926396..e7ad23239 100644 --- a/src/Magnum/Platform/EmscriptenApplication.h +++ b/src/Magnum/Platform/EmscriptenApplication.h @@ -577,7 +577,7 @@ class EmscriptenApplication { * more information. * @see @ref framebufferSize(), @ref devicePixelRatio() */ - Vector2 dpiScaling() const; + Vector2 dpiScaling() const { return _dpiScaling; } /** * @brief DPI scaling for given configuration @@ -1126,8 +1126,6 @@ class EmscriptenApplication { typedef Containers::EnumSet Flags; CORRADE_ENUMSET_FRIEND_OPERATORS(Flags) - Vector2 dpiScalingInternal(const Vector2& configurationDpiScaling) const; - void handleCanvasResize(const EmscriptenUiEvent* event); /* Sorry, but can't use Configuration::WindowFlags here :( */ void setupCallbacks(bool resizable); @@ -1165,10 +1163,14 @@ class EmscriptenApplication { Containers::Optional _context; #endif - /* These are saved from command-line arguments, and from configuration - to be reused in dpiScaling() and viewportEvent() later */ + /* These are saved from command-line arguments. Unlike with SDL or + GLFW, the _dpiScaling value is based on values set on startup and + thus cannot change afterwards, so we don't need to save any input + values coming from configuration, only the calculated result. What + *can* change is the device pixel ratio, which is tracked in + _lastKnownDevicePixelRatio above. */ bool _verboseLog{}; - Vector2 _commandLineDpiScaling, _configurationDpiScaling; + Vector2 _commandLineDpiScaling, _dpiScaling; /* Animation frame callback */ int (*_callback)(void*); diff --git a/src/Magnum/Platform/GlfwApplication.cpp b/src/Magnum/Platform/GlfwApplication.cpp index 61bc56936..1bfa01b08 100644 --- a/src/Magnum/Platform/GlfwApplication.cpp +++ b/src/Magnum/Platform/GlfwApplication.cpp @@ -191,8 +191,9 @@ Vector2 GlfwApplication::dpiScaling(const Configuration& configuration) { return dpiScalingInternal(configuration.dpiScalingPolicy(), configuration.dpiScaling()); } -Vector2 GlfwApplication::dpiScalingInternal(const Implementation::GlfwDpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling) const { - std::ostream* verbose = _verboseLog ? Debug::output() : nullptr; +Vector2 GlfwApplication::dpiScalingInternal(const Implementation::GlfwDpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling, const bool silentLog) const { + std::ostream* const output = silentLog ? nullptr : Debug::output(); + std::ostream* const verbose = _verboseLog && !silentLog ? Debug::output() : nullptr; /* Use values from the configuration only if not overridden on command line to something non-default. In any case explicit scaling has a precedence @@ -234,7 +235,7 @@ Vector2 GlfwApplication::dpiScalingInternal(const Implementation::GlfwDpiScaling SDL anyway. So keeping it to reduce the chance for unexpected minor differences across app implementations. */ #ifdef _MAGNUM_PLATFORM_USE_X11 - const Vector2 dpiScaling{Implementation::x11DpiScaling()}; + const Vector2 dpiScaling{Implementation::x11DpiScaling(output)}; if(!dpiScaling.isZero()) { Debug{verbose} << "Platform::GlfwApplication: virtual DPI scaling" << dpiScaling.x(); return dpiScaling; @@ -371,11 +372,12 @@ bool GlfwApplication::tryCreate(const Configuration& configuration) { CORRADE_ASSERT(!_window, "Platform::GlfwApplication::tryCreate(): window already created", false); - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = configuration.size()*dpiScaling(configuration); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = configuration.size()*_dpiScaling; /* Window flags */ GLFWmonitor* monitor = nullptr; /* Needed for setting fullscreen */ @@ -449,11 +451,12 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf CORRADE_ASSERT(!_window && _context->version() == GL::Version::None, "Platform::GlfwApplication::tryCreate(): window with OpenGL context already created", false); - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = configuration.size()*dpiScaling(configuration); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = configuration.size()*_dpiScaling; /* Window flags */ GLFWmonitor* monitor = nullptr; /* Needed for setting fullscreen */ @@ -693,10 +696,14 @@ void GlfwApplication::setupCallbacks() { #endif (_window, [](GLFWwindow* const window, const int w, const int h) { auto& app = *static_cast(glfwGetWindowUserPointer(window)); + /* Refresh the DPI scaling value, as it might have changed as a side + effect of the window size change. Suppress the log output as any + warnings got likely already printed during startup. */ + app._dpiScaling = app.dpiScalingInternal(app._configurationDpiScalingPolicy, app._configurationDpiScaling, /*silentLog*/ true); #ifdef MAGNUM_TARGET_GL - ViewportEvent e{app.windowSize(), {w, h}, app.dpiScaling()}; + ViewportEvent e{app.windowSize(), {w, h}, app._dpiScaling}; #else - ViewportEvent e{{w, h}, app.dpiScaling()}; + ViewportEvent e{{w, h}, app._dpiScaling}; #endif app.viewportEvent(e); }); @@ -810,14 +817,14 @@ Vector2i GlfwApplication::windowSize() const { void GlfwApplication::setWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::GlfwApplication::setWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; glfwSetWindowSize(_window, newSize.x(), newSize.y()); } void GlfwApplication::setMinWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::GlfwApplication::setMinWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; glfwSetWindowSizeLimits(_window, newSize.x(), newSize.y(), _maxWindowSize.x(), _maxWindowSize.y()); _minWindowSize = newSize; } @@ -825,7 +832,7 @@ void GlfwApplication::setMinWindowSize(const Vector2i& size) { void GlfwApplication::setMaxWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::GlfwApplication::setMaxWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; glfwSetWindowSizeLimits(_window, _minWindowSize.x(), _minWindowSize.y(), newSize.x(), newSize.y()); _maxWindowSize = newSize; } @@ -840,10 +847,6 @@ Vector2i GlfwApplication::framebufferSize() const { } #endif -Vector2 GlfwApplication::dpiScaling() const { - return dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); -} - void GlfwApplication::setSwapInterval(const Int interval) { glfwSwapInterval(interval); diff --git a/src/Magnum/Platform/GlfwApplication.h b/src/Magnum/Platform/GlfwApplication.h index 87cc81b2d..2213c8d19 100644 --- a/src/Magnum/Platform/GlfwApplication.h +++ b/src/Magnum/Platform/GlfwApplication.h @@ -519,13 +519,13 @@ class GlfwApplication { * @brief DPI scaling * * How the content should be scaled relative to system defaults for - * given @ref windowSize(). If a window is not created yet, returns + * given @ref windowSize(). If a window is not created yet, returns a * zero vector, use @ref dpiScaling(const Configuration&) for * calculating a value independently. See @ref Platform-GlfwApplication-dpi * for more information. * @see @ref framebufferSize() */ - Vector2 dpiScaling() const; + Vector2 dpiScaling() const { return _dpiScaling; } /** * @brief DPI scaling for given configuration @@ -1029,7 +1029,11 @@ class GlfwApplication { typedef Containers::EnumSet Flags; CORRADE_ENUMSET_FRIEND_OPERATORS(Flags) - Vector2 dpiScalingInternal(Implementation::GlfwDpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling) const; + /* Called from dpiScaling(const Configuration&) and then from + tryCreate() (two separate locations!) and in response to window size + events (there with silent log), with the return value cached to + _dpiScaling below. */ + Vector2 dpiScalingInternal(Implementation::GlfwDpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling, bool silentLog = false) const; void setupCallbacks(); @@ -1045,10 +1049,16 @@ class GlfwApplication { Cursor _cursor = Cursor::Arrow; /* These are saved from command-line arguments, and from configuration - to be reused in dpiScaling() and viewportEvent() later */ + to be reused in dpiScalingInternal() called in response to window + size events later */ bool _verboseLog{}; Implementation::GlfwDpiScalingPolicy _commandLineDpiScalingPolicy{}, _configurationDpiScalingPolicy{}; Vector2 _commandLineDpiScaling, _configurationDpiScaling; + /* Cached to not repeatedly do the heavy machinery of querying X11 + symbols etc. along with fallback code paths every time DPI scaling + gets queried from user code or used in setWindowSize() and such. Is + updated in response to window size events. */ + Vector2 _dpiScaling; GLFWwindow* _window{nullptr}; /* Not using Nanoseconds as that would require including Time.h */ diff --git a/src/Magnum/Platform/Implementation/DpiScaling.cpp b/src/Magnum/Platform/Implementation/DpiScaling.cpp index c82cc3393..e0a91f97f 100644 --- a/src/Magnum/Platform/Implementation/DpiScaling.cpp +++ b/src/Magnum/Platform/Implementation/DpiScaling.cpp @@ -76,7 +76,7 @@ Utility::Arguments windowScalingArguments() { } #ifdef _MAGNUM_PLATFORM_USE_X11 -Float x11DpiScaling() { +Float x11DpiScaling(std::ostream* output) { /* If the end app links to X11, these symbols will be available in a global scope and we can use that to query the DPI. If not, then those symbols won't be and that's okay -- it may be using Wayland or something else. */ @@ -107,7 +107,7 @@ Float x11DpiScaling() { #endif auto xrmDestroyDatabase = reinterpret_cast(dlsym(xlib, "XrmDestroyDatabase")); if(!xOpenDisplay || !xCloseDisplay || !xResourceManagerString || !xrmGetStringDatabase || !xrmGetResource || !xrmDestroyDatabase) { - Warning{} << "Platform: can't load X11 symbols for getting virtual DPI scaling, falling back to physical DPI"; + Warning{output} << "Platform: can't load X11 symbols for getting virtual DPI scaling, falling back to physical DPI"; return {}; } @@ -143,7 +143,7 @@ Float x11DpiScaling() { return 1.0f; } - Warning{} << "Platform: can't get Xft.dpi property for virtual DPI scaling, falling back to physical DPI"; + Warning{output} << "Platform: can't get Xft.dpi property for virtual DPI scaling, falling back to physical DPI"; return {}; } #endif diff --git a/src/Magnum/Platform/Implementation/DpiScaling.h b/src/Magnum/Platform/Implementation/DpiScaling.h index 0eb2ed8f5..3c36b3e85 100644 --- a/src/Magnum/Platform/Implementation/DpiScaling.h +++ b/src/Magnum/Platform/Implementation/DpiScaling.h @@ -30,6 +30,10 @@ #include "Magnum/Magnum.h" +#ifdef _MAGNUM_PLATFORM_USE_X11 +#include +#endif + namespace Magnum { namespace Platform { namespace Implementation { Utility::Arguments windowScalingArguments(); @@ -38,7 +42,7 @@ Utility::Arguments windowScalingArguments(); /* Returns DPI scaling for current X11 instance. Because X11 (as opposed to Wayland) doesn't have per-monitor scaling, it's fetched from the default display. */ -Float x11DpiScaling(); +Float x11DpiScaling(std::ostream* output); #endif #ifdef CORRADE_TARGET_EMSCRIPTEN diff --git a/src/Magnum/Platform/Sdl2Application.cpp b/src/Magnum/Platform/Sdl2Application.cpp index af46f69e3..0c146e72b 100644 --- a/src/Magnum/Platform/Sdl2Application.cpp +++ b/src/Magnum/Platform/Sdl2Application.cpp @@ -303,8 +303,11 @@ Vector2 Sdl2Application::dpiScaling(const Configuration& configuration) { return dpiScalingInternal(configuration.dpiScalingPolicy(), configuration.dpiScaling()); } -Vector2 Sdl2Application::dpiScalingInternal(const Implementation::Sdl2DpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling) const { - std::ostream* verbose = _verboseLog ? Debug::output() : nullptr; +Vector2 Sdl2Application::dpiScalingInternal(const Implementation::Sdl2DpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling, const bool silentLog) const { + #if defined(CORRADE_TARGET_UNIX) && !defined(CORRADE_TARGET_APPLE) && !defined(CORRADE_TARGET_EMSCRIPTEN) && !defined(CORRADE_TARGET_ANDROID) + std::ostream* const output = silentLog ? nullptr : Debug::output(); + #endif + std::ostream* const verbose = _verboseLog && !silentLog ? Debug::output() : nullptr; /* Use values from the configuration only if not overridden on command line to something non-default. In any case explicit scaling has a precedence @@ -344,7 +347,7 @@ Vector2 Sdl2Application::dpiScalingInternal(const Implementation::Sdl2DpiScaling /* Use Xft.dpi on X11, because SDL_GetDisplayDPI() returns the useless physical value on Linux, while the virtual value on Windows. */ #ifdef _MAGNUM_PLATFORM_USE_X11 - const Vector2 dpiScaling{Implementation::x11DpiScaling()}; + const Vector2 dpiScaling{Implementation::x11DpiScaling(output)}; if(!dpiScaling.isZero()) { Debug{verbose} << "Platform::Sdl2Application: virtual DPI scaling" << dpiScaling.x(); return dpiScaling; @@ -405,7 +408,7 @@ Vector2 Sdl2Application::dpiScalingInternal(const Implementation::Sdl2DpiScaling return dpiScaling; } - Warning{} << "Platform::Sdl2Application: can't get physical display DPI, falling back to no scaling:" << SDL_GetError(); + Warning{output} << "Platform::Sdl2Application: can't get physical display DPI, falling back to no scaling:" << SDL_GetError(); return Vector2{1.0f}; /* HOWEVER, on Windows it gets the virtual DPI scaling, which we don't @@ -482,11 +485,12 @@ bool Sdl2Application::tryCreate(const Configuration& configuration) { #endif #ifndef CORRADE_TARGET_EMSCRIPTEN - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = configuration.size()*dpiScaling(configuration); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = configuration.size()*_dpiScaling; /* Create window */ if(!(_window = SDL_CreateWindow( @@ -529,11 +533,12 @@ bool Sdl2Application::tryCreate(const Configuration& configuration) { windowSize = _lastKnownCanvasSize; Debug{_verboseLog ? Debug::output() : nullptr} << "Platform::Sdl2Application::tryCreate(): autodetected canvas size" << windowSize; } - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = windowSize*dpiScaling(configuration); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = windowSize*_dpiScaling; Uint32 flags = SDL_OPENGL|SDL_HWSURFACE|SDL_DOUBLEBUF; if(configuration.windowFlags() & Configuration::WindowFlag::Resizable) { @@ -578,11 +583,12 @@ bool Sdl2Application::tryCreate(const Configuration& configuration, const GLConf #endif #ifndef CORRADE_TARGET_EMSCRIPTEN - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = configuration.size()*dpiScaling(configuration); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = configuration.size()*_dpiScaling; /* Request debug context if GpuValidation is enabled either via the configuration or via command-line */ @@ -770,11 +776,12 @@ bool Sdl2Application::tryCreate(const Configuration& configuration, const GLConf windowSize = _lastKnownCanvasSize; Debug{_verboseLog ? Debug::output() : nullptr} << "Platform::Sdl2Application::tryCreate(): autodetected canvas size" << windowSize; } - /* Save DPI scaling values from configuration for future use, scale window - based on those */ + /* Save DPI scaling values from configuration for future use in viewport + events, scale window based on those */ _configurationDpiScalingPolicy = configuration.dpiScalingPolicy(); _configurationDpiScaling = configuration.dpiScaling(); - const Vector2i scaledWindowSize = windowSize*dpiScaling(); + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); + const Vector2i scaledWindowSize = windowSize*_dpiScaling; Uint32 flags = SDL_OPENGL|SDL_HWSURFACE|SDL_DOUBLEBUF; if(configuration.windowFlags() & Configuration::WindowFlag::Resizable) { @@ -829,21 +836,21 @@ Vector2i Sdl2Application::windowSize() const { void Sdl2Application::setWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::Sdl2Application::setWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; SDL_SetWindowSize(_window, newSize.x(), newSize.y()); } void Sdl2Application::setMinWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::Sdl2Application::setMinWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; SDL_SetWindowMinimumSize(_window, newSize.x(), newSize.y()); } void Sdl2Application::setMaxWindowSize(const Vector2i& size) { CORRADE_ASSERT(_window, "Platform::Sdl2Application::setMaxWindowSize(): no window opened", ); - const Vector2i newSize = dpiScaling()*size; + const Vector2i newSize = _dpiScaling*size; SDL_SetWindowMaximumSize(_window, newSize.x(), newSize.y()); } #endif @@ -862,10 +869,6 @@ Vector2i Sdl2Application::framebufferSize() const { } #endif -Vector2 Sdl2Application::dpiScaling() const { - return dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling); -} - #ifdef CORRADE_TARGET_EMSCRIPTEN void Sdl2Application::setContainerCssClass(const Containers::StringView cssClass) { magnumPlatformSetContainerCssClass(cssClass.data(), cssClass.size()); @@ -1039,14 +1042,18 @@ bool Sdl2Application::mainLoopIteration() { const Vector2i canvasSizei{canvasSize}; if(canvasSizei != _lastKnownCanvasSize) { + /* Refresh the DPI scaling value, as it might have changed as a + side effect of the canvas size change. Suppress the log output + as any warnings got likely already printed during startup. */ + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling, /*silentLog*/ true); _lastKnownCanvasSize = canvasSizei; - const Vector2i size = dpiScaling()*canvasSizei; + const Vector2i size = _dpiScaling*canvasSizei; emscripten_set_canvas_element_size("#canvas", size.x(), size.y()); ViewportEvent e{ #ifdef MAGNUM_TARGET_GL size, #endif - size, dpiScaling()}; + size, _dpiScaling}; viewportEvent(e); _flags |= Flag::Redraw; } @@ -1068,6 +1075,11 @@ bool Sdl2Application::mainLoopIteration() { https://github.com/kripken/emscripten/issues/1731 */ CORRADE_INTERNAL_ASSERT_UNREACHABLE(); #else + /* Refresh the DPI scaling value, as it might have + changed as a side effect of the window size change. + Suppress the log output as any warnings got likely + already printed during startup. */ + _dpiScaling = dpiScalingInternal(_configurationDpiScalingPolicy, _configurationDpiScaling, /*silentLog*/ true); /* {event.window.data1, event.window.data2} seems to be framebuffer size and not window size on macOS, which is weird. Query the values directly instead to be @@ -1076,7 +1088,7 @@ bool Sdl2Application::mainLoopIteration() { #ifdef MAGNUM_TARGET_GL framebufferSize(), #endif - dpiScaling()}; + _dpiScaling}; /** @todo handle also WM_DPICHANGED events when a window is moved between displays with different DPI */ viewportEvent(e); _flags |= Flag::Redraw; diff --git a/src/Magnum/Platform/Sdl2Application.h b/src/Magnum/Platform/Sdl2Application.h index 048093e9b..ec6325502 100644 --- a/src/Magnum/Platform/Sdl2Application.h +++ b/src/Magnum/Platform/Sdl2Application.h @@ -937,13 +937,13 @@ class Sdl2Application { * @brief DPI scaling * * How the content should be scaled relative to system defaults for - * given @ref windowSize(). If a window is not created yet, returns + * given @ref windowSize(). If a window is not created yet, returns a * zero vector, use @ref dpiScaling(const Configuration&) for * calculating a value independently. See @ref Platform-Sdl2Application-dpi * for more information. * @see @ref framebufferSize() */ - Vector2 dpiScaling() const; + Vector2 dpiScaling() const { return _dpiScaling; } /** * @brief DPI scaling for given configuration @@ -1581,7 +1581,11 @@ class Sdl2Application { typedef Containers::EnumSet Flags; CORRADE_ENUMSET_FRIEND_OPERATORS(Flags) - Vector2 dpiScalingInternal(Implementation::Sdl2DpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling) const; + /* Called from dpiScaling(const Configuration&) and then from + tryCreate() (four separate locations!) and in response to window / + canvas size events (two separate locations, both with silent log), + with the return value cached to _dpiScaling below. */ + Vector2 dpiScalingInternal(Implementation::Sdl2DpiScalingPolicy configurationDpiScalingPolicy, const Vector2& configurationDpiScaling, bool silentLog = false) const; #ifndef CORRADE_TARGET_EMSCRIPTEN SDL_Cursor* _cursors[12]{}; @@ -1590,10 +1594,16 @@ class Sdl2Application { #endif /* These are saved from command-line arguments, and from configuration - to be reused in dpiScaling() and viewportEvent() later */ + to be reused in dpiScalingInternal() called in response to window + size events later */ bool _verboseLog{}; Implementation::Sdl2DpiScalingPolicy _commandLineDpiScalingPolicy{}, _configurationDpiScalingPolicy{}; Vector2 _commandLineDpiScaling, _configurationDpiScaling; + /* Cached to not repeatedly do the heavy machinery of querying X11 + symbols etc. along with fallback code paths every time DPI scaling + gets queried from user code or used in setWindowSize() and such. Is + updated in response to viewport events. */ + Vector2 _dpiScaling; #ifndef CORRADE_TARGET_EMSCRIPTEN SDL_Window* _window{};