MSL: Add an option to insert texture swizzles into generated shaders. #706
Conversation
It's intended to be used with MoltenVK to support arbitrary `VkComponentMapping` settings. The idea is that MoltenVK will pass a buffer (which it set to some buffer index that isn't being used) containing packed versions of the `VkComponentMapping` struct, one for each sampled image. Yes, this is horribly ugly. It is unfortunately necessary. Much of the ugliness is to support swizzling gather operations, where we need to alter the component that the gather operates on--something complicated by the `gather()` method requiring the passed-in component to be a constant expression. It doesn't even support swizzling gathers on depth textures, though I could add that if it turns out we need it.
|
Oh ... dear. ... ._____. Is the idea here to have two shader variants? One "clean", and one to deal with swizzling if there is at least one non-identity VkComponentMapping? Did you evaluate the possibility of having one shader variant per combination of VkComponentMapping so you could statically swizzle the texture? (I suppose that would be rather insane too). |
|
I did. It would require us to recompile the shader every time a descriptor for an image with a non-identity swizzle were bound or pushed. And the code wouldn't look much better, since we have to contend with ZERO and ONE swizzles as well. ...I wonder if it would be best to discuss this with @billhollings. |
|
I'm fine with either approach from the aspect of SPIRV-Cross if this is truly needed, although as you say, it's extremely ugly. I was under the impression though that the portability layers would not attempt to go to this length of emulation. Is there some important case that has changed this stance? (This might affect how we deal with odd-ball cases like textureQueryLod ...) When I think about it, static swizzle will not work in the case of arrays of textures. Each texture could have its own swizzle potentially, don't think we have a choice in the edgiest of cases. |
|
There is. We at CodeWeavers need stencil data from depth/stencil textures to always be in green for our D3D emulation--something which cannot be done without swizzling. There are also games that need swizzling--DOOM and Wolfenstein come to mind (no surprise there, since id Software promulgated the original |
|
Do you know of any significant performance implication of doing dynamic swizzling like this (DOOM / Wolf)? Hopefully it's just limited to a few not very important passes. I'll review tomorrow. |
|
Well... In the case where a non-identity swizzle is given, it'll now take 24 compares+4 branches (6 compares+one branch per component), which I guess could add up over time for large meshes and/or high resolutions. The gather case should be less affected, since it only operates on a single component at a time (6 compares+one branch). I admit that static swizzling would give faster shaders (no compares or branches)--it might actually be preferable. Whether it's worth the tradeoff of having to recompile shaders on every bind or push of a swizzled image descriptor, I don't know. |
|
And even if we went with static swizzle, we would need this fallback path for arrays of textures, so it'd get even more complicated. :( Have you filed this bug to Apple? Lack of texture swizzle is quite glaring. Static swizzle would probably add a fair amount of CPU overhead as well compared to just checking "do we have any swizzled texture?". |
|
I filed rdar://44287804, which turns out to be a dupe of rdar://39856026--so this is a known issue to Apple. |
reference/shaders-msl-no-opt/asm/frag/texture-access.swizzle.asm.frag
Outdated
Show resolved
Hide resolved
reference/shaders-msl-no-opt/asm/frag/texture-access.swizzle.asm.frag
Outdated
Show resolved
Hide resolved
| @@ -1237,6 +1237,7 @@ struct Meta | |||
| Decoration decoration; | |||
| std::vector<Decoration> members; | |||
| uint32_t sampler = 0; | |||
| uint32_t image = 0; | |||
There was a problem hiding this comment.
I think I'll need to refactor this to SPIRCombinedSampler that HLSL uses eventually ...
It'll be useful to have an "auxiliary buffer" for other builtins--e.g. `DrawIndex` (which should be easier to implement now), or `ViewIndex` when someone gets around to implementing multiview. Pass this buffer to leaf functions as well. Test that we handle this for integer textures as well.
Just add our own separate function for analyzing sampled image usage.
While I'm at it, don't use a bitwise op with a `bool` variable. Apparently, MSVC doesn't like that.
This needs extra work to map them back to the original resource.
|
Thanks for all the work and changes. |
It's intended to be used with MoltenVK to support arbitrary
VkComponentMappingsettings. The idea is that MoltenVK will pass abuffer (which it set to some buffer index that isn't being used)
containing packed versions of the
VkComponentMappingstruct, one foreach sampled image.
Yes, this is horribly ugly. It is unfortunately necessary. Much of the
ugliness is to support swizzling gather operations, where we need to
alter the component that the gather operates on--something complicated
by the
gather()method requiring the passed-in component to be aconstant expression. It doesn't even support swizzling gathers on depth
textures, though I could add that if it turns out we need it.