Possible shader compiler bug with single line comments

Bugs
1

I know GLSL is riddled with bugs and caveats, but it seems like there is a straightforward bug with the shader compiler in babylon around handling of single line comments. Reproduction:

I also recreated in the PG just to be sure:

In all of these, there is syntax error caused by the commented out line. Delete that commented line before the return, and then no error. Also, changing to a multiline comment (with /*... */) resolves.

It seems as if the compiler is trying to parse out the for loop even if itā€™s commented.

1 Like
2

Looking at the code in the console, it looks like each semicolon in the single-line comment is causing a line break: what comes after the semicolon is on a new line which isnā€™t commented outā€¦ :frowning_with_open_mouth:

// This will stay commented out;this will be on the next line

// First line;second line;third line

https://cyos.babylonjs.com/#UYFW2F#1

1 Like
3

Thanks @Blake, that helped me know where to look.

I submitted a PR to resolve the issue (and a related issue if ever using multiline indented macros):

3 Likes
4

Amazing !!!

5

This got more complicated. I think Iā€™ve discovered three issues that are semi-related. The PR now only addresses the first one.

  1. Having semi-colons in single line comments breaks things. The PR addresses this and hopefully the CI shows no issues this time.

  2. Multiline macro definitions have a similar issue with semicolons.

  3. Single line comments are not always parsed correctly. For instance, using single line comments on a multi-line macro can still cause processor errors on commented code.

I think addressing (3) requires a better understanding of the processing than what I currently have (or have time for). I would think that whatever parses out multiline comments, should also handle single line comments. I havenā€™t found where that it is yet though. Fixing that might negate need for the change I made in this PR.

Addressing (2) might be more difficult, and maybe not worthwhile. I suspect multiline macros are used rarely, and even less with statements containing semi-colons. I think to fix would require explicit checks for #define state and \ terminators.

Here is a PG that can be used to show all three issues. In the currrent PG (before this PR is merged), it will error unless the line with the commented for loop is removed. Modifying to see the other two issues is explained in comments:

2 Likes
6

I had this problem too. I ran all my shaders through a ā€œSanitizeā€ function before trying to load them.

export function sanitizeShader(shader: string): string {
    return shader.replace(/\/\*[\s\S]*?\*\/|([^\\:]|^)\/\/.*$/gm, '$1');
}

Not ideal but it got me over the hump.

1 Like