Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(build): update rollup shaderChunks #5602

Merged
merged 4 commits into from
Aug 30, 2023
Merged

refactor(build): update rollup shaderChunks #5602

merged 4 commits into from
Aug 30, 2023

Conversation

epreston
Copy link
Contributor

improve glsl processing to compress build sizes by a modest amount.

Described in #5601

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

improve glsl processing to compress build sizes by a modest amount
Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - thanks for this @epreston! 🚀

@epreston
Copy link
Contributor Author

epreston commented Aug 30, 2023

Karma is a fitting name. I'll look into that. I haven't been running the karma tests so this is fair.

@willeastcott
Copy link
Contributor

Yeah, you have to be careful about being overzealous with trimming whitespace in shader chunks. 😄 I believe the old code prevented this.

@epreston
Copy link
Contributor Author

Running through it now. I'll update and document.

ensure final new line
@epreston
Copy link
Contributor Author

epreston commented Aug 30, 2023

shaders processing code requires a final new line for each chunk. doesn't affect the operation of the engine or examples but causes type warnings. Added final new line.

@kungfooman
Copy link
Collaborator

To quote @willeastcott from another PR:

I have a goal to eliminate as many files and configuration from the root of the repo as possible - this keeps it simple and accessible to contributors

I'm not sure why we're suddenly excited about watering down that position now?

@epreston
Copy link
Contributor Author

epreston commented Aug 30, 2023

This file is not in the root of the repo.

It is removing a chunk of code from root and moving it to utils. I feel this follows the spirit of that idea. Actually, I want to move all of rollup into utils. Good reminder. Don't want to get off topic.

@willeastcott willeastcott merged commit 5cb8fac into playcanvas:main Aug 30, 2023
7 checks passed
@epreston epreston deleted the rollup-shader-chunks branch August 30, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants