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

Post shader setting uniform #12901

Merged
merged 3 commits into from
May 16, 2020
Merged

Conversation

iota97
Copy link
Contributor

@iota97 iota97 commented May 15, 2020

Add some setting to configure post processing with uniform (up to 4, slider will be visible only if there is a setting, setting name and default value can be set in the .ini file).

I haven't tested on Windows sadly and I might have made some dumb mistake (hope not, seem working fine on android and Linux OpenGL and Vulkan).

Not sure if is worth to have such a option either.

PS: While testing I have noticed that for some reason sharpen shader only get applied on the leftmost 1/5 of the screen on my Redmi 4X on OpenGL, as it affect only the phone in OpenGL with only that shader and also on master also before shader refractoring I guess is a driver bug, kinda weird tho'.

@hrydgard
Copy link
Owner

Yeah that has to be a driver or maybe math precision bug...

What I would be concerned about here is if we could find a nice way to save the settings separately for each postshader, so they don't overwrite each other when you switch... Hm.

@iota97
Copy link
Contributor Author

iota97 commented May 15, 2020

I guess it's precision as adding 0.0001 seem to little for the float granularity, increasing it make a mess, but all over the screen at least :)

color -= texture2D(sampler0, v_texcoord0.xy+0.0001).xyz*7.0*amount;

I wonder if it happen only on Adreno 5xx or also other mobile GPU.


Saving the setting separately would be nice but might get a bit contorted.

@hrydgard
Copy link
Owner

Well, we have to at least reset the settings on switching postshaders, because you could end up with seriously out of whack values going from one to another otherwise...

@iota97
Copy link
Contributor Author

iota97 commented May 15, 2020

That happen already on PostProcScreen::onComplete (miscScreen.cpp)

Sending gpu_resize message on GameSettingsScreen::OnPostProcShaderChange should get the uniform recalculated as well, doesn't it?

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 15, 2020

Since it's floating point slider, maybe setting a custom step would also be useful, when the range of the setting is too big, going by 0.01 might be anoying.

If you'd like to try a different sharpen filter, I included one in my overgrown shader ~ http://forums.ppsspp.org/showthread.php?tid=6594, it's called complex sharpen or something, I recall borrowing it from some media player. Might look better with accuracy issues, through it'll definitely be heavier than the simple sharpen we have.

@hrydgard
Copy link
Owner

@iota97 Oops, missed that. Yeah, maybe good enough.

Agree with LunaMoo that a step for the slider would also be useful.

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 15, 2020

Here's a separated Sharpen Complex V2 effect if you want to test how it looks on that phone where simple sharpen doesn't work.
sharpenComplexV2.zip

On a side note, with shader settings we could easily join things like 5xBR and 5xBR_lv2 and even add additional corner variants or xBRZ wariant since a lot of the code is shared between them. Same we could add a complex sharpen variant to simple sharpen while those are completely different, having a different variant of the same effect that might work better on some devices through setting seems like a better idea than adding more effects making the list too long.

Oh and I think there are two ways people typically use post process effects:

  • use one with same settings for ALL games(coloring effects or AA are usually universal),
  • use different effect per game and use per-game settings.

Both groups should be fine with effects not storing settings separately per effect due to still being stored separately per game if needed.

@iota97 iota97 force-pushed the postshader-setting branch from 8aaa338 to a666635 Compare May 15, 2020 16:09
@iota97
Copy link
Contributor Author

iota97 commented May 15, 2020

Made the slider step configurable and added a basic shader for gamma, saturation, contrast and brightness (I've seen at least brightness setting asked few time).

I was also wondering as the uniform get send always might be fun to have a PPSSPP specific CWCHEAT to set them from game rather than from a config in the future.

@hrydgard
Copy link
Owner

Ha, I guess that could be interesting, not sure what the best use cases would be. Maybe switch postprocessing between ingame and video?

Another thing we might want eventually is to apply multiple effects in sequence, and then we'll definitely need to separate the settings. But that's a bigger project.

@hrydgard
Copy link
Owner

I'll think a little more about naming but overall I like this and intend to merge soon.

@iota97
Copy link
Contributor Author

iota97 commented May 15, 2020

Tbh I was thinking about effect on getting hit like blur or so, or effect when you use a consumable, but even switching effect on area might be good (isn't video uniform already present tho' or misunderstood it?).

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 15, 2020

Ah, multi pass effects, would be nice at some point. Same as access to older frame(for motion blur-like effects).

We can already apply different effects for video and not in post process effects:), but using cheats for that could definitely be more accurate than just checking if video is playing with some games using videos as backgrounds.
I think even better would be to for example add screen shake or some other effects on being hit, or if in the future we get some motion blur effects possible, adding a motion blur and some shaking when reaching higher speed in racing games could also be nice:).

@unknownbrackets
Copy link
Collaborator

I was actually thinking about the multipass while refactoring, of course. What I thought was the easiest way would be, in the ini:

Parent=OtherShaderName
Hidden=True/False (in case this is only for other things to use as a parent)

Then we'd resolve the chain, and we could just basically loop the application with more FBOs. I tried to reorganize the pipelines and FBO handling so this is easier in the future.

I didn't think through if SSAA or output res etc. could apply to shaders in the middle, though.

-[Unknown]

@unknownbrackets
Copy link
Collaborator

Personally, I'd use a std::map for these settings btw - we don't have many (any?) settings like that, but it'd be like the Recent list. Then we could allow the settings to be set per shader. I feel like that would make more sense.

i.e. shaderName + StringFromFormat("Setting%d", i) as the ini key, maybe in a separate section like [PostshaderSettings].

-[Unknown]

@iota97
Copy link
Contributor Author

iota97 commented May 15, 2020

I tried to use a std::map, but I have some issues:

  1. ATM it use both a global config for the current shader setting and a map, I'm not sure if using the map directly might have some performance impact reading from it every frame to update the uniform.

  2. I can't find a way to stop game with custom config to overwrite global as well.

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you're missing unloadGameConfig? That's where it should reload the global values.

-[Unknown]

Core/Config.cpp Outdated Show resolved Hide resolved
UI/GameSettingsScreen.cpp Outdated Show resolved Hide resolved
GPU/Common/PresentationCommon.h Outdated Show resolved Hide resolved
@hrydgard hrydgard merged commit 67ab51b into hrydgard:master May 16, 2020
@iota97 iota97 deleted the postshader-setting branch May 17, 2020 17:22
@unknownbrackets unknownbrackets added this to the v1.10.0 milestone Jun 22, 2020
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.

4 participants