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

PlatformCore/Qt: Add sharp bilinear shader #340

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

GranMinigun
Copy link
Contributor

Does what it says on the tip. Helps with pixel irregularities at non-integer scaling factors, especially on lower output resolutions. Looks identical to nearest-neighbour interpolation at integer scaling factors and is not recommended to use with such configuration. Fairly cheap. Ported from RetroArch Slang shader collection. Tested in combination with all other shaders. Config and enum names are up for debate.

@fleroviux
Copy link
Member

Thanks! This is working well on my end and the code is looking good to me. The only thing I am worried about is that it might be hard for users to tell the difference between nearest and sharp, especially when tested at integer scaling factors. Do you think it would make sense to deprecate the "nearest" option in favor of a "sharp" option which for efficiency falls back to the nearest code path at integer scales?

@GranMinigun
Copy link
Contributor Author

I personally would prefer not to introduce any additional heuristics, to keep the code simpler. At this point, I think the shader is widespread and known enough to trust the user with a choice (see, e.g., ares, DOSBox, most recently Dolphin). And the cost of a mistake isn't that high — the shader runs just fine on mobile devices and SBCs like Raspberry Pi 4. Changing the default shader might be a better overall option.

@fleroviux
Copy link
Member

Ah, alright. That's fair. I will go ahead and merge the PR then.

@fleroviux fleroviux merged commit b2a2cfd into nba-emu:master Dec 13, 2023
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.

2 participants