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

Ambiguous function call #93

Closed
moradin opened this issue Sep 7, 2018 · 10 comments
Closed

Ambiguous function call #93

moradin opened this issue Sep 7, 2018 · 10 comments
Assignees
Labels

Comments

@moradin
Copy link

moradin commented Sep 7, 2018

http://shader-playground.timjones.io/124766c34cc34e47ca09c3df1038889d

The above link demonstrates the issue. FXC can compile fine but switching to XShaderCompiler results in errors.
There are actually more errors visible in the shader playground than the ones I'm getting but that might be because I couldn't paste the whole shader

@LukasBanana
Copy link
Owner

The other errors occur due to the lack of an option for the shader input version in shader-playground (e.g. HLSL3 or Cg). In HLSL5 the intrinsics tex2D etc. are not allowed (at least not with Xsc).

For the ambigious function call, Xsc clearly shows why this errors occurs:

context error (<unnamed>:41:9) : ambiguous function call 'Sample_f4(Sampler, float2, double)'
    return Sample_f4(s_tex, uv, 0.0);
           ^~~~~~~~~~~~~~~~~~~~~~~~~
candidates are:
  'float4 Sample_f4(Sampler, float2, float)' (<unnamed>:16:8)
  'float4 Sample_f4(Sampler, float3, float)' (<unnamed>:26:8)

The last parameter is the literal 0.0 which is from type double while 0.0f would be from type float.
However, non of your function delcarations takes a double as input parameter.
Thus, your function call is indeed somehow ambigious.

Of course, there is only one function that fits best, but since there is no clear specification for this behavior of HLSL, I would say that in this case the compiler you are using is "always right".
There are similar differences between Clang, GCC and MSVC.

Easiest solution:
when you have heavily overloaded functions like the one in your example, just make the function call more clear for the compiler, so in this case just add the f suffix to your literal.

@moradin
Copy link
Author

moradin commented Sep 7, 2018

Sure I understand that this solution would work, I just saw that it compiles fine on FXC so I was wondering if this should be an error or not.

@LukasBanana
Copy link
Owner

LukasBanana commented Sep 7, 2018

Fxc also compiles this one:

float4 VS() : SV_Position {
    const float c = 1;
    c += 5;
    return (float4)c; // Returns (1, 1, 1, 1)
}

:-D

But seriously, those differences can be annoying but I think this is more of a specification issue. So far Xsc is not really wrong about this, but it is of course worth mentioning to relax the conditions to find a suitable overloaded function.
Right now, It has no high priority for me though.

@moradin
Copy link
Author

moradin commented Sep 11, 2018

Unfortunately even fixing those issues leads to another problem:
http://shader-playground.timjones.io/89ce492f30e78046b2818f65d04f8223

I'm converting ps_3_0 shaders and defining my own Sample function. The error I get from XSC is this:

context error (lights_ps.hlsl:238:23) : intrinsic 'Sample' requires shader model 4.0, but only 3.1 is specified
float4 v_clr = Sample(s_clr, v_uv.xy, 0.f);

Just FYI the XSC version in shader playground gives completely different errors.

I will try to find where this context error is emitted in XSC and work it around but I'm posting this before because maybe you already have an idea how to fix it.

@LukasBanana
Copy link
Owner

This is a good point. Sample is considered an intrinsic of the Texture2D class for instance from HLSL4+. But when used globally, Xsc erronously interprets the identifier as a global intrinsic (there is even a dedicated error report for this). I never had the idea to delcare a function that uses the same name as a class intrinsic. It looks like a bad interpretation, though.

@LukasBanana LukasBanana self-assigned this Sep 11, 2018
@LukasBanana
Copy link
Owner

With commit d5c673b, you can now use functions with equal names to class intrinsics (this will emit a warning though!).

@LukasBanana
Copy link
Owner

If you still encounter any issues with this, just re-open the ticket.

@LukasBanana
Copy link
Owner

@moradin Xsc erroneoulsy considered double precision floats the default for floating-point literals. This is fixed now in cf76322.

Better late than never 😅

@moradin
Copy link
Author

moradin commented May 14, 2019

Thanks! Are we going to see another release in Shader Playground?

@LukasBanana
Copy link
Owner

As soon as I make a new release, I will ask Tim Jones about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants