Page MenuHomeWildfire Games

Contrast-Adaptiv-Sharpening pass
Needs ReviewPublic

Authored by OptimusShepard on Feb 20 2020, 8:50 PM.

Details

Reviewers
vladislavbelov
Trac Tickets
#5677
Summary

Adds an sharpening pass option to the game.

Test Plan

Select CAS as sharpening pass in the graphic settings. Select different sharpness factors by slider.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1794/display/redirect

nani awarded a token.Feb 21 2020, 12:02 AM

@OptimusShepard I tested your patch and when I select CAS in the options, the error appears:

ERROR: Failed to compile shader 'shaders/glsl/cas.fs': 0(64) : warning C1503: undefined variable "rcp" 0(70) : warning C1503: undefined variable "rcp" 0(73) : warning C1503: undefined variable "rcp" 0(64) : error C1008: undefined variable "rcp" 0(70) : error C1008: undefined variable "rcp" 0(73) : error C1008: undefined variable "rcp"

Imarok added a subscriber: Imarok.EditedFeb 23 2020, 3:44 PM

I got

ERROR: Failed to compile shader 'shaders/glsl/cas.fs':
0(64) : error C1008: undefined variable "rcp"
0(70) : error C1008: undefined variable "rcp"
0(73) : error C1008: undefined variable "rcp"

Edit: Tested on Ubuntu 18.04 with GTX 960

@OptimusShepard I tested your patch and when I select CAS in the options, the error appears:
ERROR: Failed to compile shader 'shaders/glsl/cas.fs': 0(64) : warning C1503: undefined variable "rcp" 0(70) : warning C1503: undefined variable "rcp" 0(73) : warning C1503: undefined variable "rcp" 0(64) : error C1008: undefined variable "rcp" 0(70) : error C1008: undefined variable "rcp" 0(73) : error C1008: undefined variable "rcp"

May I ask you which setup did you use?

Stan added a subscriber: Stan.Feb 23 2020, 4:14 PM

AFAIK @gameboy uses Windows.

OptimusShepard retitled this revision from Contras-Adaptiv-Sharpening pass to Contrast-Adaptiv-Sharpening pass.Feb 23 2020, 4:17 PM
In D2642#110914, @Stan wrote:

AFAIK @gameboy uses Windows.

Do you know from which vendor his GPU is?

OptimusShepard added a subscriber: vladislavbelov.

Replace rcp(x) to 1/x.
rcp seems to be a performance optimized function for AMD GPUs. As strange, rsqrt is also a special function, which doesn't work with my GPU. I had it replaced bevor my first upload.
@vladislavbelov do you know more about such things? Is it possible to implement a parallel path for AMD hardware?

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1806/display/redirect

I got

ERROR: Failed to compile shader 'shaders/glsl/cas.fs':
0(64) : error C1008: undefined variable "rcp"
0(70) : error C1008: undefined variable "rcp"
0(73) : error C1008: undefined variable "rcp"

Edit: Tested on Ubuntu 18.04 with GTX 960

Works with the new version.

I use Intel CPU, and the graphics card USES NVIDIA GTX850M. The operating system is Windows10 64bit. Do you support Intel CPU and NVIDIA series of graphics CARDS in this patch

I use Intel CPU, and the graphics card USES NVIDIA GTX850M. The operating system is Windows10 64bit. Do you support Intel CPU and NVIDIA series of graphics CARDS in this patch

Thanks. As I replaced the rcp function, it should now work with every GPU.

OptimusShepard edited the test plan for this revision. (Show Details)

Add slider for sharpness factor.

Stan added inline comments.Feb 25 2020, 8:27 PM
binaries/data/mods/public/shaders/glsl/cas.fs
4

Might want to say you edited it :)

Angen added a subscriber: Angen.Feb 25 2020, 8:31 PM
Angen added inline comments.
binaries/data/mods/public/shaders/glsl/cas.fs
4

If edited, it needs to be said

OptimusShepard added inline comments.Feb 25 2020, 8:44 PM
binaries/data/mods/public/shaders/glsl/cas.fs
4

It's a port of the Reshade port from AMDs CAS. So, do I need to say I edited a port, or is it only necessary to say the original (AMD) source has been edited?

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1815/display/redirect

Add an editing info to the cas.fs header.

OptimusShepard marked 2 inline comments as done.Feb 26 2020, 3:59 PM
OptimusShepard added inline comments.
binaries/data/mods/public/shaders/glsl/cas.fs
4

I compared the original source and the Reshade port. It's nearly the same. They only difference I've adopted is, to put the scalar operations together to a vector. I don't think this is license relevant?

4

I added an commentary to the end of the header.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1818/display/redirect

nani added a subscriber: nani.Feb 26 2020, 4:45 PM
nani added inline comments.
binaries/data/mods/public/shaders/glsl/cas.fs
52

Some compilers don't infer the value type so better change vec2(1, 1) to vec2(1.0, 1.0).
Do the same for all the other values meant to be a float.

Change the representation of floats in cas.fs.

