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

Allow specialization constant overrides in config #46

Merged
merged 3 commits into from
Jan 5, 2020

Conversation

pchome
Copy link
Contributor

@pchome pchome commented Jan 4, 2020

Only simple types (bool, int, float) for now.

Example:

reshade = reshade-shaders/Pixelate.fx

# type  = slider
# min   = 2
# max   = 48
# Cell Size
#cell_size = 4
cell_size = 2

# type  = slider
# min   = 0.0
# max   = 1.0
# Smoothness
#avg_amount = 0.333
avg_amount = 1.0

Only simple types (bool, int, float) for now.

Example:
```ini
reshade = reshade-shaders/Pixelate.fx

# type  = slider
# min   = 2
# max   = 48
# Cell Size
#cell_size = 4
cell_size = 2

# type  = slider
# min   = 0.0
# max   = 1.0
# Smoothness
#avg_amount = 0.333
avg_amount = 1.0
```
@DadSchoorse
Copy link
Owner

Interesting. The my question is if this would be easier for the end user than editing the shader file directly. Also different shaders could have a spec constant with the same name and the user probably does not want to set all of them to the same value.

src/effect_reshade.cpp Outdated Show resolved Hide resolved
@pchome
Copy link
Contributor Author

pchome commented Jan 4, 2020

The my question is if this would be easier for the end user than editing the shader file directly.

  1. Yes, if the shader file is in git managed directory.
  2. Yes, if it's me, who don't want to keep and maintain additional patch, and who generating config file automatically with protonfixes (e.g util.set_vkbasalt_option('Power', '3.5')) 😄

Anyway, they don't forced to configure effects or add every option in other case. It's even better when all unused options will be commented out, then their default values will be used.

@DadSchoorse
Copy link
Owner

DadSchoorse commented Jan 4, 2020

could the name in the config file be changed to something like *effect_name*.*spec_constant_name*?
So that multiple shaders with the a spec constant with the same name can be set independently?
e.g:

effects = effect1:effect2

effect1 = ...
effect2 = ...

effect1.cell_size = 2

effect2.cell_size = 5

@pchome
Copy link
Contributor Author

pchome commented Jan 4, 2020

I guess per-effect config file support would be better.
E.g. if I use CRT.fx, then check if there is CRT.conf in current workdir. This will keep main config file cleaner, and will allow to have per-game settings.

But shouldn't be a problem to cut *effect_name*. string too, if there's such settings in the main config file.

@DadSchoorse
Copy link
Owner

I guess per-effect config file support would be better.

Yes I think that would be the best solution .

@pchome
Copy link
Contributor Author

pchome commented Jan 4, 2020

effect1.cell_size

Also, the problem is that some effects can have long names with spaces. E.g. PD80 Contrasts & ColorFX.fx.

@DadSchoorse
Copy link
Owner

DadSchoorse commented Jan 4, 2020

maybe we should support quotes?

effect1 = "reshade-shaders/PD80 Contrasts & ColorFX.fx"

@pchome
Copy link
Contributor Author

pchome commented Jan 4, 2020

Or maybe INI file format. It will allow to use sections, and spaces in section names. Should be possible to do something like this:

[reshade-shaders/PD80 Contrasts & ColorFX.fx]
option = 1
option2 = 2.2

[reshade-shaders/CRT.fx]
option = 1

@pchome
Copy link
Contributor Author

pchome commented Jan 4, 2020

BTW, similar thing (dynamic spec constants) can be done for your "simple effects", what (I guess) will let you merge all separate effects code. The downside is that additional dependency will be needed, e.g. spirv-cross.

Sample code for config file generator:

#include "spirv_cross/spirv_cross.hpp"
...
  // Read SPIR-V file
  std::ifstream rawCFile(spvFile, std::ios::binary);
  std::vector<char> spirv_data((std::istreambuf_iterator<char>(rawCFile)), std::istreambuf_iterator<char>());

  // evil magic
  spirv_cross::Compiler comp(std::move(*reinterpret_cast<std::vector<uint32_t>*>(&spirv_data)));

  for (auto& con : comp.get_specialization_constants()) {
    spirv_cross::SPIRConstant cnst = comp.get_constant(con.id);
    output
      << "c: " << con.constant_id
      << " : " << comp.get_name(con.id)
      << " = " << cnst.scalar()
      << " t:" << cnst.constant_type
    << std::endl;
  }

Since you already have SPIR-V file loaded, you could feed it's data to spirv_cross::Compiler and get what you need from it.

LDFLAGS += -lspirv-cross-core -lspirv-cross-cpp required.

But that just FYI.

@crosire
Copy link

crosire commented Jan 4, 2020

INI format would have the advantage that you could plug in existing ReShade preset files: ReShade generates files of the format:

Techniques=Comma,separated,list,of,active,technique,names

[Effect1.fx]
Variable1=Value1
Variable2=Value2

[Effect2.fx]
Variable1=Value1
Variable2=Value2

Where the INI categories are effect file names (file name only, no directory, but with fx extension) and the Techniques value is e.g. Techniques=SMAA,AspectRatioPS (so not file names, but names of any techniques inside the effect files that should be used and rendered).

But that would mean adding an INI parser, which, while not overly complicated, may not be desireable.

pchome and others added 2 commits January 5, 2020 12:02
@DadSchoorse
Copy link
Owner

I'm merging this for now but the config system needs a overhaul

@DadSchoorse DadSchoorse merged commit 7762519 into DadSchoorse:wip-reshade-fx Jan 5, 2020
@pchome pchome deleted the patch-2 branch January 11, 2020 10:04
DadSchoorse added a commit that referenced this pull request Feb 2, 2020
* Allow specialization constant overrides in config

Only simple types (bool, int, float) for now.

Example:
```ini
reshade = reshade-shaders/Pixelate.fx

# type  = slider
# min   = 2
# max   = 48
# Cell Size
#cell_size = 4
cell_size = 2

# type  = slider
# min   = 0.0
# max   = 1.0
# Smoothness
#avg_amount = 0.333
avg_amount = 1.0
```

* Move temporary variables initializations into loop

* Use smallCamelCase for variables

I want a at least somewhat consistent naming scheme

Co-authored-by: DadSchoorse <[email protected]>
@ukos-git ukos-git mentioned this pull request Apr 11, 2021
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.

3 participants