LukasBanana / XShaderCompiler

Shader cross compiler to translate HLSL (Shader Model 4 and 5) to GLSL
BSD 3-Clause "New" or "Revised" License
352 stars 48 forks source link

Feature request: Add an option to treat "sampler" as "sampler2D" #88

Closed moradin closed 6 years ago

moradin commented 6 years ago

We discussed this privately and I'm running modified code currently but I think it would be good to have this implemented properly.

My changes are simple, in GenerateSamplerTypeDict() I just change how "sampler" is treated:

{ "sampler", T::Sampler2D },

I would like to have this connected with an option though and I'm not sure how to achieve that.

LukasBanana commented 6 years ago

There are two functions with the name GenerateSamplerTypeDict, but I guess you mean the one in the HLSL frontend (in HLSLKeywords.cpp).

The sampler keyword is from HLSL3 (D3D9), but in this online docu it seems that it's also used as a sampler state rather than a combined texture-sampler (like in GLSL for OpenGL):

sampler MeshTextureSampler = sampler_state {
    Texture = /* ... */;
    MipFilter = LINEAR;
    MinFilter = LINEAR;
    MagFilter = LINEAR;
};

The Texture field will then (as I understand this) determine the actual sampler type. In this case, the keyword "sampler" should be mapped to something like <T::SamplerDynamic> for instance. I never used this kind of sampler myself and if I had to guess, I'd say that this is a relict from the effect files (*.fx).

If you can show a small example where you use such a sampler we can probably clear this up. Just adding yet another option to translate a shader to a specific use case (in this case a sampler2D output) is not really what I'd like to add here. However, I agree that the current output is not right but I just want to find a generic solution to avoid that another issue will be opened in which someone wants to add a further option to translate it to sampler1D etc. (you get the idea).

moradin commented 6 years ago

A simple HLSL example:

sampler tex0: register(s0);

struct PSinput
{
    float2 Texcoord0: TEXCOORD0;
};

void main( in PSinput input, out half4 output : COLOR )
{
    output = tex2D( tex0, input.Texcoord0.xy );
}
LukasBanana commented 6 years ago

I entered this code at shader-playground.timjones.io but most cross-compilers can't process this code (including Xsc).

Fxc seems to be the only compiler being able to compile it to this:

ps_3_0
dcl_texcoord v0.xy
dcl_2d s0
texld_pp oC0, v0, s0

Looks like sampler is indeed interpreted as sampler2D but I can't find any documentation on the web about this. Is it possible that this is some undocumented feature just like structure inheritance in HLSL which almost nobody knows?

moradin commented 6 years ago

It is possible, unfortunately I have to deal with a lot of code that uses it :)

LukasBanana commented 6 years ago

Since it seems that Fxc treats the sampler keyword as sampler2D by default and the sampler states in HLSL3 are currently not fully supported anyway (e.g. sampler foo = sampler_state { ... }) I added a quick fix into the STL-export-fix branch in 54fa065.

I'll merge this together with the fixed STL-export issues into the master when I have more time.

That said, I still think adding a compiler option for this is not a good idea since I consider it as a corner case and I don't want to add options to enable/disable quirky features. I'd rather let it always enabled since there doesn't seem to be any other usecase for the sampler keyword in HLSL3.

If you still have trouble with this issue, just re-open this ticket.

moradin commented 6 years ago

Ok thanks for the quick fix. I was thinking about adding the option because I thought some people might want to actually fail conversion when this is used but it's really not that big of a deal.