godot: Shader no longer supports 1f, 0f ... float literal syntax

Godot version

3.5 beta2

System information

Windows 10, GLES3

Issue description

Shader no longer supports 1f, 0f … float literal syntax. produces error: error(16): Invalid (float) numeric constant

Steps to reproduce

shader_type canvas_item;

uniform float a = 1f;

Minimal reproduction project

No response

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Reactions: 10
  • Comments: 21 (20 by maintainers)

Commits related to this issue

Most upvoted comments

There’s definitely a bug in 3.5 beta2. It’s not accepting the 255f there but 255.0 seems to work fine:

image

That’s because 255.0 is valid GLSL while 255f is not.

We decided to enforce valid GLSL as otherwise your shader will break unexpectedly on GPUs that strictly enforce the GLSL specification.

I think it’s fine to keep this change as long as it’s mentioned in the 3.5 changelog. Fixing old shaders can be done with sed or similar – a fancy regex should be able to perform this replacement while not touching hexadecimal numbers.

This results in a breaking change… fine for 4.x but seems unnecessary in 3.x. I mean if someone needs to strictly enforce the specification shouldn’t that be up to them, considering the long history of this being valid syntax? I know Godot “loosely” follows semantic versioning but that’s a lot of broken shaders

Well, it’s very simple to fix the old projects. I think the issue is that it wasn’t announced anywhere so it looks like a bug.

Is 1f and 0f allowed in raw GLSL? Godot’s stance on number formats in the shader language is that the same number formats should be supported as in raw GLSL (no more, no less).

Using 1.f and 0.f should work though.

I agree that the shader language should be ‘correct’

IMO more complicated method which transforms 0f to 0.0 inside the godot shader code would be ideal for 3.5, with a breaking change in 4.0 (but that would be adding complication etc and may not be trivial).

My challenges when updating shaders is that shader code can appear (sometimes embedded) in a few different file types (.shader, .tres, .tscn) so ‘write a regex’ is not as trivial as it sounds…

I agree it’s worth it to be correct in this case, even if there’s only a small chance of a driver rejecting shader compilation due to this issue. It’s extremely important to me to avoid any errors that only appear at runtime on diverse hardware.

This results in a breaking change… fine for 4.x but seems unnecessary in 3.x.

I understand your concern, if we could easily backport the warning system in shaders, this would have been better implementation as a warning in 3.5 and an error in 4.0

I mean if someone needs to strictly enforce the specification shouldn’t that be up to them, considering the long history of this being valid syntax? I know Godot “loosely” follows semantic versioning but that’s a lot of broken shaders

Yes and no. Our goal with Godot is that the same code runs on all supported platforms. We don’t expect that users know the GLSL spec, accordingly, we need to ensure that shaders that ship with Godot projects will actually run on all target devices. Without this change, there is a risk that users will write shader code that breaks randomly on some devices. While that wouldn’t necessarily happen frequently, it would be very hard to debug for many users. At the time of merging, the cost of updating shaders appeared to be outweighed by the risk of shaders breaking randomly for some users.

On the other hand, I understand how frustrating it is for a tool to suddenly decide that something that was valid is no longer valid. If it helps, previously we only allowed the 1f syntax because we didn’t have a system in place to ban it, not because we intentionally exposed it to users.

Since it sounds like you have a project in development, could you please let me know approximately how much effort is involved with updating your shaders? My intuition is like Calinous above, that this can be fixed up with some regex pretty quickly, but if it is a difficult and labour intensive process for most users then we may want to rethink the error.

This is not a “Known issue”, it’s an intentional change. It should be added to the CHANGELOG in the “Changes” section, not as a regression in the release notes.

“Known issues” are either regressions that we found out before the release and couldn’t fix, or that we found out after the release and which are critical enough to be mentioned in the release notes. This is not a regression but a breaking change.

It would have been nice to know up front what I was in for instead of having to Google around to find out what happend.

Done.

Looks like it’s not supported by glsl. Although I wonder why it was working fine before.

https://stackoverflow.com/questions/54000835/why-does-the-f-suffix-when-defining-floats-sometimes-cause-glsl-compiler-error