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

Use less FBOs for chained post-processing shaders #12921

Merged
merged 7 commits into from
Jul 13, 2020

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented May 17, 2020

A few changes (for v1.11.x):

  • Reuse FBOs for larger chains, if they use the same size. Alternates.
  • Makes Draw use GL uniform locs rather than looking up names each uniform update.
  • Fixes texture/sampler assignment in Draw for any but the first texture.

This adds sampler1 to read the original source, which isn't tested thoroughly but seems to work in GL/D3D9/D3D11.

In Vulkan, I tried fixing it up, but if I assign the second sampler in the descriptor set update - the shader somehow doesn't apply (but the image still presents?) With RenderDoc attached it simply crashes, and there's no validation layer feedback. Arg.

See anything?

-[Unknown]

@hrydgard
Copy link
Owner

Cool. I'll help out with the Vulkan issue later, but maybe not tonight.

@iota97
Copy link
Contributor

iota97 commented Jun 12, 2020

With Vulkan pause menu is totally black as well for me now so there is some bigger issues I think behind.

I could create a in game dump with renderdoc on my machine (the first shader is grayscale, the second should use it for the left part of the screen and the original for the right one).

renderdoc dump

There is some code that look weird to me tho', I will comment on the code it self about that (might be irrelevant).

VkBuffer buffer_;

bool operator < (const DescriptorSetKey &other) const {
if (imageView_ < other.imageView_) return true; else if (imageView_ > other.imageView_) return false;
if (sampler_ < other.sampler_) return true; else if (sampler_ > other.sampler_) return false;
for (int i = 0; i < MAX_BOUND_TEXTURES; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop seem weird to me, it have return in it so unless they are the same it will end asap, is this fine is this order? I'm not sure what is this used for tho'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, this is just a compare to say - is this key the same? It's so it can build the map. This should be fine.

-[Unknown]

unknownbrackets and others added 7 commits June 12, 2020 12:35
Before, we created things at the wrong size, then recreated at the right.
Now we just start at the correct size once.
If you apply 4 in a row at the same resolution, we only need two actual
FBOs.  This will use less VRAM.
So that we can initialize and bind samplers.
Not working properly though...
@unknownbrackets unknownbrackets marked this pull request as ready for review June 12, 2020 19:43
@hrydgard hrydgard merged commit bb6219e into hrydgard:master Jul 13, 2020
@unknownbrackets unknownbrackets deleted the postshader branch July 13, 2020 20:58
hrydgard added a commit that referenced this pull request Jul 13, 2020
…ationCommon::CopyToOutput.

Validation caught an issue where old stuff lingered in sampler 1 and texture 1.

Bug probably introduced in #12921, but could also be others.
hrydgard added a commit that referenced this pull request Jul 13, 2020
…ationCommon::CopyToOutput.

Validation caught an issue where old stuff lingered in sampler 1 and texture 1.

Bug probably introduced in #12921, but could also be others.
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