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

refactor(dashboard): dashboard improvements, codec & foveation presets #2704

Merged
merged 37 commits into from
Mar 1, 2025

Conversation

Meister1593
Copy link
Collaborator

@Meister1593 Meister1593 commented Feb 15, 2025

  1. Improvements to sign about not running steamvr (easier to understand)
  2. Added more notes to various places in dashboard and changed strings (statistics without connected client, debug tab for recording buttons, changed few notifications and added one more)
  3. Added presets for foveation and codec
  4. Adjusted all presets to go from low-to-high (or first disabled, then others)
  5. Added a note to foveated encoding (to prevent users from disabling them without knowing the risks)
  6. (internal) Higher order choice now uses display name for default value instead of indexes
  7. Nvidia icd vulkan paths are now checked and chosen from two available ones and fallback to env param (i thought about finding first available, but using 32 bit vulkan icd most likely will break steamvr, playing it safe here)
  8. Microphone on windows now disabled by default

@Meister1593 Meister1593 added the enhancement New feature or request label Feb 15, 2025
@Meister1593 Meister1593 self-assigned this Feb 15, 2025
@Meister1593
Copy link
Collaborator Author

Meister1593 commented Feb 15, 2025

Closes #2189

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

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

beside the comments below, you have clippy and format to fix

@Meister1593 Meister1593 marked this pull request as draft February 16, 2025 11:22
@Meister1593 Meister1593 requested a review from zmerp February 16, 2025 12:23
@Meister1593 Meister1593 marked this pull request as ready for review February 16, 2025 12:24
@Meister1593 Meister1593 changed the title WIP: Dashboard improvements refactor: general dashboar improvements, codec & foveation presets Feb 16, 2025
@Meister1593 Meister1593 changed the title refactor: general dashboar improvements, codec & foveation presets refactor: general dashboard improvements, codec & foveation presets Feb 16, 2025
Copy link
Collaborator

@The-personified-devil The-personified-devil left a comment

Choose a reason for hiding this comment

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

I'm aware english is far from your first language, but running updated help strings through a grammar checker might be worthwhile as badly worded ones can make it even harder to understand what to do, especially if a user also isn't that good at english.

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

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

This PR still needs fixing some lints, and replacing the critical tooltip with notice. I can work on notice first in another PR then you can rebase

@zmerp
Copy link
Member

zmerp commented Feb 23, 2025

I was meaning to do a PR but I pushed to master by mistake... in any case this PR can be rebased, and use the new notice field string in place of the critical tooltip

@Meister1593
Copy link
Collaborator Author

Meister1593 commented Feb 27, 2025

I haven't noticed any issues with rounding that is caused by this PR, and actually for ex. latency overstep modifier being set to 0.999999 is fine, but UI rounds it to 1.0, only when i set it to 0.9999999 it rounds to 1.0 through rounding with this code

@The-personified-devil
Copy link
Collaborator

The-personified-devil commented Feb 27, 2025

@zmerp You're right actually, however there's a simple workaround where it'll actually round to the number of valid places, not decimals (I got confused prior by assuming we were using this mode actually). See this example. This however does break larger integers at a certain point, so as I already mentioned we need to treat those separately. For floats however this should be perfect. However this should only be used in the comparison, not in the rounding when storing (which is kinda useless in the first place, but shouldn't really matter all things considered).

@zmerp
Copy link
Member

zmerp commented Feb 27, 2025

Good call, we should use the .Xe notation then, and exclude integers

@zmerp zmerp force-pushed the dashboard_improvements branch from 39fee93 to a5f6b3d Compare February 27, 2025 19:48
@zmerp zmerp force-pushed the dashboard_improvements branch from a5f6b3d to bb65c25 Compare February 27, 2025 19:51
@zmerp zmerp force-pushed the dashboard_improvements branch from 62fc5f3 to d65f13b Compare February 27, 2025 23:11
@zmerp zmerp force-pushed the dashboard_improvements branch from d65f13b to 85b6031 Compare February 27, 2025 23:16
@zmerp
Copy link
Member

zmerp commented Feb 27, 2025

@The-personified-devil @Meister1593 Let me know if this is mergeable. I am aware of the disagreement on how to handle float comparison. The current method is affected by these issues: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=5657af39041c776b9957976adaa93a53
But for what we want, we shouldn't need a workaround for this, potentially with a non-negligible cost, any time soon.

@Meister1593
Copy link
Collaborator Author

I'm okay with any solution that works with that really, comment should suffice for the time being

@The-personified-devil
Copy link
Collaborator

Only that one thing btw, after that the pr is fully done and ready to merge.

@zmerp
Copy link
Member

zmerp commented Mar 1, 2025

The value equality check should be fixed for real now

@The-personified-devil The-personified-devil self-requested a review March 1, 2025 01:36
@zmerp zmerp force-pushed the dashboard_improvements branch from ca88d40 to b85e8b7 Compare March 1, 2025 02:09
@zmerp zmerp merged commit 4848f2f into master Mar 1, 2025
9 checks passed
@zmerp zmerp deleted the dashboard_improvements branch March 1, 2025 14:14
@zmerp zmerp mentioned this pull request Mar 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants