Recursive #include-Directives causes "Maximum call stack size exceeded"

[WaldemarLehner]

Assuming we have 3 Shader files, main.glsl, a.glsl and b.glsl. main imports a, a imports b, b imports a. This results in a cycle between a and b.

The Plugin does not check if a shader was already included. It also does not check for Preprocessor Instructions like #ifndef which could resolve this issue.

I have set up a repo to reproduce here: https://github.com/WaldemarLehner/-vite-plugin-glsl-bug-report1

[WaldemarLehner]

I did a quick and dirty rewrite on my fork (https://github.com/WaldemarLehner/vite-plugin-glsl/commit/512eb5f026abe3d2b0c129857daad5d5b5701535#diff-00131d7bfec6394b6ba508bee3a23b03977cf1e22ee594e749a45283e9698657=) that basically keeps track of which files have been included. If the file is encountered again, the #include-Directive is dropped.

[UstymUkhman]

Hi @WaldemarLehner ! Thanks for starring, forking and opening an issue on this plugin. Also thank you for a "dirty rewrite", I was thinking about the same type of check for already included chunks, but I'm going to do it within the "loadShader.ts" file.

Although I'm sure your rewrite might work and it includes some useful logic about splitting parts of this plugin into different files, it's a little bit to radical and might introduce some unwanted issues I'm not aware of. That's why I'm not going to ask you to create a PR with that. Hope you understand. Anyway thanks for the hint and for your contribution, I will release a fixed version asap. 😊

By the way, I'm pretty sure #ifdef and #ifndef instructions are supported so they could potentially be used for a hacky workaround of this issue. In fact, you can check some examples of how I've used them to conditionally include shader chunks here and here (this is for the second example).

Cheers!

[UstymUkhman]

Update: Alright, I think I got your point now about including shader chunks with preprocessor conditions. You're right about the fact that this plugin does not actually excludes chunks based of the condition but rather incudes them all and then on run time the GPU will actually decide which part of the shader should be executed. Sorry for the misunderstanding. 😅

This is pretty fine to me because #ifdef, #ifndef and #define are GLSL instructions and use them to conditionally exclude files is beyond the scope of this plugin. I would prefer to stick with this logic and fix the main issue in another way. Working on it... 🙂

[WaldemarLehner]

Personally I only really use the #ifndef-Pattern to #include something once. So if the #include would make sure that a file is only included once, that would solve the issue for me :)

[UstymUkhman]

Having to track all included chunks and remove imports based on #defined constants (an also track those) will transform this plugin into some sort of a "compiler", a thing that I'd really like to avoid. An update I came out with is to throw a more informative error when the recursion occurs. It looks like this:

I know this is not an actual solution to your problem, but if you have to check for shader chunks being executed based on some preprocessors constants, I would recommend using an approach similar to the examples linked above. It will look something like this:

#define USE_CHUNK_1

#ifdef USE_CHUNK_1
  // This code will be executed
  #include "chunk1.glsl";
#else
  // This code will be included
  // but never executed
  #include "chunk2.glsl";
#endif

void main (void) {
  #ifdef USE_CHUNK_1
    chunk1FunctionCall();
  #else
    chunk2FunctionCall();
  #endif
  // ...
}

Of course, if you're sure chunk1.glsl and chunk2.glsl aren't going to conflict and no function or variable will be overwritten, you can always include both and use just the needed one:

#define USE_CHUNK_1

#include "chunk1.glsl";
#include "chunk2.glsl";

void main (void) {
  #ifdef USE_CHUNK_1
    chunk1FunctionCall();
  #else
    chunk2FunctionCall();
  #endif
  // ...
}

In my opinion this will lead to a cleaned and simpler way of tracking and debugging code that's actually in use and will avoid weird spaghetti-like file imports.

[terkelg]

This breaks when when using Lygia. Is there a way to disable this check?

[UstymUkhman]

@terkelg Not yet, but I can add an option to disable this check, thanks for reporting! Actually, v0.1.3 was a little raw, can you confirm please that the issue is still relevant with v0.1.5?

[terkelg]

@UstymUkhman I will try again end of week and get back to you!

[UstymUkhman]

Thanks @terkelg, I appreciate it!

[terkelg]

The issue still persists on the latest version. Updates seems to in incompatible with Lygia.

[UstymUkhman]

Ok, clear, thanks for checking. I will add an option to disable this check asap.

[UstymUkhman]

@terkelg I'm closing this as I've released v0.2.0 with a fix for false recursion errors. I've tested with some shaders in the Lygia library and it seems to work properly now. Although you may see a lot of warning messages in the console because of duplicated imports, so you might consider disabling those with warnDuplicatedImports set to false.

Thanks again for reporting this issue, if you'll find some lygia shaders that still trigger a "Recursion Error" feel free to open a new issue. Also, please consider giving a ⭐ to this project if you've found it useful, thanks!