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

switch: use native screen resolution and remove old workarounds #373

Closed
wants to merge 6 commits into from

Conversation

p-sam
Copy link
Contributor

@p-sam p-sam commented Oct 10, 2023

I had to apply the same kind of workaround needed for the meson bug encountered with libzstd. Most Mesa and libnx bugs we encountered have since been fixed, so I tried removing quirks and ifdef'd code in the switch code.

Also, the SDL window size will now adjust to the native resolution of the screen you're playing on (tv, handheld). This is mostly handheld by the switch SDL port under the hood, which resizes the window if not flagged fullscreen.

src/video.c Outdated
Comment on lines 779 to 785
// Catch resizes by the SDL portlib itself, when the console is docked/undocked
if(video_get_backend() == VIDEO_BACKEND_SWITCH) {
video_handle_resize(event->window.data1, event->window.data2);
}
Copy link
Member

Choose a reason for hiding this comment

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

I believe SDL must generate a SDL_WINDOWEVENT_RESIZED for this.

Documentation for SDL_WINDOWEVENT_SIZE_CHANGED from https://wiki.libsdl.org/SDL2/SDL_WindowEventID:

window size has changed, either as a result of an API call or through the system or user changing the window size; this event is followed by SDL_WINDOWEVENT_RESIZED if the size was changed by an external event, i.e. the user or the window manager

In other words, SDL_WINDOWEVENT_SIZE_CHANGED always happens, followed by SDL_WINDOWEVENT_RESIZED in pretty much every case except SDL_SetWindowSize called by the application.

Can you please try to get it fixed on their end? I don't mind keeping this workaround until that's done, but I'd like to see a bug report link in the comment at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I now have an issue on their fork for that, and will let you know if anything happens

src/video.c Show resolved Hide resolved
src/config.c Outdated Show resolved Hide resolved
config_set_int(CONFIG_VID_WIDTH, nxGetInitialScreenWidth());
config_set_int(CONFIG_VID_HEIGHT, nxGetInitialScreenHeight());
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what this is for. If SDL is supposed to automatically resize the window based on the docked/undocked state, it should enforce the correct size when the window is created too. Does it not do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it doesn't, because Switch SDL port only handles the resize after the proper event, and not initially, but that's probably irrelevant since the width/height present in the config is used to create the window anyway, which would be whatever resolution was the native when Taisei was last launched

Copy link
Member

Choose a reason for hiding this comment

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

This falls under the same category as the SDL_WINDOWEVENT_SIZE_CHANGED thing for me… I'm pretty unhappy about this strange implementation choice in the SDL port and would consider it a bug, but I'm ok with keeping this workaround until it's fixed, if ever.

meson.build Outdated
@@ -208,7 +208,7 @@ dep_webp = dependency('libwebp', version : '>=0.5', required : t
dep_webpdecoder = dependency('libwebpdecoder', version : '>=0.5', required : false)
dep_zlib = dependency('zlib', required : true)
dep_zstd = dependency('libzstd', version : '>=1.4.0', fallback : ['libzstd', 'libzstd_dep'])
dep_zip = dependency('libzip', version : '>=1.5.0', required : opt_vfs_zip, allow_fallback : true)
dep_zip = dependency('libzip', version : '>=1.5.0', required : opt_vfs_zip, fallback : ['libzip', 'libzip_dep'])
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what was the problem here and/or do you have a failed build log?

I think the problem with zstd was related to meson.override_dependency() misbehaving when the dependency is needed for both native and cross targets, but I don't remember the details. I wouldn't think it's the same problem here; IIRC libzip is only used by taisei itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The relevant error line in the build log was:

WARNING: Subproject 'libzip' did not override 'libzip' dependency and no variable name specified

I eventually traced back the error to an issue you posted on the meson repo which led me to believe this was the same problem.

Copy link
Member

Choose a reason for hiding this comment

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

Was that built in a pristine container or just locally? In the later case you might have an older version of libzip checked out under subprojects, which doesn't call meson.override_dependency(). See if it still happens after a meson subprojects update or just rm -rf subprojects/libzip.

But if this happens under a totally clean checkout, that is very odd and needs further investigation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not happen after a clean checkout, so I removed the change from the PR

@Akaricchi Akaricchi changed the base branch from master to staging November 27, 2023 04:35
@Akaricchi
Copy link
Member

Massaged this a little bit and merged manually into staging

@Akaricchi Akaricchi closed this Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants