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(client): Client-side Snapdragon Game Super Resolution #2723

Merged
merged 18 commits into from
Mar 1, 2025

Conversation

zeroxoneafour
Copy link
Contributor

Could offer improvement in visual quality, especially around the edges. I don't really know how to fine-tune the settings for SGSR so more improvement could probably be made on top of upscaling already. Of course this is toggleable too so shouldn't be a big issue if it doesn't look better for a lot of people.

@zeroxoneafour zeroxoneafour changed the title Client-side Snapdragon Game Super Resolution feat(client): Client-side Snapdragon Game Super Resolution Feb 23, 2025
@zmerp
Copy link
Member

zmerp commented Feb 23, 2025

There are a few problems with this PR:

  • Since you don't seem to increase the target resolution, the effect will probably just be slight sharpening instead of upscaling.
  • If you're not increasing the resolution, it should not be done on the client side, because every compression and foveated encoding artifact will be accentuated.
  • Don't hijack the staging pass, instead convert the shader code to wgpu and put the code in the stream pass.
  • You should probably expose more parameters from the shader instead of hardcoding them.

If you intended to just using this as a sharpening algorithm, move it to the server side. If instead you want to upscale the image you need to increase the final resolution.

@zeroxoneafour
Copy link
Contributor Author

I moved the shader to the stream pass and exposed more parameters. Staging resolution is now also scaled up, but I'm not 100% sure if thats the right way to go about it. The problem I'm running into is passing the original texture dimensions into the wgsl from the staging resolution before scaling it up (ORIGINAL_TEXTURE_WIDTH and ORIGINAL_TEXTURE_HEIGHT) for some reason, probably mistyped something but I can't figure it out for the life of me.

@zeroxoneafour
Copy link
Contributor Author

Ok I think it's actually working now. An fxaa pass might be needed before using sgsr as it's hard to see tangible improvements especially on the edges of the screen below a 2x dimension multiplier, but it should work good enough without it. Might try using 3-4x "ultra performance" and see if that works any better without burning my headset to a crisp.

@zeroxoneafour
Copy link
Contributor Author

So as it turns out using 4x dimensional multiplier on a Quest 3 does in fact burn it to a crisp. It could be left in in case someone is using ex. potato settings with an AVP, for some reason.

@zmerp
Copy link
Member

zmerp commented Feb 23, 2025

Yeah, i am worried about the headset not handling the final resolution. 4x actually means 16x the amount of pixels to render, which would definitely not work on any current headset. I think Virtual Desktop handles this by reinterpreting the resolution selector by including the scale factor of upscaling, which means that with same resolution, upscaling would reduce bitrate usage. This is basically what we are doing with foveated encoding. It's not trivial to choose what to do, any opinions from others? @Meister1593 @The-personified-devil

@zeroxoneafour
Copy link
Contributor Author

I tried playing bonelab on 2x upscale factor/low resolution and client fps interestingly dropped to 45. It seems that 2x is the limit to what Quest 3 can do without being put into some form of ASW. Pc fps stayed stable at 90. Maybe some sort of scaling based on target resolution should be used instead, but this could limit certain outlying setups with large disparities between their headset and PC power.

@zmerp
Copy link
Member

zmerp commented Feb 24, 2025

I suggest doing what i said, that is keeping the same final resolution and scale down the transcoding resolution. Also you're saying the client drops to 45 fps, what is the client compositor latency in the graphs (the red strip)?

@zeroxoneafour
Copy link
Contributor Author

screenshot

Client compositor latency is still fairly low (averaging around 1ms). In my experience, the system latency goes up rather than the client latency for whatever reason. Foveated rendering client-side seems to fix it.

As for scaling down the transcoding resolution, I don't really think that's a good approach. Virtual Desktop, according to their discord, implements SGSR by rescaling whatever input they get in to the headset panel resolution. The game runs at whatever resolution is selected, and the upscaler just adds a bit on top of it. This is what I set out to do by introducing upscaling, and in order to keep it independent of SteamVR, I think it's better treated as an addition on top of whatever resolution you're running for a bit more PC performance. The slider of course adds a bit more user configurability to the upscaler, but silently changing the render resolution when using upscaling may cause upscaled output to appear worse from the lower res.

@zmerp
Copy link
Member

zmerp commented Feb 24, 2025

Ok. If you think that it causes non trivial performance issues, then let's keep it off by default as you're doing. but if instead we would keep it on by default then we would probably need to scale down the default resolution presets

@zeroxoneafour
Copy link
Contributor Author

I think the biggest problem here is that there's no default setting that would work well because it's dependent on both render resolution and headset speed. I tested with a Quest 3 on low res/90hz, so a Quest 2 would probably only benefit from 1.3-1.5x while an AVP on the same settings could probably push 3x and still see gains. The most important thing is probably just being explicit in the tooltips/guides so users know how to tweak the settings to their hardware.

