Skip to content

Conversation

@notash23
Copy link

This could be useful when using separate frag and vert files. The current defaults for Fragment code injection and Vertex code injection are made for three.js.

… to original code injection if stageName is not found.
@racz16
Copy link
Owner

racz16 commented Aug 20, 2025

Sorry, I'm a bit late, but now I have found the time to work a bit on the project and review this PR. Overall, it looks good to me, but I have some nitpicks.

  • Could you change vert to vertex, and frag to fragment in variable and function names? I know it's a very small thing, but I'd like to keep the names as I named things earlier.
  • I think now that we have 3 ways to inject code, we should remove the default injection code. One of them is Three.js, the other is ShaderToy; it doesn't make sense to inject them at the same time.

Comment on lines +171 to +176
const injectionSource =
this.stage === ShaderStage.FRAGMENT
? GlslEditor.CONFIGURATIONS.getFragInjectionSource()
: this.stage === ShaderStage.VERTEX
? GlslEditor.CONFIGURATIONS.getVertInjectionSource()
: GlslEditor.CONFIGURATIONS.getCodeInjectionSource();
Copy link
Owner

Choose a reason for hiding this comment

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

It'd be nice to refactor this snippet into its own function, and with ifs instead of nested ternary operators.

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.

2 participants