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

Scale factor calculation on Linux #878

Open
taylon opened this issue Jun 2, 2020 · 6 comments
Open

Scale factor calculation on Linux #878

taylon opened this issue Jun 2, 2020 · 6 comments
Labels
A-rendering Area: Rendering artifacts, features etc. bug Something isn't working platform-linux Platform: Linux

Comments

@taylon
Copy link
Contributor

taylon commented Jun 2, 2020

Hi!

I noticed an issue with the scaling factor calculation in Linux that causes a very high scale factor to be set, which in turn makes Oni2 as an example to render like this:

Looking at

let display = Sdl2.Window.getDisplay(sdlWindow);
let dpi = Sdl2.Display.getDPI(display);
let avgDpi = (dpi.hdpi +. dpi.vdpi +. dpi.ddpi) /. 3.0;
let scaleFactor = max(1.0, floor(avgDpi /. 96.0));
I can see that the the DPI value returned by SDL2 is wrong:

[DEBUG]   +20ms Revery.Core.Window : hdpi: 130048.000000 / vdpi: 36576.000000 / ddpi: 95525.617188
[TRACE]    +0ms Revery.Core.Window : _getScaleFactor - Linux - inferring from DPI: 910.000000

I'm using a very unusual monitor resolution of 5120x1440 which might be an edge case that causes some weird bug, or it might be X returning it wrong for whatever reason.

Either way, what do you think would be a good mitigation for this?

We could say that if the resulting scaleFactor is higher than a given threshold then assume that the calculation is wrong and set it to 1.0?

@glennsl glennsl added A-rendering Area: Rendering artifacts, features etc. bug Something isn't working platform-linux Platform: Linux labels Jun 2, 2020
@zbaylin
Copy link
Member

zbaylin commented Jun 3, 2020

Part of me thinks this is the result of an error that SDL is giving us but we aren't catching. If we look here:
https://github.com/revery-ui/reason-sdl2/blob/fd489ba88457503c899fa06c655601aedb645db8/src/sdl2_wrapper.cpp#L415-L428

We can see that we dont check for an error like is documented here: https://wiki.libsdl.org/SDL_GetDisplayDPI

I can't imagine why it would return values like the ones you posted if it were operating properly. I'll investigate this tonight, sorry you're having this issue!

@taylon
Copy link
Contributor Author

taylon commented Jun 3, 2020

No problem, actually let me investigate that one :)

@zbaylin
Copy link
Member

zbaylin commented Jun 3, 2020

@taylon haha be my guest! Let me know what you come up with!

@taylon
Copy link
Contributor Author

taylon commented Jun 5, 2020

So I did some investigation and it is pretty complicated 😆

The values returned by SDL2 are indeed wrong, and that is because the value reported by xrandr itself is wrong (more precisely here SDL_x11modes.c).

It seems to be a pretty well known issue though, the following links have more details:
glfw/glfw#1019
mosra/magnum@ae31c3c

So at least in X11 it seems that the best way to obtain that information is through Xresources, since that is what most DEs use and also how glfw implements it:
https://github.com/glfw/glfw/blob/250b94cd03e6f947ba516869c7f3b277f8d0cacc/src/x11_init.c#L938

The best solution would probably be to implement something similar to glfw somewhere (potentially in SDL2?), although even then that would only cover X11, I have not investigated Wayland...

Either way It is hard to say how many users would end up with a messed up scale factor like I did, but I don't think it is worth the risk.

When users notice that the UI is too small they would look for zoom or something else in the documentation to make it bigger, but when they can't see anything it is a lot harder to know where to start looking for a solution. So my suggestion would be to default to 1.0 if GDK_SCALE is not set, at least until a more definitive solution gets implemented.

I went ahead and created a PR #896 for that and another one revery-ui/reason-sdl2#78 for handling the error potentially returned from SDL.

But let me know what you guys think :)

@zbaylin
Copy link
Member

zbaylin commented Jun 5, 2020

Wow that is pretty complicated. Thanks for doing all that research, @taylon! I think your fix in #896 is reasonable. To me, it makes more sense to have a potentially low-scale display then one that is way too big, as is what's seen in your case. It might make sense to patch something into Revery_Native to get a better solution when the SDL call returns None.

I'm curious to know what @bryphe thinks though. To me it seems that from a purely operational standpoint for programs like Oni, small rendering is better than none at all.

@bryphe
Copy link
Member

bryphe commented Jun 5, 2020

Thanks for the detailed investigation, @taylon ! Indeed... a scale factor of 910 is quite undesirable...

When users notice that the UI is too small they would look for zoom or something else in the documentation to make it bigger, but when they can't see anything it is a lot harder to know where to start looking for a solution. So my suggestion would be to default to 1.0 if GDK_SCALE is not set, at least until a more definitive solution gets implemented.

Agreed! This sounds like the right solution. I believe we had that inferred-DPI code back when we used GLFW, which may have worked correctly at that point (as it seems like GLFW handles the xrandr issue). Now that we have SDL2 though, it'd be better to just use the GDK_SCALE to at least prevent that massive-DPI issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rendering Area: Rendering artifacts, features etc. bug Something isn't working platform-linux Platform: Linux
Projects
None yet
Development

No branches or pull requests

4 participants