preserveUniforms clash with function parameters

[mariusgabris]

Hi Leo,

I've noticed the following issue while have preserveUniforms on true, it's not a critical issue but I think you might be interested in this one. Input 1: uniform float scale; float test(float scale){ // do something with the parameter }

Expected result 1: uniform float scale;float A(float B){}

Actual result 1: uniform float scale;float A(float scale){}

Input 2: float test(float scale){ // do something with the parameter } uniform float scale;

Expected result 2: float A(float B){}uniform float scale;

Actual result 2: float A(float B){}uniform float B;

The problem I see is that the reserveKeywords should get an argument of variableType and mark the ones from the function parameters with a type. Then the condition from minifyToken should first filter non uniforms out and then check for existing. What do you think?

Keep up the good work, Marius

[leosingleton]

Interesting catch. Unfortunately, I don't see an easy fix here--it's really just a limitation of the simple one-pass parser I used for the minification. As you've seen from the code, it's not actually building up a full AST and parsing the GLSL language--it's just a quick one-pass translation using 2 lookback tokens. The parser isn't aware that the scale token may be a uniform in some places and a local variable in another.

There's quite a few other optimizations that could be done with a more sophisticated parser that parses the full tree and understands block scopes, like reusing variable names or eliminating dead code. But I haven't really had a need for it myself.

[mariusgabris]

I know, I tried to look for an easy fix myself before I posted the issue. But as I said, it's not a major issue and if we pay attention not to fall in the second scenario, it's fine.

[micahjon]

@mariusgabris , we ran into this as well and ended up throwing en error whenever a uniform is conflated with a different variable and minifed. You can see the code here: https://github.com/befunkyinc/esbuild-plugin-glslify-minify/blob/main/lib/minify.js#L263

Not an amazing solution, but at least this way it doesn't happen without you realizing it.