search

NVIDIAGameWorks / NVIDIAImageScaling

NVIDIA Image Scaling SDK
MIT License
507 stars 59 forks source link

NIS 1.0.1 regression vs NIS 1.0.0 #4

Closed CptLucky8 closed 2 years ago

CptLucky8 commented 2 years ago

Hi,

I've noticed a bug introduced since 1.0.1 which is still not fully corrected with the latest commit https://github.com/NVIDIAGameWorks/NVIDIAImageScaling/commit/aa37be760496e6d32a1f2d6b6faba40a00c76a9d

The color planes are not fully aligned and shifting is visible. This was clear with NIS 1.0.0.

Here is a screenshot... In this screenshot we can see the red color plane is not aligned with the others. _NIS scale 66% sharpen 50%_ ![FS2020_20220111_134937_NIS_150_50](https://user-images.githubusercontent.com/96778648/149005473-668dad72-794c-4d3e-a386-c5f2ba189788.png)

Thanks!

abernalnv commented 2 years ago

@CptLucky8, we can't repro de issue. What are the input/output sizes? Are you using HLSL or GLSL? version? Is it in HDR or LDR?

abernalnv commented 2 years ago

Also, could you provide a low resolution input image and your sampler configuration?

CptLucky8 commented 2 years ago

Input is about 2100x2100 and output is about 3100x3100, with HLSL 5 DX11 LDR FP32.

Swapping the shader only using NIS 1.0.0 gives clear results, but since 1.0.1, colour planes are shifting.

When v1.0.1 was first introduced the change was immediately noticeable: all pixels globally shifting in the 135º direction and colour planes separating (we used an app allowing to toggle between bilinear and a NIS). With the v1.0.1 hotfix, I didn't check specifically for the global shifting, but the red colour plane is still separating.

Please note back when v1.0.1 was released, I've tried to find what code changes would be producing this bug. So I've replaced most of the functions one by one in isolation from 1.0.1 code back into 1.0.0 code, but I couldn't pin point anything specifically, except the change in kernel size: in 1.0.0 filters was 8 and support was 6, whereas in 1.0.1 they both are 6. Is it possible the change is making the code logic picking up the wrong cell in shEdgeMap[] or shPixelsY[] ?

CptLucky8 commented 2 years ago

Also, could you provide a low resolution input image and your sampler configuration?

The setup is here: https://github.com/mbucchia/OpenXR-Toolkit/blob/65f8db0bdc79f0d2262c260a6722dc069e6af427/XR_APILAYER_NOVENDOR_toolkit/d3d11.cpp#L620

The initialization is here: https://github.com/mbucchia/OpenXR-Toolkit/blob/65f8db0bdc79f0d2262c260a6722dc069e6af427/XR_APILAYER_NOVENDOR_toolkit/nis.cpp#L144

I'll see what I can do for a low res input image (schedule permitting).

abernalnv commented 2 years ago

Thank you, a low res input will definitely help! What gpu are you using?

CptLucky8 commented 2 years ago

This is on a 2070S with driver 497.09, but IIRC @mbucchia is using a different NVidia card (and most likely a different driver) and he cross-checked with me that he can see the same bug too. It really is happening only since 1.0.1 (both initial and fix). However 1.0.0 is spot-on.

Also note the re-centering fix which was added lately is helping a little bit but doesn't resolve the bug entirely. For example, with 1.0.1 initial, any cyan text was completely changing color and the red shift was as pronounced as in my screenshot. With 1.0.1 fix, the cyan text remains cyan ok, but red is still shifting.

These effects, and especially the cyan invariance with the 1.0.1 fix, is what lead me to think the shEdgeMap/shPixelsY size changes could be a factor in this bug (especially if the coefs are pre-computed for a certain size which is now different - if ever this is related).

If the Shader code is dependent on GPU setup in a way though (sampler, texture format, etc...) I fail to find what we're doing wrong on our code about this, especially because it seems to me the changes between 1.0.0 and 1.0.1 are mostly about 1) using the ternary operator wherever it benefits 2) simplifying some maths and code logic 3) reducing the 'filtersize' dimensions ?!?!

https://github.com/NVIDIAGameWorks/NVIDIAImageScaling/blob/59677206df08743b252999648b11dd45949a69ee/NIS/NIS_Scaler.h#L318

CptLucky8 commented 2 years ago

I've remembered NIS ships with the sample app, so I've tried it out to compare. With this sample app, 1.0.1a and 1.0.0 are visually identical?! (see screenshots below)

So I'll revisit in game (it is in VR) and if this exhibit the problem, I'll try to find a way to capture both screenshots at the same time.

Bilinear ![bil](https://user-images.githubusercontent.com/96778648/149041629-b040bf8f-3ae5-4b5f-b1be-5b20ff789807.png)
NIS 1.0.0 ![nis_1 0 0](https://user-images.githubusercontent.com/96778648/149041705-d5ff2322-a20f-47e3-84c1-edbf1f6e40a0.png)
NIS 1.0.1 ![nis_1 0 1](https://user-images.githubusercontent.com/96778648/149041727-579415a1-7a85-4119-b556-3a3175f93d8f.png)
NIS 1.0.1a (fix) ![nis_1 0 1a](https://user-images.githubusercontent.com/96778648/149041761-05fa47a0-56fd-4c7b-af0a-d917c3b5b305.png)
CptLucky8 commented 2 years ago

You can close this issue now, it is all working fine!

I don't know what went wrong this morning but all seems working fine now?!

NIS 1.0.1-fix with Scale 66% Sharpen 50%... ![FS2020_20220111_213736_NIS_150_50](https://user-images.githubusercontent.com/96778648/149054739-2331e498-cb33-439a-8b7d-4982f0f18b43.jpg)
abernalnv commented 2 years ago

No problem. Thank you for your feedback. We appreciate your work and the time you took to integrate NIS in your application! Please let us know of any issues you may find.

mbucchia commented 2 years ago

Thank you!

I'll quote some of the things our users have been saying about NIS once we added support for it: "extraordinary", "magic", "fantastic", "game changer" ;)