Farama-Foundation / Arcade-Learning-Environment

The Arcade Learning Environment (ALE) -- a platform for AI research.
GNU General Public License v2.0
2.12k stars 420 forks source link

Resize window when using SDL/display_screen #446

Open floringogianu opened 2 years ago

floringogianu commented 2 years ago

Using display_screen on a 1920x1080 display opens a window taller than the display itself. Resizing the window is also disabled. This is on Ubuntu 21.10.

Is there any way to scale or adjust the DPI of the SDL window? Or maybe disable HiDPI if it's enabled by default? Looking through the repo I couldn't find anything. Also usual environment flags I've been using in the past with other applications, such as GDK_SCALE=0.5 GDK_DPI_SCALE=0.5 are not working in this instance.

Thank you!

JesseFarebro commented 2 years ago

Hmm, there are a couple of things going on here. First, do you have multiple monitors? Or just the one 1080p monitor?

We enable HiDPI if it's available. Furthermore, we have a naive scaling that shouldn't exceed 40% of max(monitor width, monitor height).

https://github.com/mgbellemare/Arcade-Learning-Environment/blob/0af0ff4a49a981d113c67b866fa152dbc7a1c0a7/src/common/ScreenSDL.cpp#L170-L193

I'm not sure GDK_SCALE or GDK_DPI_SCALE will work with SDL (SDL doesn't use the GDK backend). Let me know if you have multiple displays, if not we can try to troubleshoot this further.

floringogianu commented 2 years ago

Thank you for the quick reply!

Yes, I was testing this with a dual monitor setup, the external is 1920x1200 but still not HiDPI. I can confirm the window size is correct with just one display.

I guess in the snippet above SDL_GetCurrentDisplayMode, mode.w might be the combined resolution of the two monitors?

JesseFarebro commented 2 years ago

@floringogianu it's very likely I should be using SDL_GetDesktopDisplayMode instead of SDL_GetCurrentDisplayMode. I don't currently have access to a multi-monitor setup but this is on my list of things to tackle for the next release.

floringogianu commented 2 years ago

I just compiled the project locally (from master) in order to investigate the issue further and try your suggestion of using SDL_GetDesktopDisplayMode instead of SDL_GetCurrentDisplayMode.

Surprisingly, the window was now properly sized (I was loading ale-py directly from the build directory using PYTHONPATH). I then installed the package properly, using pip install . and again, the window was sized properly on my dual-monitor setup.

~So I guess that between 0.7.3 and the current master this was fixed? I looked through the commits but couldn't find anything related.~

edit: I just checked and when installing 0.7.4 from PyPI, which I assume comes with precompiled binaries, this issue crops up again. It's fine if you install from source though. Sound support also works only when installing from source.

JesseFarebro commented 2 years ago

Thanks for investigating this @floringogianu! So you think the issue has to do with the precompiled SDL we ship with the Python wheels? I wish there was a way I could debug this but I don't have access to the appropriate setup. I'll circle back and see if I can find any more info now that we've narrowed it down.

JesseFarebro commented 2 years ago

@floringogianu could you tell me what version of SDL you were using when building locally?

JesseFarebro commented 2 years ago

@floringogianu can you try this build: https://test.pypi.org/project/ale-py/0.7.5/ and see if it resolves the issue?

pip install -i https://test.pypi.org/simple/ ale-py==0.7.5
floringogianu commented 2 years ago

@JesseFarebro Sorry for the late reply, I somehow missed the notifications.

could you tell me what version of SDL you were using when building locally?

Between then and now I upgraded my ubuntu distribution, but I recreated the steps above. The problem persists on 0.7.4 with precompiled SDL. When building locally the current SDL version appears to be 2.0.20, the latest.

can you try this build: https://test.pypi.org/project/ale-py/0.7.5/ and see if it resolves the issue?

pip install -i https://test.pypi.org/simple/ ale-py==0.7.5

I don't have much experience with pypi test env and I'm not sure how to get over this error:

ERROR: Could not find a version that satisfies the requirement ale-py==0.7.5 (from versions: 0.6.1.dev51+838fca27a7ccbdeda3999f9f32412d2d1a33af33, 0.6.2.dev78+ed003a945392c44cff7c02342b3ff2551c14c1cc, 0.6.2.dev79+556dde24ca4a2d4dce9adb02c32e24d9a2cdea55, 0.6.2.dev20201228+a6874be, 0.6.2.dev20201228+c770c01, 0.6.2.dev20201228+cd625e0, 0.6.2.dev20201229+87db156, 0.6.2.dev20210719+108713e, 0.6.2.dev20210719+a2ba9a7, 0.6.2.dev20210719+f80952e, 0.6.2.dev20210719+3547358, 0.6.2.dev20210801+61cfedc, 0.7.dev20210823+e8ba878, 0.7rc0+e8ba878, 0.7rc1+c02550f, 0.7rc2+c81b5fe, 0.7rc3+baa2e06, 0.7rc4+5b4e75e, 0.7+6d3b8ab, 0.7+80025fd, 0.7+a54a328, 0.7+d213611, 0.7.1rc0+dd9a841, 0.7.1+b7b0c1a, 0.7.1+dd9a841, 0.7.2+a7a216c, 0.7.3+978d2ce, 0.7.4+069f8bd, 0.7.4+286bdea, 0.7.4+3e89471, 0.7.4+480d996, 0.7.4+732fbdc, 0.7.4+fe6e0d9, 0.7.5+30f56cf, 0.7.5+7e27732, 0.7.5+bfdd2b6, 0.7.5+db37282, 0.7.5+f3e60d1)
ERROR: No matching distribution found for ale-py==0.7.5
JesseFarebro commented 2 years ago

@floringogianu sorry, you can install ale-py==0.7.5 from the release PyPi repository. Just pip install ale-py==0.7.5.

floringogianu commented 2 years ago

Nope, the problem persists. Let me know what else I can do to help with this.

JesseFarebro commented 2 years ago

@floringogianu I think SDL 2.0.18 will fix this issue. There's some other build-related issues when upgrading so I'll have to fix those first. I'll update you once I have a new wheel for you to try out. Thanks for your patience!