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

Video: implement output resampling (upscaling/downscaling) methods #11999

Merged
merged 5 commits into from Aug 18, 2023

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Jun 27, 2023

Follow up to:
#11850

This PR implements custom upscaling/downscaling techniques in our default post process pass.
By default no custom resampling is done, Dolphin just falls back on the GPU hardware (bi)linear sampling (which has no effect unless the emulation output (XFB) resolution doesn't match the window resolution).
If the user selects any resampling method other than default, then the default post process pass will be executed, and it will do some custom resampling (before any other user selected post process), with gamma correction (colors are converted to linear space before resampling).

I have implemented a multitude of algorithms, all of them have been rigorously tested and checked against each other and against many other code bases (e.g. Cemu, Yuzu, DXVK, github random samples, ...).
There's no "best" resampling method so each user can select what they prefer depending on the case.
image

Given that resampling is optional and that Dolphin doesn't have major performance issues, the shaders might not be as optimized as they could, but for now I'd say there isn't much to worry about.
Everybody is welcome to add new resampling methods, and the Catmull-Rom is basically a template for any other bicubic algorithms.

Upscaling (from 1x):
Bilinear
Bicubic
Hermite
Catmull-Rom
NearestNeighbor
SharpBilinear

Downscaling (from 8x):
Bilinear
Bicubic
Hermite
Catmull-Rom
NearestNeighbor
SharpBilinear

New Box Resampling (for downscaling) by @Sam-Belliveau.
Box
Catmull-Rom
Box might look a bit less "sharp", but it's extremely termporal stable if the source resolution is high enough.

More samples:
#12018

@Filoppi Filoppi force-pushed the post_process_fixes branch 2 times, most recently from 7fe5467 to 530f172 Compare June 27, 2023 23:54
@Filoppi Filoppi marked this pull request as ready for review June 28, 2023 00:26
@Filoppi Filoppi force-pushed the post_process_fixes branch 3 times, most recently from a632745 to 50124b9 Compare June 29, 2023 21:12
@AdmiralCurtiss
Copy link
Contributor

I can't say much about the shader code, but this is a really good improvement to image quality when downsampling, it's always been pretty terrible before. This actually makes resolutions above the screen size worth using.

Copy link
Contributor

@MayImilae MayImilae left a comment

Choose a reason for hiding this comment

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

So I can't actually test this right now, on account of my computer being in pieces atm. But I can just review the UX!

Also it feels really weird to have gamma correction in the dropdown for scaling methods but, if that's how the hardware works then I guess that's that. If you could give me a quick explanation on that I'd appreciate it.

@Filoppi
Copy link
Contributor Author

Filoppi commented Aug 4, 2023

I've merged SamB changes into the branch and solved all the remaining issues mentioned above.
I've unexposed nearest neighbor resampling from users and added a comparison with SSMAA in the tooltip.

New tooltip:
Dolphin_g25nI1o9lO

There is still a minor issue with area resampling where it shifts the output image by half a pixel but it's barely noticeable and SamB will fix that later.
Edit: I submitted a workaround for that, though it's still better if Sam looks at it.

@Sam-Belliveau
Copy link
Contributor

I dont know how much of an improvement the current tooltip is over how it was before?

Before:
image

After:
image

We might wanna think about how to make it clearer?

@Filoppi Filoppi force-pushed the post_process_fixes branch 3 times, most recently from 8b5c927 to ee2a49e Compare August 5, 2023 11:27
Copy link
Contributor

@Sam-Belliveau Sam-Belliveau left a comment

Choose a reason for hiding this comment

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

Looked over the GLSL code and the tool tip.

LGTM

@Filoppi Filoppi force-pushed the post_process_fixes branch 2 times, most recently from 221506b to 8058236 Compare August 6, 2023 14:50
@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Aug 17, 2023

Looks like I introduced a conflict here when I switched the tooltip for the Color Correction button to our fancy ones. Can you rebase?

Filoppi and others added 5 commits August 18, 2023 02:00
…penGL

This was because the shader uniforms between the pixel and vertex shaders
were willingly left different, to avoid filling the vertex shader with unnecessary
params. Turns out all backends are fine with this except OGL.
The new behaviour is now much more consistent and well explained,
the "default" shaders are the ones that always run, and the non default
ones are the user selected ones (if any).
@Filoppi
Copy link
Contributor Author

Filoppi commented Aug 17, 2023

@AdmiralCurtiss done.

@AdmiralCurtiss AdmiralCurtiss merged commit 3441fe6 into dolphin-emu:master Aug 18, 2023
11 checks passed
@Frieslover69
Copy link

I've unexposed nearest neighbor resampling from users and added a comparison with SSMAA in the tooltip.

Hi @Filoppi, first of all thank you for adding this awesome feature! Being able to select the output filtering modes is really something every emulator should have imho.
Speaking of basic filtering modes, is there a reason why you decided to remove the nearest neighbor resampling from the dropdown? I know some could argue that sharp bilinear or area sampling would make it obsolete, but some users (such as myself 😄) might want to enjoy the games on nearest neighbor (a.k.a. no filtering). The reason for this is that area sampling still introduces some "ghosting" (for a lack of better words) to the pixels of the image. Whereas nearest neighbor filtering truly gives you the pure unadulterated expierience of absolute sharp edges in all cases. Having the option to basically selecty no filtering at all would make sense for such a dropdown imho.

So my suggestion would be to maybe add the nearest neighbor filtering back to the menu? All the pixel purists out there would greatly appreaciate it, I'm sure! 🙏

Best regards 😊

@mbc07
Copy link
Contributor

mbc07 commented Jan 4, 2024

@Frieslover69 there's an explanation in this blog post regarding the downsides of a "pure" Nearest Neighbor filter in older consoles like the GC/Wii and that's why we're not providing it as an option...

@Frieslover69
Copy link

@mbc07 Thanks for the reply! Yeah, I'm aware of the downsides of nearest neighbor. But some things might be considered before ultimately abandoning that feature:

  1. The flickering gets less noticable at larger upscales. At some point it becomes even unnoticable! A bump to let's say 150% of the original resolution leads to noticable flickering. That is because the thickness of the columns (as it is called in the blog) alternates between 1 or 2 pixels. One column is twice as thick as the other! But if you bump the resolution to 250% using NN, then the thickness of the column alternates between 2 or 3 pixels. So the "bad" columns are only a third off the good column size. In that case, the flickering is less noticable. Especially on 1440p monitors you'd usually use the NN to scale the original image to even more than 250%, resulting in even less flickering.
    So TL;DR: flickering is no big deal on higher upscales (i.e. bumping from original GC res to 1440p).
  2. Other filters have their downsides too... Some of them might give you an undesirable soft look, but they've been left in nonetheless.
  3. The simple option to have NN wouldn't hurt. If a user is unhappy with the look, they can just choose to select another filter from the list. 😄

So... do you think NN has a chance of getting accepted if someone submits a pull-request? The code for the filter is still in the default_pre_post_process.glsl file, so adding it back would only need a relatively small change. And the first post even says everyone is free to add their own filters. 😊

@Filoppi
Copy link
Contributor Author

Filoppi commented Jan 5, 2024

@Frieslover69 leaving nearest neighbor has been discussed at length and we decided against (I'm not as against it as others). We are aware there are some positives of keeping it, but at the same time, it's also extremely easy for users to re-enable it by just commenting out in default_pre_post_process.glsl (the code is still there, just disabled).
The main drawback of nearest neighbor is that GC and Wii games resolution is almost always scaled from its native aspect ratio to ~4:3 or ~16:9, thus nearest neighbor doesn't make much sense given pixels weren't squared. I suggest using sharp bilinear to achieve the best results.

These kind of discussions are probably best had on discord, join the devs channel there if you want to have further discussion, or just make a PR to add back nearest neighbor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants