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

Add support for resolutions multipliers up to 12x (~8k) #12171

Merged
merged 1 commit into from Oct 6, 2023

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Sep 7, 2023

Simple change to allow resolution multipliers up to 12x.
Qt UI also updated.
Dolphin_T5KCvhD25h
This has been tested on OpenGL, DX11, DX12 and Vulkan.

This especially makes more sense now that we have proper downsampling of the image (area resampling) (#11999). When playing at 12x, the temporal stability is incredibly high, and texture show more detail if we had high resolution texture packs. The performance impact is minimal.

GFX_MAX_EFB_SCALE was not even respected by the "Auto Internal Resolution Scale" setting, so on 8k monitors resolution could already go beyond 8x (to 12x or 13x).
I've made the change to respect it in this PR https://github.com/dolphin-emu/dolphin/pull/12170 so we can be sure we don't try to allocate textures bigger than the maximum supported (and/or tested) texture size.

@Filoppi Filoppi force-pushed the support_8k branch 2 times, most recently from 478ba2d to 9bf73c4 Compare September 9, 2023 12:53
@AdmiralCurtiss
Copy link
Contributor

I'm wondering if your change to how the dropdown text is put together might negatively impact localization.

@mbc07
Copy link
Contributor

mbc07 commented Sep 10, 2023

I'm wondering if your change to how the dropdown text is put together might negatively impact localization.

As the maintainer of one of the translations Dolphin ships with, if I'm reading the code correctly, that change will actually result in less strings to translate compared to the current approach. The added "%1x Native (%2x%3)%4" string even allow reordering the terms more easily, if needed...

@Filoppi
Copy link
Contributor Author

Filoppi commented Sep 10, 2023

I'm wondering if your change to how the dropdown text is put together might negatively impact localization.

I've re-uploaded

@MayImilae
Copy link
Contributor

MayImilae commented Sep 10, 2023

As one of the dozen people with a silly 8k computer monitor, I'm completely fine with this. Like, obviously, I'm the one that requested the max internal resolution setting! I already have 12x native exposed! So, by all means I'm ok with this for me.

However, in addition to the higher resolution than their screen making aliasing worse issue, there was another reason why we didn't expose high internal resolutions to users: lots of users expect emulators to be lightweight, and just immediately crank all settings. We didn't want users with weak computers to crank internal resolution to the maximum and complain about Dolphin being "unoptimized" because their integrated GPU ran out of VRAM. On proper gaming desktop GPUs today, that is not as much of a concern. It is a bit of a concern on integrated graphics however, as RCPS3 and Yuzu have helped with that "emulators are light" notion, it's not as big of an issue as it once was.

But on mobile... yikes. So I'm going to ask the scary question - does Max Internal Resolution setting do anything on Android? If it does, you'll have to jump through a few hoops. We can't reveal 8k on Android. @t895 @JosJuice

@t895
Copy link
Contributor

t895 commented Sep 10, 2023

On Android we already selectively expose resolution options so this shouldn't be a problem.

@Filoppi
Copy link
Contributor Author

Filoppi commented Sep 10, 2023

I doesn't do anything on Android.

@JosJuice
Copy link
Member

The options on Android are always 1x-6x, regardless of the user's settings and regardless of the device. This PR doesn't affect it.

@JMC47
Copy link
Contributor

JMC47 commented Sep 10, 2023

LGTM.

@AdmiralCurtiss AdmiralCurtiss merged commit 1c433d5 into dolphin-emu:master Oct 6, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants