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

[rambda]

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

[Calinou]

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.

[clayjohn]

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

[Chaosus]

I think I've fixed that syntax in #55623. I also think that it should not be returned.

[clayjohn]

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

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.

[dmaz]

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

[piratesephiroth]

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.

[Calinou]

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.

[clayjohn]

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.

[jamie-pate]

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.

[ZodmanPerth]

I just upgraded from 3.4.2 to 3.5 and hit this issue. While it was inconvenient for my shader code to break I would rather be compatible with GLSL given what I'm trying to do on my project.

That said, I think it would be worthwhile to add this to the list of known issues for 3.5. After all, the known issue list on the announcement page states:

We'll update this list with any further regression that the first users of 3.5-stable may report.

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.

[Calinou]

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.

[ZodmanPerth]

Thanks @Calinou. If there is a way to ensure it gets put on future v3 releases as well (v3.5 rc1, v3.6, etc) we should probably do that too.

[akien-mga]

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.

[akien-mga]

I put it in the "Removed" section so it's more prominent as compat breaking, and not just behavior changing.

Fixed by #65459.

[ZodmanPerth]

Thanks for the clarification @akien-mga . It's better to have it in a more permanent place.

I can't see it in the removed section for 3.5. Does the change log need to be regenerated or something?

[Calinou]

I can't see it in the removed section for 3.5. Does the change log need to be regenerated or something?

The changelog in the blog post is separate from the CHANGELOG.md file, which is being edited by #65459.

[ZodmanPerth]

@Calinou Can you please drop a link about the changelog you are referring to? I'm expecting to see it in either this curated changelog or this chronological changelog but I can't.

I want to ensure that as I take on new builds I know up front what issues I'm going to have with my project.

[akien-mga]

https://github.com/godotengine/godot/blob/3.5/CHANGELOG.md

This was committed by the PR linked above 2 days ago, it can't be retroactively added to the 3.5-stable tag from August without rewriting the whole Git history.

[ZodmanPerth]

OK great. Thanks folks.

[AndreSacilotto]

If someone want a Regex for this I used:

1. Search: (\d*.\d+)f Substitution: $1
2. Search: (\d+)f Substitution: $1.0

The sequence is important.

[akien-mga]

I don't think these regexes are correct, what you want to replace is:
1f -> 1.0f
Existing code which is correct doesn't need the f stripped, it's only <int>f which is invalid.