search

laurentlb / shader-minifier

Minify and obfuscate GLSL or HLSL code
https://ctrl-alt-test.fr/minifier/
Apache License 2.0
424 stars 29 forks source link

Opt out of "Merge for-declaration with the previous declaration" #397

Closed thscott closed 1 month ago

thscott commented 1 month ago

Would it be possible to have a way to opt out of the change introduced in #371?

The form int a=2, b=3; for (; ...) is not valid in GLSL ES, and hence not valid in WebGL 1.

See Appendix A of the spec here, specifically these lines:

  • The for statement has the form:
    • for ( init-declaration ; condition ; expression ) statement
  • init-declaration has the form:
    • type-specifier identifier = constant-expression

Consequently the loop variable cannot be a global variable.

laurentlb commented 1 month ago

Nice finding, thanks! I should just rollback this commit. The optim is not worth it, and there was another related bug report (#375).

therontarigo commented 1 month ago

I'd rather not lose this optimization because of such an ancient GLES version. Checking the compression stats, the only shaders for which it hurt compression were those also using --no-renaming - which breaks the context models anyway, so they shouldn't be trusted too much.

add --language option with hlsl, glsl, glsl100es glsl300es as choices since we already have --hlsl option for language selection, only a new --version option with 460 100es 300es as choices would be needed. With version selection we don't need an optimization-specific flag here, even if this is the only thing it will affect thus far. Webgl1 (100es) is the only lang with this restriction. Desktop GLSL also allows int literals in more places than 300es does, though this isn't exploited yet.

laurentlb commented 1 month ago

In https://github.com/laurentlb/shader-minifier/commit/5b8f9453863dab087460a1af73de1eca277e9e0f#diff-5a7085bc7afa14a48d0b7c3c68992949b5c04e13ef2f76a46eb17d106b6c6d62, the compression tests are all done with the default flags and include renaming (so they don't match the .expected files). Based on that, this optimization doesn't save space after compression.

A reason to keep it is that it can be useful for people who care about the uncompressed size. It may also help future optimizations (maybe).

therontarigo commented 1 month ago

Thanks, clearly I was thinking compression inputs differed only by --format text. I don't mind seeing it disabled for now. This puts #398 as low priority, though it relates also to another optimization I'm currently working.