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

Basic shader generation refactoring #6250

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Apr 12, 2024

This PR uses recently added support for #include in the Preprocessor (#6242) to refactor the basic shader generation. Instead of a JS code that glues strings and chunks together, it creates a list of defines based on the generation options, and the vertex and fragment code is simply processed with those. This makes it a lot easier to understand the shader code, which is now specified as a single string.

This is a prototype of where we want to go with the standard shader in the future.

New public API:

  • Shader.constructor not accepts list of includes and defines for both vertex and fragment shaders, which are used to drive the shader code processing.

@mvaligursky mvaligursky self-assigned this Apr 12, 2024
@mvaligursky mvaligursky added area: graphics Graphics related issue enhancement labels Apr 12, 2024
@mvaligursky mvaligursky requested a review from a team April 12, 2024 11:44
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.

as the weird clown says, I'm lovin it

@mvaligursky mvaligursky merged commit af7d32c into main Apr 12, 2024
7 checks passed
@mvaligursky mvaligursky deleted the mv-basic-shader-refactor branch April 12, 2024 12:28
import { ShaderGenerator } from './shader-generator.js';

const vShader = `
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const vShader = `
const vShader = /* glsl */`

}
`;

const fShader = `
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const fShader = `
const fShader = /* glsl */`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants