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

app/gui: support HiDPI on macOS #3248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KhoraLee
Copy link
Contributor

@KhoraLee KhoraLee commented Mar 12, 2024

This PR will enable HiDPI on macOS.

Screenshots

Latest Vita3K This PR
Screenshot 2024-03-13 at 4 15 28 AM Screenshot 2024-03-13 at 4 15 24 AM
Screenshot 2024-03-13 at 4 15 49 AM Screenshot 2024-03-13 at 4 15 45 AM
Screenshot 2024-03-13 at 4 19 31 AM

@KhoraLee KhoraLee changed the title vita3k: support HiDPI on macOS app/gui: support HiDPI on macOS Mar 12, 2024
@@ -319,10 +319,18 @@ bool init(EmuEnvState &state, Config &cfg, const Root &root_paths) {
window_type |= SDL_WINDOW_ALLOW_HIGHDPI;
state.dpi_scale = ddpi / 96;
}
#elif defined(__APPLE__)
window_type |= SDL_WINDOW_ALLOW_HIGHDPI;
state.dpi_scale = 2; // It is fixed value on macOS.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does using the windows/linux approach (calling SDL_GetDisplayDPI then dividing it by 96) not return 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS's SDL_GetDisplayDPI returns display's real ppi (in my case around 255: 3024x1964 14.2").

Comment on lines +331 to +333
#else
state.window = WindowPtr(SDL_CreateWindow(window_title, SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, DEFAULT_RES_WIDTH, DEFAULT_RES_HEIGHT, window_type | SDL_WINDOW_RESIZABLE), SDL_DestroyWindow);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think calling SDL_CreateWindow with DEFAULT_RES_WIDTH and DEFAULT_RES_HEIGHT should be the correct behavior on all platforms

Comment on lines 121 to +129
SDL_GetWindowSize(state->window, &w, &h);
ImGui_ImplSdl_GetDrawableSize(state, display_w, display_h);
#if __APPLE__
// On macOS, if HiDPI is enabled(SDL_WINDOW_ALLOW_HIGHDPI) SDL will create a window that size is 960x544 points.
// But it is actually 1920x1088 pixels. Since SDL_GetWindowSize returns 960x544 points, we need to multiply by 2.
w *= 2;
h *= 2;
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDL_GetWindowSize returns the screen dimensions in coordinates while ImGui_ImplSdl_GetDrawableSize returns the same screen dimension in pixel, this is done because of hidpi scaling. What you are doing is making ImGui believe the dpi scaling is 1.0 which sounds kind of counter-intuitive (and also forces you to implement workarounds like the one below for SDL_GetMouseState)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But from my windows pc with 150% scale, EmuEnvState's dpi_scale is 1.5 but it's w and display_w is same(960) and h and display is also same(544) so ImGui's DisplayFramebufferScale is set to (1, 1).

Also as screenshot shows it was rendering at real pixel. So I was thinking HiDPI is correctly working.

@Macdu
Copy link
Contributor

Macdu commented Mar 23, 2024

Hmm, I'm not entirely sure, but looking at SDL high_dpi stuff. We might actually not care at all about the SDL_WINDOW_ALLOW_HIGHDPI flag. What we could instead is not use this flag and just scale everything using the dpi scale. The result would be the same but not have different behavior on mac and other devices.

What setting SDL_WINDOW_ALLOW_HIGHDPI does is make screen coordinates bigger than screen pixels, but we really don't care about that as we use our own renderer and everything. What we care about is just making everything big/sharp enough which we should be able to do only using the dpi_scale variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants