garbear / xbmc

XBMC Main Repository
http://xbmc.org
Other
134 stars 53 forks source link

Windows renderer: Nearest neighbor video scaling #85

Closed VelocityRa closed 7 years ago

VelocityRa commented 7 years ago

This PR implements nearest neighbor video filtering/scaling for the Pixel Shader renderer used on Windows.

Description

Nearest neighbor scaling is the conventional scaling used in emulators for older systems, as it results in the most accurate and crisp image. The lack of it was the first thing I noticed when I initially tried RetroPlayer, so I decided to take a crack at it.

It's implemented on Linux, but for windows it was only implemented in the software renderer. DXVA and the Pixel Shader renderer didn't support it. This PR implements it for the Pixel Shader renderer. It's not needed for DXVA (would be pretty hacky anyway; it doesn't seem to allow you to change the scaling), since we'll probably ditch it for RetroPlayer.

For a few implementation details, here's a copy of the main commit message:

Motivation and Context

For RetroPlayer, this was without a doubt needed eventually. Low res games were really ugly/smudgy with the default filtering. With nearest neighbor the image looks as crisp as it should.

How Has This Been Tested?

Tested in RetroPlayer by loading a Game Boy emulator and a game. The difference is quite obvious :)

Screenshots (if appropriate):

Before: image

After: image

(more obvious full-screen)

Types of change

Checklist:

VelocityRa commented 7 years ago

Handled your remarks, capitalized the getColorRange function and moved its implementation to the source file. Btw, the function needed to return

const std::vector<const float>&

for your last remark to compile.

garbear commented 7 years ago

Good job on the update, now the only comment I have left is the use of m_isNearest. Is this because of the extra define when using NN?

VelocityRa commented 7 years ago

I think it's needed because we need to keep this state to detect/compare (Render -> HandleScalerChange) whether a relevant change in the settings has occured. It's indeed possible that we already have this information from someplace else, however. I'll check later to make sure.

VelocityRa commented 7 years ago

Yeah m_isNearest is needed. It's the simplest solution, the alternative (if it even works) would be pretty hacky. Edit: MSVC decided it has problems with const in the vector now. Fixed it.