Opened 4 years ago

Closed 3 years ago

#12590 closed defect (fixed)

MAC OS X: Window getting smaller with each start

Reported by: criezy Owned by: criezy
Priority: high Component: Port: Mac OS X
Version: Keywords: has-pull-request
Cc: Game:

Description

When starting ScummVM the first time (with a fresh config file), the window is bigger than expected with a size of 1120x974. It also has a strange aspect ratio (neither 320/200 nor 320/240). And then every time I close and reopen ScummVM the window gets twice as small.

I also noticed that a size (last_window_width and last_window_height) gets added to the config file, but I don't know where it comes from (it doesn't even have the same aspect ratio than the window has).
After the first launch, the size in the config file is 504x420, then 252x210 after the second launch.

See screenshot where I just start and close ScummVM 3 times without doing anything and the window gets smaller and smaller.

This is on a non-retina macOS X 10.15 laptop with SDL 2.0.9 and current code from master. I am using the OpenGL graphics mode and aspect correction is off.

I will try with a more recent SDL version when I have a bit more time.

I also just noticed while testing to turn on aspect ratio than each time I change the aspect ratio (either with the Ctr+Alt+A shortcut, or in the Options dialog) the window size gets smaller. So I don't need to close and reopen ScummVM to reproduce that issue.

Attachments (3)

scummvm_start1.png (160.0 KB ) - added by criezy 4 years ago.
scummvm_start2.png (103.6 KB ) - added by criezy 4 years ago.
scummvm_start3.png (53.0 KB ) - added by criezy 4 years ago.

Download all attachments as: .zip

Change History (9)

by criezy, 4 years ago

Attachment: scummvm_start1.png added

by criezy, 4 years ago

Attachment: scummvm_start2.png added

by criezy, 4 years ago

Attachment: scummvm_start3.png added

comment:1 by criezy, 4 years ago

I have now tried with SDL 2.0.14 and the behaviour is the same.

I will tried to debug that issue this weekend, but any pointer on what to look at would be welcome (I am guessing this issue is probably related to the HiDPI changes).

comment:2 by sev-, 4 years ago

The problem is in openglsdl-graphics.cpp:308. I divide width and height by two. We need to wrap it in "if (hidpi)" condition.

To help me doing it, could you please tell me the output of line 300? For me it is "Setting 2016 x 1680 -> 576 x 480 -- 3.5" on the first run when there are no last_window_width/last_window_height specified.

comment:3 by criezy, 4 years ago

The output you gave is actually from gui/gui-manager.cpp line 142. The debug line in openglsdl-graphics.cpp, which is on line 300 and not 308, starts with "req:"
I get

Setting 1008 x 840 -> 864 x 720 -- 1.16667
req: 1008 x 840  cur: 1008 x 840, scale: 2

The scale we get from getDpiScalingFactor() is incorrect. As mentioned this is not a retina screen, and the scale should be 1 and not 2. I had a quick look at the code. The implementation of getDisplayDpiFromSdl() looks a bit fiddly with hardcoded default DPIs. 72DPI used to be the DPI for old (classic) MacOS, and early MacOS X days, but with the transition to LCD screens this has been deprecated for a long time in favour of a screen scaling factor (which no fixed default DPI). If we want an accurate scaling factor in getDpiScalingFactor() we might want to use [NSScreen backingScaleFactor] (or the equivalent SDL API if there is one, but I assume there isn't based on the way we currently try to guess that scaling).

Also from what I see getDpiScalingFactor() can only return 1 or 2, which may be incorrect. In some cases. With very high DPI screens as found on some iOS and Android devices, the scaling should be 3 (and thus similarly assuming 2 for HiDPI backends in GuiManager::computeScaleFactor() can be wrong).

I will try to look at adding a OSystem::getHiDPIScreenScaleFactor() this weekend so that backends can report the proper scaling, and I can implement this on iOS and macOS.

comment:4 by criezy, 3 years ago

Keywords: has-pull-request added

I created a pull request to fix this issue: https://github.com/scummvm/scummvm/pull/3264

comment:5 by sev-, 3 years ago

Priority: normalhigh

This would be nice to get fixed before the release.

comment:6 by criezy, 3 years ago

Owner: set to criezy
Resolution: fixed
Status: newclosed

The pull request has been merged, so this is now fixed.

Note: See TracTickets for help on using tickets.