@zmerp
Copy link
Member

zmerp commented Feb 25, 2025

For foveated encoding we decided to have it enabled by default, as that would only reduce the transcoding resolution, and while we found it caused some issues on low spec android phones (when used for PhoneVR), and then thankfully fixed, we found that generally having it enabled by default was the better choice. I say that if the performance issues are only dependent on final resolution (and not resolution-independent shader issues) we should think of reducing the transcoding resolution proportional to the upscaling increase, like foveated encoding, and enabling upscaling by default. If you say that the performance issues you face are unique to the upscaling algorithm (beyond an equivalent native resolution), then let's keep it off.

In any case, the resolution and framerate defaults are themselves already a compromise between different headsets hardware, this is nothing new. We should just be careful of not having defaults that make the experience unusable on low spec headsets

@zeroxoneafour
Copy link
Contributor Author

Yeah I think off by default is the best solution. ALVR already pushes the client, this is just something to try if you think you have a bit of extra client side processing headroom but your PC is maxing out. Plus some people are more sensitive to upscaling artifacts than they are to blurred screens.

@zmerp
Copy link
Member

zmerp commented Feb 25, 2025

Sounds good to me. Does VD default to it on or off?

@zeroxoneafour
Copy link
Contributor Author

Off I believe, it also doesn't have any of these config options and it limits you from using it to upscale/sharpen on resolutions that are too high. In VD its just a toggle.

@Meister1593
Copy link
Collaborator

Yeah, i am worried about the headset not handling the final resolution. 4x actually means 16x the amount of pixels to render, which would definitely not work on any current headset. I think Virtual Desktop handles this by reinterpreting the resolution selector by including the scale factor of upscaling, which means that with same resolution, upscaling would reduce bitrate usage. This is basically what we are doing with foveated encoding. It's not trivial to choose what to do, any opinions from others? @Meister1593 @The-personified-devil

Not sure if i have much to say about this to be completely honest... As someone who doesn't know much about this feature, i think it needs to be properly explained what it does, how to use and what implications it has. Notice (yellow box) would be nice for sure.

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.

Overall looks good to me. You should also rebase from master, there is a conflict

@zeroxoneafour
Copy link
Contributor Author

Rebased and squashed. Should be good to go

@zmerp
Copy link
Member

zmerp commented Feb 27, 2025

A lints need to be fixed

@zeroxoneafour zeroxoneafour requested a review from zmerp February 27, 2025 23:52
zmerp
zmerp previously approved these changes Feb 28, 2025
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.

I just tested this, the effect is great, although this also sharpens the pixelation of the foveation areas. I understand that this is not easy to fix, we would probably need to implement a separate pass. I think this is usable and useful as it is, any improvements can be done later.

I'll wait another day before merging if anyone else wants to chime in for a review

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 didn't really manage to get a good overview of all the changes, but apart from those two things it lgmt.

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.

Lgtm, tho not a thorough review

@zmerp
Copy link
Member

zmerp commented Mar 1, 2025

LGTM. Any more fixes can be done in a future PR

@zmerp zmerp merged commit 4db9b86 into alvr-org:master Mar 1, 2025
9 checks passed
zmerp pushed a commit that referenced this pull request Mar 9, 2025
* add broken sgsr

* working now

* Rebase with SGSR

* renamed some values and it stopped working

* minor rework but still nroken

* trying to pass upscale settings into alvr

* upscaling achieved yippee

* forgot to pass upscale factor oops

* allow excessive upscale factor (not necessarily recommended)

* better defaults centered around FSR balanced

* default to 1.5x dimensional, better factor tooltip, use builtin as_vec2

* flattened shader code and addded scale_resolution() to handle upscaling calculations

* fix up functions

* adjust resolution handling

* increase upscale power on edges when foveated rendering

* ok rust

* add help to edge direction
@zmerp zmerp mentioned this pull request Mar 9, 2025
zmerp pushed a commit that referenced this pull request Mar 11, 2025
* add broken sgsr

* working now

* Rebase with SGSR

* renamed some values and it stopped working

* minor rework but still nroken

* trying to pass upscale settings into alvr

* upscaling achieved yippee

* forgot to pass upscale factor oops

* allow excessive upscale factor (not necessarily recommended)

* better defaults centered around FSR balanced

* default to 1.5x dimensional, better factor tooltip, use builtin as_vec2

* flattened shader code and addded scale_resolution() to handle upscaling calculations

* fix up functions

* adjust resolution handling

* increase upscale power on edges when foveated rendering

* ok rust

* add help to edge direction
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