-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Vulkan: Allow custom texture upscaling shaders #13235
Conversation
dst[ 6] = mix(dst[ 6], blendPix, (needBlend && doLineBlend && haveShallowLine) ? 0.25 : 0.00); | ||
} | ||
|
||
// select output pixel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a compute shader, we can actually go ahead and just write all 16 pixels instead of just picking one, although we would have to change the scaling shader framework a bit ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is literally just a copy/paste of the shader that was hardcoded before.
I think we could do this by calling different functions though, i.e. the shader could expose a 16 pixel func.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I know it was copy pasted, just commenting anyway.
[Tex4xBRZ] | ||
Type=Texture | ||
Name=4xBRZ | ||
Author=Hyllian | ||
Compute=tex_4xbrz.csh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the scaling shader is hardcoded for a specific scale factor (4x in this case), I think that should be specified here somehow, and checked / enforced in the VK texture cache code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kinda like having a different "output resolution", but I suppose at least it just not be forced for all shader types.
I guess it would just ignore your configured scale then?
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, something like that. What does this do right now if the texture scale isn't set to 4x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just runs the shader for each pixel, so it works fine.
In other words, in basically the same way the "5xBR Upscaler" postshader works when you don't use 5x render resolution (which also does not force anything.)
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, so since it uses the mix function at the end to choose the pixel, we end up with a kind-of bilinearly downsampled (but not across blocks) version of the 4x upscale. I suppose that's alright-ish, though for "non-native" scale factors we should probably just have it scale up to to the native 4x in this case, output all the pixels, then downscale after that with a better filter in case the goal is to save memory/bandwidth during rendering.
Obviously out of scope for this PR though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Citra uses xBRZ freescale which is resolution independent, but I'm not sure how much work would be required to get it working without linear sampling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm starting to think that the overhead of one more copy and creating a temporary Image isn't actually all that high. In that case we can get free bilinear. Might do that soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's probably a wise idea 👍
Similar to post-processing shaders, this makes the texture scaling shader configurable. I reused a lot of postshader logic because it's very similar, even conceptually.
That said, no chaining (is that even useful? probably no?) and no settings. And it only happens on > 1x scaling.
Maybe we should allow even when scaling is off, so you can for example apply effects to textures. Settings likely make sense too.
Still Vulkan only. We might end up having a hardcoded "off" if we move clut processing into this shader.
-[Unknown]