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

Preprocessor to resolve ifdefs is applied to shader programs #4156

Merged
merged 12 commits into from
Apr 5, 2022

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Mar 31, 2022

A new private class is added to preprocess defines and ifdefs in the shader with the goal of removing unused code. This is a first preparation step. Following steps will further modify the shader code, in order to convert uniforms to uniform blocks.

New (private) API:

/**
 * Run c-like proprocessor on the source code, and resolves the code based on the defines and ifdefs
 *
 * @param {string} source - The source code to work on.
 * @returns {string|null} Returns preprocessed source code.
 */
static Preprocessor.run(source) {

Note - Simple expression evaluation only is supported for ifdefs, handles cases:

  • expression
  • defined(expression)
  • !defined(expression)

It does not handle more complex cases, which would require more complex system:

  • defined(A) || defined(B)
    And so few use cases in the shaders were converted to the supported format.

Unit test has also been added.

Additionally, Debug logging functionality has been extended with trace function.
New (private) API (and also examples of use):

Debug.setTrace(`Preprocessor`, true);
Debug.trace(`Preprocessor`, "message")

and this is displayed in the console:
[Preprocessor] message

@mvaligursky mvaligursky self-assigned this Mar 31, 2022
@mvaligursky mvaligursky added enhancement area: graphics Graphics related issue labels Mar 31, 2022
@mvaligursky mvaligursky requested a review from a team March 31, 2022 12:54
@yaustar
Copy link
Contributor

yaustar commented Mar 31, 2022

Does this affect the sourcemap generation?

@mvaligursky
Copy link
Contributor Author

Does this affect the sourcemap generation?

This is only used to process shaders in memory .. so no.

@willeastcott
Copy link
Contributor

This is definitely something for @slimbuck to review - I'll pass.

src/graphics/program-lib/chunks/lit/clusteredLight.frag.js Outdated Show resolved Hide resolved
src/core/preprocessor.js Outdated Show resolved Hide resolved
src/core/preprocessor.js Show resolved Hide resolved
@mvaligursky
Copy link
Contributor Author

Updates based on comments @slimbuck
@willeastcott - could you please look at the Debug.trace part I added.

Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

We'll probably update the preprocessor over time, but this is a reasonable first pass.

rollup.config.js Show resolved Hide resolved
@mvaligursky mvaligursky merged commit 0720039 into main Apr 5, 2022
@mvaligursky mvaligursky deleted the mvaligursky-shader-preprocessor branch April 5, 2022 09:25
@mvaligursky mvaligursky mentioned this pull request Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants