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

feat(display): add display mode remapping option #3529

Merged
merged 5 commits into from
Jan 12, 2025

Conversation

FrogTheFrog
Copy link
Collaborator

Description

This is part 3 of #3441.

This PR adds an option to remap requested resolution and/or FPS to another resolution and/or refresh rate.

Screenshot

When remapping is possible for both:
image

When remapping is possible for only resolution:
image

When remapping is possible for only FPS:
image

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Copy link

codecov bot commented Jan 11, 2025

Codecov Report

Attention: Patch coverage is 69.72477% with 33 lines in your changes missing coverage. Please review.

Project coverage is 8.84%. Comparing base (012a99c) to head (6dd9b97).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/config.cpp 0.00% 7 Missing and 12 partials ⚠️
src/display_device.cpp 85.39% 0 Missing and 13 partials ⚠️
src/config.h 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master   #3529      +/-   ##
=========================================
+ Coverage    8.45%   8.84%   +0.39%     
=========================================
  Files          90      90              
  Lines       16054   16159     +105     
  Branches     7626    7679      +53     
=========================================
+ Hits         1357    1430      +73     
- Misses      12212   12219       +7     
- Partials     2485    2510      +25     
Flag Coverage Δ
Linux 9.67% <62.26%> (+0.50%) ⬆️
Windows 6.74% <70.37%> (+0.55%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/config.h 0.00% <0.00%> (ø)
src/display_device.cpp 59.67% <85.39%> (+9.89%) ⬆️
src/config.cpp 0.00% <0.00%> (ø)

@FrogTheFrog
Copy link
Collaborator Author

@moi952 could I ask you to please test this build. Just if the remapping still works? I don't have access to host PC at the moment :/

@moi952
Copy link

moi952 commented Jan 11, 2025

@moi952 could I ask you to please test this build. Just if the remapping still works? I don't have access to host PC at the moment :/

Yes of course, I'm not at home, but I can test when I get back if it's not too late, or tomorrow morning.
Thanks

@moi952
Copy link

moi952 commented Jan 11, 2025

@moi952 could I ask you to please test this build. Just if the remapping still works? I don't have access to host PC at the moment :/

I just got back, it works perfectly

@RebelliousX

This comment was marked as off-topic.

@FrogTheFrog

This comment was marked as off-topic.

@RebelliousX

This comment was marked as off-topic.

@moi952

This comment was marked as off-topic.

@RebelliousX

This comment was marked as off-topic.

@ReenigneArcher
Copy link
Member

Thank you everyone for testing and giving feedback on this PR, but let's not make these PRs grow too large. It causes burnout for both the contributors and maintainers. The UI can be improved in a later PR, and this is not a small ask since we don't have any patterns in place for resolution inputs currently. The old resolution input also did not have any input validation and strictly relied on the user entering the values correctly.

@FrogTheFrog
Copy link
Collaborator Author

FrogTheFrog commented Jan 12, 2025

As RA has stated, this will be UI for the 1st iteration.

You are welcome to improve it later on as I'm already burned out and no longer plan to work on a feature that I would not be even using in the end.

EDIT:
Don't get me wrong, this is all a valid feedback and suggestions, but it does not take minutes, but hours to properly implement (also there is no easy way to get all available display modes without involving DXGI). However, it all starts to feel like a job at this point.

@ReenigneArcher ReenigneArcher added this to the stable release milestone Jan 12, 2025
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 New issues
2 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@ReenigneArcher ReenigneArcher merged commit 1b94e93 into LizardByte:master Jan 12, 2025
35 of 37 checks passed
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.

4 participants