OptimusShepard marked an inline comment as done.Feb 26 2020, 6:51 PM

Thx

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1820/display/redirect

My friend, would you consider updating something?

The patch is waiting for reviewing, what update do you want to have? The patch is mostly working. As it have the same problem as the FXAA patch, that it doesn’t work on all maps, and I don't know how to solve it, I will wait and see, how Vladislav is solving this problem.
If you want to test it, you could choose e.g. the new shirmish maps Obedska Bog or Sahyadri Buttes.

Stan added a comment.Apr 20 2020, 10:20 AM

The patch is waiting for reviewing, what update do you want to have? The patch is mostly working. As it have the same problem as the FXAA patch, that it doesn’t work on all maps, and I don't know how to solve it, I will wait and see, how Vladislav is solving this problem.
If you want to test it, you could choose e.g. the new shirmish maps Obedska Bog or Sahyadri Buttes.

@vladislavbelov is working on the issue. The problem lies in finding a good way to order shader passes because they cannot be used in any order (which could be overriden by maps)

OptimusShepard planned changes to this revision.Apr 27 2020, 10:05 AM

Waiting until D2712 is committed. A little modification of the code of this patch has to be done.

Stan added a comment.Apr 27 2020, 10:13 AM

Some comments for later. :)

binaries/data/mods/public/shaders/glsl/cas.fs
4

2020? I guess it is still copyrighted?

source/renderer/PostprocManager.cpp
38

Nuke.

598

Wondering if it wouldn't be better to always call

m_SharpTech = g_Renderer.GetShaderManager().LoadEffect(CStrIntern(m_SharpName));

and let it error out if it isn't found.

Maybe even better if you can load the json to figure that out.

@OptimusShepard Please update it as soon as possible, I will help you test the latest CAS

OptimusShepard added inline comments.Apr 27 2020, 2:12 PM
binaries/data/mods/public/shaders/glsl/cas.fs
4

Their Git seems to be long time not updated. It's still 2017-2019. How does it affect us?
(https://github.com/GPUOpen-Effects/FidelityFX)

Stan added inline comments.Apr 27 2020, 5:04 PM
binaries/data/mods/public/shaders/glsl/cas.fs
4

I guess leave it as is. nvm :) From a quick google search I can't find anything.

Could you create the patch with context?

  1. Updating D2642: Contrast-Adaptiv-Sharpening pass #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Edit some comments.

Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Apr 27 2020, 11:23 PM

First time using Arcanist. Something went wrong, files are missing. How can I turn back to previous version?

Stan added a comment.Apr 28 2020, 12:00 AM

arc patch --diff 11434 --force

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2036/display/redirect

Have you fixed it, my friend?

Revert to previous version and edit some comments.

OptimusShepard marked 2 inline comments as done.Apr 28 2020, 10:05 AM
OptimusShepard added inline comments.
source/renderer/PostprocManager.cpp
598

You're right. If we check with the if (line 595) the allowed configurations, we don't need the errors, I think?

Stan added inline comments.Apr 28 2020, 10:18 AM
source/renderer/PostprocManager.cpp
598

Well the question is what should we do when the user enters an incorrect / unsupported value, should we tell him?

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2041/display/redirect

OptimusShepard marked an inline comment as done.Apr 28 2020, 10:54 AM
OptimusShepard added inline comments.
source/renderer/PostprocManager.cpp
598

Hm. As the user uses a drop down menu to select the filter, the only possibility to get an incorrect value is to edit the config file. But this wont lead to an crash. So is a error message really necessary? If we think so, we have to add error warnings to all parameter of the config file.

Stan added inline comments.Apr 28 2020, 10:56 AM
source/renderer/PostprocManager.cpp
598

I guess keep it consistent with what is done currently then:)

Modified it, so it could be used on all maps.

Vulcan added a comment.May 3 2020, 1:41 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2071/display/redirect

Updating the licence and the link, as AMD had updated it. Changing the game options name to the AMD recommented one.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/scripting/JSInterface_Renderer.h
|  27| namespace·JSI_Renderer
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'namespaceJSI_Renderer{' is invalid C code. Use --std or --language to configure the language.

source/renderer/PostprocManager.h
|  26| class·CPostprocManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classCPostprocManager{' is invalid C code. Use --std or --language to configure the language.

source/ps/CStrInternStatic.h
|   1| /*·Copyright·(C)·2014·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2014"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2268/display/redirect

Shouldn't the CAS license be mentioned in LICENSE.txt?

Angen added inline comments.Sat, Jun 13, 11:03 AM
source/ps/CStrInternStatic.h
1

could you please update year?

Add license, change header year.

Shouldn't the CAS license be mentioned in LICENSE.txt?

I had looked on you fxaa patch. BSD is here correct, too?

OptimusShepard marked an inline comment as done.Sat, Jun 13, 1:51 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2419/display/redirect

Stan added a comment.Sat, Jun 13, 2:23 PM

Tested on my UHD 630 and GTX1070 no problem. It even works nicely with the MSAA patch.