Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Sdl|Glfw)Application: _commandLineDpiScalingPolicy always default, app-defined scaling never takes effect #416

Closed
rune-scape opened this issue Jan 12, 2020 · 5 comments

Comments

@rune-scape
Copy link

If there is no --magnum-dpi-scaling specified on the command line, dpi-scaling is set to "default"

Which according to this code, which is run by every ctor of GlfwApplication:

/* Save command-line arguments */
if(args.value("log") == "verbose") _verboseLog = true;
const std::string dpiScaling = args.value("dpi-scaling");
if(dpiScaling == "default")
_commandLineDpiScalingPolicy = Implementation::GlfwDpiScalingPolicy::Default;
#ifdef CORRADE_TARGET_APPLE
else if(dpiScaling == "framebuffer")
_commandLineDpiScalingPolicy = Implementation::GlfwDpiScalingPolicy::Framebuffer;
#else
else if(dpiScaling == "virtual")
_commandLineDpiScalingPolicy = Implementation::GlfwDpiScalingPolicy::Virtual;
else if(dpiScaling == "physical")
_commandLineDpiScalingPolicy = Implementation::GlfwDpiScalingPolicy::Physical;
#endif
else if(dpiScaling.find_first_of(" \t\n") != std::string::npos)
_commandLineDpiScaling = args.value<Vector2>("dpi-scaling");
else
_commandLineDpiScaling = Vector2{args.value<Float>("dpi-scaling")};

_commandLineDpiScalingPolicy is set here and only here to GlfwDpiScalingPolicy::Default, which is just an alias to the platform-dependant default policy.

Crucially GlfwDpiScalingPolicy::Default is not 0
so... when this code comes along:

/* Use values from the configuration only if not overriden on command line.
In any case explicit scaling has a precedence before the policy. */
Implementation::GlfwDpiScalingPolicy dpiScalingPolicy{};
if(!_commandLineDpiScaling.isZero()) {
Debug{verbose} << "Platform::GlfwApplication: user-defined DPI scaling" << _commandLineDpiScaling.x();
return _commandLineDpiScaling;
} else if(UnsignedByte(_commandLineDpiScalingPolicy)) {
dpiScalingPolicy = _commandLineDpiScalingPolicy;
} else if(!configuration.dpiScaling().isZero()) {
Debug{verbose} << "Platform::GlfwApplication: app-defined DPI scaling" << _commandLineDpiScaling.x();
return configuration.dpiScaling();
} else {
dpiScalingPolicy = configuration.dpiScalingPolicy();
}

and checks if(UnsignedByte(_commandLineDpiScalingPolicy))
The other 2 cases are not possible!

@rune-scape
Copy link
Author

rune-scape commented Jan 12, 2020

I made a tiny edit in Magnum/Platform/Implementation/DpiScaling.cpp
changing this line:

args.addOption("dpi-scaling", "default")

to
args.addOption("dpi-scaling")
taking out the default option, because that seems to be what the first branch was made to parse
and it does what i would expect it to do

@rune-scape
Copy link
Author

this is also because the glfw dpi detection always uses monitor dpi instead of glfwGetWindowContentScale()

@mosra
Copy link
Owner

mosra commented Jan 12, 2020

Re glfwGetWindowContentScale() -- this is on my list to fix for Windows (#411), unfortunately right now i only have access to a Mac, where the DPI scaling is the weirdest of all, impatiently waiting for my real computer to get back from a repair 😅

Re the other thing -- yep there's definitely something fishy. I'll check what SDL does also, I remember at least one of the two had it working correctly at some point.

@rune-scape
Copy link
Author

thank you!

@mosra mosra added this to the 2020.0a milestone Jan 12, 2020
@mosra mosra added this to TODO in Platforms via automation Feb 15, 2020
@mosra
Copy link
Owner

mosra commented Feb 15, 2020

The overrides should be fixed in 65743b5, thanks a lot for reporting this -- somehow it was completely broken right from the start :/

Virtual DPI scaling in GLFW on Windows is implemented in 444b925, which makes #243 almost done, except for DPI change events.

@mosra mosra closed this as completed Feb 15, 2020
Platforms automation moved this from TODO to Done Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Platforms
  
Done
Development

No branches or pull requests

2 participants