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

Material.onBeforeCompile() enhancements #13446

Closed
2 of 3 tasks
pailhead opened this issue Feb 27, 2018 · 4 comments
Closed
2 of 3 tasks

Material.onBeforeCompile() enhancements #13446

pailhead opened this issue Feb 27, 2018 · 4 comments

Comments

@pailhead
Copy link
Contributor

pailhead commented Feb 27, 2018

I would like a more robust Material.onBeforeCompile system to work with the current state of the default materials.

  1. It's not actually documented, but from the example it's not obvious that there is a very specific way in which the callback needs to be written. Any conditional logic inside the callback may or may not be executed depending on the order in which the materials have been added to the scene or encountered by the first render call.
    See:
    [BUG]: onBeforeCompile hashing #13192
    https://codepen.io/anon/pen/KQPBjd
    The workaround for this seems extremely hacky, and one has to think of functions/callbacks in a specific way, if that makes sense. Literally the raw code you write in the callback matters. I would like to not think of this function as data, but the stuff inside it as data. If you are writing some kind of a utility that somehow configures onBeforeCompile in different ways, rather than writing:
function myOnBeforeCompile(shader){
  if(someUniform){
    //replace some chunk
  }

  if(someArbitraryProperty){
    //replace some chunk
  }
 
  if(someCondition){
     //replace multiple chunks
  }

}
someMaterial.onBeforeCompile = someUniform ? onBeforeCompileForCombinationFOO : someOtherOnBeforeCompile
//or maybe something like this, but it feels weird, 
//whatever this Manager would do would probably look gnarly
someOtherMaterial.onBeforeCompile = OnBeforeCompileManager.getOnBeforeCompile( someUniform, someProperty)
  1. It's kinda verbose, and really wants me to understand that shader is a string. I would like it to be more like data. Instead of these chained "do stuff to string" calls, i would like to have a configurable object where i can say { chunkName: chunkString}. If replace() is the only thing i will ever want to call in this callback, on fragmentShader or vertexShader it would be nice if it's hidden away:
someMaterial.onBeforeCompile = shader = >{
   shader.fragmentShader = shader.fragmentShader.replace('#include <foo>', myFoo)
   shader.fragmentShader = shader.fragmentShader.replace('#include <bar>', myFoo)
   shader.fragmentShader = shader.fragmentShader.replace('#include <baz>', myFoo)
   // don't like calling shader.fragmentShader = shader.fragmentShader 
   // don't like looking for #include <> if I know that it's a chunk and it's name is foo,bar,baz
   // don't like calling string.replace()
//chunks as data
someMaterial.replacableChunksOrWhatever.foo = fooGLSL  
someMaterial.replacableChunksOrWhatever.bar = barGLSL  
someMaterial.replacableChunksOrWhatever.baz = bazGLSL  
  1. It's not obvious where uniforms, functions, attributes and varyings should be injected. Prepending the shader in the callback should work, but causes a warning for example with #extension directives (there's more stuff that happens above the pre-compile shader). Again i'd like to see it more as data.
someMaterial.onBeforeCompile = shader =>{
  shader.fragmentShader = myPrependedAndFormattedUniformsVaryings +  shader.fragmentShader 
  //...
}
//i'd rather see it as data and not concern myself over the semantics of how it gets injected
someMaterial.customGLSL.vertexUniforms.foo = {type: 'v4', value: new THREE.Vector4()} 

I've tried to implement these changes in:
#13198

Thoughts @Mugen87 ?

Per discussion with @mrdoob all of this would go away in favor of all the materials being rewritten with the node material. Until that happens, i would like to see a way to interact with the default materials thats more robust.

I think this is called configuration over convention. I would configure these materials with custom stuff upon construction, and then not care about the string manipulation, and particular moments in time like compile time.

Three.js version
  • Dev
  • r90
  • ...
@Mugen87 Mugen87 changed the title [feature request] onBeforeCompile functionality Material.onBeforeCompile() enhancements Feb 27, 2018
@pailhead
Copy link
Contributor Author

pailhead commented Mar 2, 2018

The main idea behind this is based on an observation that passing data in seems like a more intuitive way of dealing with this problem. Configuration over convention?

(not aware of onBeforeCompile - asking to pass data, and configure)
#13364

(aware of onBeforeCompile it seems more natural / easy to monkey patch THREE.ShaderChunk, previous PRs did exactly this, except on material level not global)
#12977

(another expectation coming from someone not aware of onBeforeCompile)
#11562 (comment):

onBeforeCompile can be powerful, but replaceable chunks are easier to use, not?

(one of the first PRs to tackle this, configuration over convention)
Configure an object, pass it to material, no functions:
#7581

@lojjic
Copy link
Contributor

lojjic commented Sep 17, 2019

It's not exactly the same but in case others find it useful, I wrote this createDerivedShader utility as a way to easily modify builtin materials with custom shader logic for common things like vertex and gl_FragColor transforms, in a way that hides all the onBeforeCompile and regex messiness behind a simpler declarative interface: https://github.com/protectwise/troika/blob/master/packages/troika-three-utils/src/DerivedMaterial.js

I'm using it for lots of things now, such as rendering SDF text based on builtin materials: https://troika-examples.netlify.com/#text

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 17, 2019

@lojjic Tip: The forum (category Resource) is the ideal place to promote such work 😊.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2021

In the meanwhile I don't think it makes sense to put any more effort in an enhanced version of Material.onBeforeCompile(). It's more promising to focus on node materials which will be the standard material system of the new WebGPURenderer. This revised system will also be useable with the existing WebGLRenderer (see #21117).

@Mugen87 Mugen87 closed this as completed Feb 28, 2021
Repository owner locked as resolved and limited conversation to collaborators Feb 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants