Shader parse error on working shaders

[CallumBugajski]

Distribution

Linux x64 APT package

Bug description

When compiling a pack that contains Shaders from objmc (https://github.com/Godlander/objmc), I've been getting errors from Shaders that work correctly within Minecraft. Seems like they cannot be parsed but are correctly formatted.

Output:

PackSquash v0.3.1-8-g349eaf0 (release, 2022-04-14) for x86_64-unknown-linux-gnu
Minecraft resource and data pack optimizer (CLI)

This program comes with ABSOLUTELY NO WARRANTY.
This is free software, and you are welcome to redistribute it
under certain conditions. Use the -v command line switch for
more details about these conditions.

- Reading options from file.toml...
- Options read. Processing pack...
- Working around automatically detected Minecraft quirks: bad_entity_eye_layer_texture_transparency_blending
! assets/minecraft/shaders/include/objmc_light.glsl: Shader parse error: error: 0: at line 5:
  if (isCustom == 0) {color *= vertexColor;}
  ^
  expected ';', found i
  
  1: at line 5, in Alt:
  if (isCustom == 0) {color *= vertexColor;}
  ^
  
  2: at line 1, in Many1:
  //objmc
  ^
  
  
! assets/minecraft/shaders/include/objmc_main.glsl: Shader parse error: error: 0: at line 4:
  isCustom = 0;
  ^
  expected ';', found i
  
  1: at line 4, in Alt:
  isCustom = 0;
  ^
  
  2: at line 1, in Many1:
  //objmc
  ^
  
  
! assets/minecraft/shaders/core/render/block.vsh: Shader parse error: error: 0: at line 43:
      #define BLOCK
      ^
  expected '}', found #
  
  
! assets/minecraft/shaders/core/render/entity_overlay.vsh: Shader parse error: error: 0: at line 52:
      #define ENTITY
      ^
  expected '}', found #
  
  
! assets/minecraft/shaders/core/render/entity.fsh: Shader parse error: error: 0: at line 36:
      #define ENTITY
      ^
  expected '}', found #
  
  
> assets/minecraft/shaders/include/fog.glsl: Minified
> assets/minecraft/shaders/core/rendertype_cutout.json: Minified
> assets/minecraft/textures/block/outworld.png: Optimized with 100% quality color quantization
> assets/minecraft/textures/block/outgui.png: Barely optimized. If not optimized externally, try tweaking options for extra savings
! assets/minecraft/shaders/core/render/entity.vsh: Shader parse error: error: 0: at line 50:
      #define ENTITY
      ^
  expected '}', found #
  
  
> assets/minecraft/shaders/core/rendertype_translucent.json: Minified
> assets/minecraft/textures/block/outhand.png: Optimized with 100% quality color quantization
> assets/minecraft/shaders/core/rendertype_item_entity_translucent_cull.json: Minified
> assets/minecraft/shaders/include/objmc_tools.glsl: Minified
> assets/minecraft/shaders/core/rendertype_solid.json: Minified
> assets/minecraft/textures/block/shuba.png: Optimized with 93% quality color quantization
> assets/minecraft/textures/block/cube.png: Optimized with 97% quality color quantization
! Pack processing error: An error occurred while processing a pack file
  Another error message with more information was emitted before. You might need to scroll up to see it.
  These troubleshooting instructions might be useful: <https://packsquash.page.link/Troubleshooting-pack-processing-errors>

Reproduction steps

1. Download the following folder from objmc (the resource pack example): https://github.com/Godlander/objmc/tree/main/objmc
2. Use the pack squash command on the downloaded folder with the basic config file:

pack_directory = 'PATH_TO_PACK'
output_file_path = 'PATH_OUTPUT'
zip_spec_conformance_level = 'disregard'

Expected behavior

The pack should compile normally

Attached files

No response

Additional context

No response

[AlexTMjugador]

Hi! Thanks for reaching out to report your issue 😄

As it happens with JSON files, the GLSL parser used by PackSquash is more strict than Minecraft. However, unlike with JSON files, I did not intend the GLSL parser to be that way, so I agree that PackSquash should improve in this regard, and understand why it may be seen as a "bug". (There is a point in enforcing strict JSON syntax, because JSON is meant to be a data exchange format, and Minecraft accepts deviations that other commonly used programs do not. However, GLSL is rather application-specific, especially when considering the fact that Mojang extended it with preprocessor directives such as #moj_import.)

The errors you experience are caused by two known limitations, described in this issue comment:

• Preprocessor directives are only supported at the external declaration position (i.e. top level of a shader file). This means that PackSquash will spew errors if you use #moj_import inside a function, for example.
• Included shaders (.glsl files) are parsed as translation units (i.e. standalone, normal vertex or fragment shaders). Therefore, included shaders must only contain declarations. They can't contain expressions that would only be valid inside declarations, such as var = 3. Such included shaders are only valid if they are always #moj_import'ed within a declaration (function, data type definition, etc.), which is currently not possible due to the previous point, but it's still worth keeping in mind.

Luckily, these limitations can be worked around with no functional differences (i.e., without affecting how the shaders work):

• #moj_import directives can be replaced with the contents of the include shaders they import.
• Other preprocessor directives (i.e., lines that start with #) can be moved to the top level of the shader file.

I reckon that applying these workarounds at scale can be tedious, but they should be enough to get your pack to compile for now. Sadly, I don't expect them to be fixed soon, due to current technical constraints caused by how PackSquash is designed, which will be addressed for v0.4.0.

Please let me know if this comment was helpful to you.

[CallumBugajski]

Hi Alex,

Thank you for your quick reply, we'll give the workarounds a try and I'll let you know if we need anything else.

Thank you, Callum

[Godlander]

Some preprocessor directives cannot simply be moved to the top of files, such as #ifdef, #else and #endif. These are not Mojang extensions and are part of OpenGL Specifications.

Although without the use of #moj_import these can be worked around by modifying the code in each individual shader to only use the parts that would be included after the preprocessor.

[AlexTMjugador]

Some preprocessor directives cannot simply be moved to the top of files, such as #ifdef, #else and #endif. These are not Mojang extensions and are part of OpenGL Specifications.

Right, I wasn't thinking on those! Indeed, they are part of the specification. And, while the specification is not crystal-clear about it, I think that PackSquash is wrong here, because the specification makes several statements that equate the GLSL preprocessor with the one you'd find on C.

The only reason for this particular limitation to exist is that the fork of the third-party GLSL parser PackSquash uses has it. Upstream has been unresponsive for quite some time now and AFAIK there are no better GLSL parsers PackSquash could use, so any fix will likely be made by me or other PackSquash contributor on that fork. Sadly, time is finite for me now, and I'm prioritizing getting v0.4.0 released, but PRs are welcome 😄

[Godlander]

That's understandable, and it is possible to work around the limitation so it is not a big deal.

I believe it can be resolved by keeping the newlines around any line starting with #, but that might be a suboptimal solution.

[gp-Airee]

Is there a way to turn off the parsing of .glsl files or exempt specific files from processing?

[AlexTMjugador]

Is there a way to turn off the parsing of .glsl files or exempt specific files from processing?

I'm afraid that PackSquash does not provide any options to do that, sorry. Would you mind elaborating on why are you interested in disabling such parsing? Are the discussed workarounds impractical in your scenario?

Ideally, I would like to address this issue by fixing the PackSquash GLSL parser rather than letting people sweep its problems under the rug.

[Joshinn-io]

I am currently in your # | packsquash support channel in discord, but we're hoping to get the minify-shaders option to work to skip trying to parse glsl files. I sent the example of how we tried to use it, but it didn't seem to work.

[AlexTMjugador]

Note to self (and potentially other interested parties): the naga crate offers the GLSL validation and transpilation features required by PackSquash and seems much better maintained and widely used. It might be worth switching to it to get rid of the issue of restrictive preprocessor directive positions. glsl-lang looks like another interesting alternative. glslopt may be an interesting addition, but it is abandoned and may not work fine with Minecraft or newer GLSL constructs. glsl-include may be useful as a base to implement #moj_import resolution and proper validation in this case, although it's likely it can't be used as is due to PackSquash design constraints.

[UltraFaceguy]

I recently wrote some shaders and was very disappointed to realize I can't release an update because of this. Definitely is "at scale" enough for the level of frustration to workaround is too high. I support an option to skip shader validation. No reason not to throw in a skip if the validator currently has known issues validating the base OpenGL Specifications, at least until it's actually accurate.

[AlexTMjugador]

I am aware of the disappointing reality that some shaders can't be used as-is with the current PackSquash release. I am also aware that it a fix for the situation is warranted and necessary, especially in situations where refactoring shaders is not a viable option. I'm sorry for the inconvenience it causes to some users.

However, I won't add an option to skip parsing GLSL files. I don't believe that is the proper way to approach this issue for several reasons:

• The GLSL parser used by PackSquash is not black magic that no one can understand: as mentioned above, it could be changed to a better one without much effort to fix the most egregious issue of rejecting preprocessor declarations in most positions. It just has not been done yet, and at the time this feature was implemented there were no alternative parsers. My time is limited and, while shaders are important for some people, others value improvements in other regards more, such as performance, protection, ease of use and compression in other asset types. I want to make all of them happy, but that is not a trivial task.
• An additional option to skip parsing GLSL would default to some value. Or should it? If I make it mandatory, it'd be cumbersome for those who don't use shaders in their packs. Defaulting to not skip parsing would mean that people would still face this issue, and maybe even ask for support, before realizing that such an option exists. On the other hand, defaulting to skip parsing would render the minification feature useless by default, even though only a subset of shaders are affected by this issue (I know that some popular armor shader packs are affected by this, but they might not be representative of the whole population of shaders in the wild).
• Validating shaders aligns with my vision of what I want PackSquash to do. It'd be next to impossible to improve upon its compression capabilities without having more knowledge about specific data formats and their validity. In the long term, offering an escape hatch for validation is likely to be detrimental towards gaining such knowledge, as it renders it easier to form a "it just works this way" attitude towards things that is not technically constructive.

That being said, while I appreciate feedback and understand that the problem can be frustrating, I'd like to kindly ask further commentors to avoid further "I'm affected by this too" comments, unless they add something new and constructive to the topic. I want to make PackSquash as good as it can be 😄

[UltraFaceguy]

Well I can respect having a vison and wanting things to align to it, my vision as an end-user is to compress a valid resource pack.

All the reasoning in the world can't work around that fact that I have a valid formatted resource pack with GLSL spec compliant shaders and can no longer use this project to compress it. Even if only a "subset of shaders" are affected at the end of the day preventing a project from failing to perform it's core purpose over the prospect of a temporary config boolean seems a bit silly.

[Darwin1546]

This project declares itself as Minecraft: Java Edition resource and data pack optimizer
Not as Minecraft: Java Edition resource and data pack parser