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

Resolution scaling hotkeys #3185

Merged
merged 22 commits into from
Jul 24, 2022
Merged

Resolution scaling hotkeys #3185

merged 22 commits into from
Jul 24, 2022

Conversation

MutantAura
Copy link
Collaborator

Pretty simple.

PageUp = Increase resolution scale by a factor of 1.
PageDown = Decrease resolution scale by a factor of 1.

Scaling up is capped to 4x scaling and scaling down is limited to a lowest value of 1x. Custom scaling values are also affected by this. For example a custom scale of 6x will be able to be stepped down with PageDown but cannot be increased with PageUp. Likewise a scale of 0.2x will scale to 1x with PageUp but will not scale to 0x with PageDown.

This is a runtime change and does not update the configuration file etc.

@Niwu34
Copy link
Contributor

Niwu34 commented Mar 8, 2022

Works very well - Thx Mutant!
https://user-images.githubusercontent.com/67392333/157248327-ff7adbdc-b2b9-47fd-b9df-bd92b9777dce.mp4
(not going to uplaod it somewhere else as it would temper with the video quality)

@gdkchan
Copy link
Member

gdkchan commented Mar 8, 2022

What if I want to map PageUp/PageDown as regular controller buttons?

@MutantAura
Copy link
Collaborator Author

What if I want to map PageUp/PageDown as regular controller buttons?

Is this not an issue with every hotkey? Function keys are less of a problem but I don't think they're too intuitive for two hotkeys that are so clearly designed to be used together. I'm open to suggestions but I don't think say F9 and F10 would be that great...

Ultimately this is always going to be a problem for specific users until hotkeys themselves are configurable.

@gdkchan
Copy link
Member

gdkchan commented Mar 8, 2022

It would be better to make them configurable. I think Fn as hotkeys is generally fine as that's their intended use. The only exception right now is Tab used to toggle VSync (a poor choice imo but that's another issue). I brought that up because I usually map SL/SR of left and right joycons to PageUp/PageDown and Home/End, and people doesn't seems to take in consideration the ones playing on keyboard when doing those changes, so I felt like it was worth it.

Btw those hotkeys are not really discoverable right now, so one would most likely find them by accident (and then start wondering what happened, why their game is suddenly low res or why it crashed due to it running out of memory with a res that's higher than their hardware supports).

@gdkchan gdkchan added enhancement New feature or request gui Related to Ryujinx.Ui labels Mar 8, 2022
Ryujinx/Ui/RendererWidgetBase.cs Outdated Show resolved Hide resolved
Ryujinx/Ui/RendererWidgetBase.cs Outdated Show resolved Hide resolved
Ryujinx/Ui/RendererWidgetBase.cs Outdated Show resolved Hide resolved
Ryujinx/Ui/RendererWidgetBase.cs Outdated Show resolved Hide resolved
@yell0wsuit
Copy link
Contributor

yell0wsuit commented Mar 9, 2022

Btw those hotkeys are not really discoverable right now, so one would most likely find them by accident (and then start wondering what happened, why their game is suddenly low res or why it crashed due to it running out of memory with a res that's higher than their hardware supports).

This. IMO RS is not a general-use function that should be bound to fixed hotkeys (esp. low-end users as they don't want to touch these hotkeys). If this got merged, I'd expect a whole bunch of issues to disable those hotkeys. I'd rather have configurable hotkeys for this.

@MutantAura MutantAura marked this pull request as draft March 9, 2022 15:35
@MutantAura
Copy link
Collaborator Author

Drafting until I decide what scaling functionality this should have and also until the hotkey issue has been sorted. Likely after hotkeys become configurable as I agree having this set by default could cause more issues than the functionality would add.

@MutantAura MutantAura force-pushed the res-hotkeys branch 2 times, most recently from 9fbab12 to 615c650 Compare July 12, 2022 18:03
@MutantAura MutantAura marked this pull request as ready for review July 12, 2022 18:26
@MutantAura
Copy link
Collaborator Author

Rebased and updated with Avalonia support.

Now using implementation outlined by @jduncanator above for more compact code and a more intuitive outcome from each keypress. Default hotkey for this is Unbound on both Avalonia and GTK but can be bound either through the GUI with Ava or via the config file with GTK (unsure if keeping the GTK Imp was even worth it?).
Locales were added via DeepL: https://www.deepl.com/translator and as such may not be accurate to a native speaker. Feedback is appreciated here! Locales that had no support for hotkey localisation in the first place (French etc.) haven't been localised and should be tackled in a larger locale update for those languages.

Ready for review.

Ryujinx.Ava/AppHost.cs Outdated Show resolved Hide resolved
Ryujinx/Ui/RendererWidgetBase.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@marysaka marysaka left a comment

Choose a reason for hiding this comment

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

lgtm, only one thing

Ryujinx.Ui.Common/Configuration/ConfigurationState.cs Outdated Show resolved Hide resolved
@marysaka marysaka added the review/waiting-on-author Waiting on the contributor for a PR label Jul 23, 2022
Copy link
Member

@gdkchan gdkchan left a comment

Choose a reason for hiding this comment

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

lgtm, thanks.

@gdkchan gdkchan merged commit 62f8ceb into Ryujinx:master Jul 24, 2022
@MutantAura MutantAura deleted the res-hotkeys branch July 24, 2022 18:45
@marysaka marysaka removed the review/waiting-on-author Waiting on the contributor for a PR label Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gui Related to Ryujinx.Ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants