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 chained post-processing shaders #12905

Merged
merged 11 commits into from
May 17, 2020

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented May 16, 2020

With #12883 merged, this was the next step. Required a little reorganization of render target size and uniforms, but pretty straight forward now.

I can fix this up after #12901 is merged, or tweak that to handle each layer's settings.

Basically, here are some examples:

[InverseVignette]
Name=Inverse Colors + Vignette
Parent=InverseColors
Fragment=vignette.fsh
Vertex=fxaa.vsh

[Complex]
Name=Complex
Parent=ComplexHelper1
OutputResolution=True
Fragment=vignette.fsh
Vertex=fxaa.vsh

[ComplexHelper1]
Visible=False
Parent=5xBR-lv2
OutputResolution=True
Fragment=natural.fsh
Vertex=natural.vsh

In this case, "ComplexHelper" would not show up in menus. "Complex" wouild, and it would first apply 5xBR-lv2, then natural colors blur, then finally a vignette. The visible flag makes it possible to add a link in the chain that isn't useful alone.

One thing I wasn't sure about is upscaling shaders. Currently, we use nearest after we see the first one. I don't know if that always makes sense, i.e. if you apply vignette after an upscaler.

Should it maybe only use nearest if it's the last shader? Or only for the next one?

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Btw, I did think about potentially using less framebuffers (it's probably? a common case that multiple in a row would be the same size.) I think this VRAM hungry approach is simpler and we can test out for bugs.

For saving VRAM, we'd just need to create at most two framebuffers for any given size. Then we'd keep a separate vector for freeing of the unique ones, and have the vector of for-use ones just never use the same one twice in a row.

But, figure we can do that after.

-[Unknown]

@hrydgard
Copy link
Owner

I was actually mainly imagining that users would like to configure multiple postshaders, like combining some contrast and vignette - but I'm sure there are use cases for multipass shaders too!

I'll look at it in a little while.

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 16, 2020

Didn't had time to test this yet myself, but I know there are a lot of people that hate the XMB ripoff and the whole dependency on single piece of software, but still choose to use libretro port purely for the multi pass shaders that wasn't supported by PPSSPP, this will be a gamechanger for those people as libretro large effects library will be easily convertable.

As for forcing nearest for upscaling shaders - render at x1 res stretched to anything above will be too blurry to correctly apply effect, hence why it's needed, however after we get upscaled image from such filter applying something like a vignette should already deal only with image that was at output res, so nearest or linear screen scaling filter should not make a difference.

@unknownbrackets
Copy link
Collaborator Author

Okay, I made it use nearest only for that shader then.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 16, 2020

After merge with master(which confusingly doesn't show any problems) this PR doubles this code:

	uint16_t indexes[] = { 0, 1, 2, 0, 2, 3 };
	draw_->UpdateBuffer(idata_, (const uint8_t *)indexes, 0, sizeof(indexes), Draw::UPDATE_DISCARD);

Hence builds will fail.

@unknownbrackets
Copy link
Collaborator Author

Hm, I just didn't notice a637069. Will rebase.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 16, 2020

Made a quick test with your example effects and seems there's something wrong with upscaling effects. As I understand "Complex" shader should do:

  1. pass - 5xBR-lv2
  2. pass - natural colors
  3. pass - vignette
    Which in desired effect would be high res 5xBR-lv2 with modified colors and vignette applied. It however is low res as if second pass would still make use of the input res.

Edit: Or nah, it's the filtering, seems it makes a difference after all. Don't know why, upscaling shader should make both linear and nearest screen scaling filter look same after it upscales. This now kind of looks as if nearest was not applied to screen scaling filter at all.

ULUS10062_0001.zip
Here's a dump from SFA3Max which stretches into a blurry mess hence it's good example to test.

@unknownbrackets
Copy link
Collaborator Author

You're right, you'd probably want OutputResolution=True on the later two passes to preserve the resolution.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 16, 2020

Your right, adding OutputResolution=True to other passes makes it correct, maybe this setting should be applied for all passes automatically if one passes makes use of it? Wonder if SSAA has similar interaction.

@unknownbrackets
Copy link
Collaborator Author

Also, here's the reuse (will wait to make a pull after), seems to work well:
unknownbrackets/ppsspp@postshader...postshader2

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

unknownbrackets commented May 16, 2020

Yeah, SSAA would do the same.

I guess I could have the next pass default to the previous resolution instead of going back to render resolution each time...

If you chain multiple SSAA filters, what's the expected result? Original resolution * SSAA for each, or increase by SSAA level each time?

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented May 16, 2020

I think original res * SSAA would be safer as this can go very crazy very fast, but at the same time there could be some use case where increasing res just a bit between applying additional filter would have sense, so I'd personally go for the latter, creating such chain requires a person's input so let's assume whoever makes a shader knows how to use that power and doesn't end up applying x256 render res by accidently chaining few SSAA multipliers for no reason.:)

@hrydgard
Copy link
Owner

Looks good. I've merged #12901, so fix up as needed and this should be good to go.

@unknownbrackets
Copy link
Collaborator Author

Okay, there we go. Now it shows settings in the UI for each of the shaders in the chain, and updates the uniforms for each one when applying, as well.

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Good to go on this?

-[Unknown]

@hrydgard
Copy link
Owner

Yeah.

@hrydgard hrydgard merged commit 7a6489e into hrydgard:master May 17, 2020
@unknownbrackets unknownbrackets deleted the postshader branch May 17, 2020 14:23
@unknownbrackets unknownbrackets added this to the v1.10.0 milestone May 17, 2020
@LunaMoo
Copy link
Collaborator

LunaMoo commented May 17, 2020

@unknownbrackets can we somehow get initial sampler0 we had at pass0 in higher passes? I'd like to make my simple test shader last into the chain it draws all the effects on one side of the screen and original picture on another, but without adding that separation for each pass it seems impossible now?

@unknownbrackets
Copy link
Collaborator Author

Yes, see #12921. Vulkan isn't working, though.

-[Unknown]

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