I'm curious if there's a better way to handle dynamic uniform updates.
Currently, when using dynamic uniforms, onUpdate
callbacks are defined on each THREE.Uniform
. Unfortunately, if there are several uniforms for a given material that need dynamic updates, this means multiple callback executions per render call. Based on Chrome's profiler, I'm a little concerned about how much time is spent in evalDynamic()
in my app.
Would it be acceptable to rework the renderer's handling of render list items a bit, and instead allow an object to define a single callback to update multiple material uniforms?
For example, something like this:
var object = new THREE.Mesh();
object.onRender(function(material, camera) {
material.uniforms.textureWeight.value = this.textureWeights[material.textureWeightIndex];
material.uniforms.localDiffuseColor.value = this.localDiffuseColor;
... etc etc ...
});
or flipped around:
var material = new THREE.ShaderMaterial();
material.onRender(function(object, camera) {
this.uniforms.textureWeight.value = object.textureWeights[this.textureWeightIndex];
this.uniforms.localDiffuseColor.value = object.localDiffuseColor;
... etc etc ...
});
I'm not sure if the overhead of checking for the presence of an onRender
callback for each object in the scene graph would be more taxing than the cost of calling multiple callbacks per material with dynamic uniforms.
I'm happy to do the work on this, but I'd like a bit of input on whether I'm barking up the wrong tree, or there are other plans to improve uniform handling.
[ ] ...
[x] All of them
[ ] Internet Explorer
[x] All of them
I feel that I proposed exactly this in the past also. I would vote for placing it on the Mesh
(Object3D
).
Sounds good!
I can take a look at this later this week. I realize this might break @arose's code again (a la the recent update to include material in the callback params), so apologies about that.
I found where it was previously suggested: https://github.com/mrdoob/three.js/pull/7048#issuecomment-185489120
I realize this might break @arose's code again (a la the recent update to include material in the callback params)
Actually, it doesn't have to break. I think we can keep both code passes?
Hmm, true. I was thinking this would supersede dynamic uniforms, but it certainly doesn't have to.
What do we want to name the callback? onRender
? onWillRender
?
I think onRender
...
Ok. I've added a onBeforeRender
callback to start. WebGLUniforms need some refactoring but this works:
var gl = renderer.getContext();
function onBeforeRender() {
if ( this.material.program ) {
this.material.program.getUniforms().map.diffuse.setValue( gl, this.userData.color );
}
}
object.onBeforeRender = onBeforeRender;
Just to confirm, in the above example, if I'm using MultiMaterial
, I'll need to iterate over every element in this.material.materials
from inside the onBeforeRender
context, right?
In this first implementation, yes. We can improve things on the next cycle.
Sounds good!
This seems much more elegant.
Was something like this.material.program.getUniforms().map.diffuse.setValue( gl, this.userData.color );
even possible we implemented dynamic uniforms initially?
Was something like
this.material.program.getUniforms().map.diffuse.setValue( gl, this.userData.color );
even possible we implemented dynamic uniforms initially?
Not sure...
@tschw FYI, I'll be changing WebGLProgram
and WebGLUniforms
so the API looks more like this:
this.material.program.getUniform( 'diffuse' ).setValue( this.userData.color );
Not to offend anyone, but the dynamic uniforms are what spawned this?
Can we at least add on onHasRendererd
too? There are sooooooooo many other things that can be done with this, mainly via gl
call injection. For example one is working with the stencil buffer which wasnt supported at r75 or so when i left fiddling with this.
I see there is no more bind, but call has some overhead too?
https://github.com/mrdoob/three.js/pull/7048#issuecomment-185734213
I understand a bit of three.js, and a bit of javascript to know that it's definitely better to scope this function once somewhere during construction or configuration rather than have a bind/call call that changes the scope within a loop within a render loop. I'd also like to use this to do stuff with the stencil buffer, which could require a clean up, (can be right after the drawcall, can be after rendering the scene etc), what i don't know is git, i have trouble making pull requests and keeping up with the rate of updates on three.js.
Could i have been pinged here considering that i've given the suggestion and try to do the pull request? I'm really curious why this pull wasn't accepted, nor why we haven't discussed the performance. It's basically the same exact thing that's being addressed here, and the solution. I've tried to do it as clean as possible, but was missing an example, even though i've listed a bunch of suggestions in code:
https://github.com/mrdoob/three.js/pull/7806
I've also experimented with not having this just before the object drawing but within the renderBuffer...
as well. You might want to render multiple geometries sequentially to mimic "render passes". The camera and TRS uniforms get set outside the shader interface anyway, so you might leverage that for some free performance boost.
Also onBeforeRender
might be a bit ambigous, lots and lots of things can be happening before render, unity and other engines seem to be using will
, it was also the wording in my original suggestion and the unsuccessful pull request. It would imply that the rendering is going to happen immediately after the callback. Say we have a onWillUpdate
- it would also kinda qualify to be before the rendering.
Please please please consider something more than just the dynamic uniforms. It might be me, but i never liked the idea and the bind inside the nested loops, and with this, it may actually become a bit redundant, it's now just a uniform that you change, like anything else you might want to do in this call. IE. it would seem slightly less automatic to warrant it's special class, yet we haven't implemented the second half of this solution.
I found where it was previously suggested: #7048 (comment)
There was also a pull:
https://github.com/mrdoob/three.js/pull/7806
I realize this might break @arose's code again
The dynamic uniform becomes somewhat redundant:
var sharedMat = new THREE.ShaderMaterial();
var myMesh = new THREE.Mesh( geom , sharedMat );
myMesh.onWillRender = function(){
this.material.uniforms.myUniform.value = this.customData; // it's just a REGULAR uniform that gets set via a different mechanism (will Render callback)
}.bind(myMesh);
Uniforms are kinda dynamic by their very nature, it's a variable that is being sent to the shader, it's prone to change. I really fail to see why is such a powerful feature being driven by something so esoteric. This shouldn't be in any way tied to the dynamic uniforms, the dynamic uniforms should be one of the unlimited amount of things you could do with this callback.
I didn't follow everything, but saying that something broke this code "again" is not a great sign. I also thing it's prompting users to approach this problem in a wrong way. If i remember correctly, instancing was the way to go, otherwise, render callbacks. Bottom line, there is no need for a DYNAMIC uniform. _The uniform is always dynamic, it becomes a problem of timing - choosing when to update it (every time before a draw call, on window update, on gui change, etc etc)_
Dunno off the top of my head but doing
blah.onWillRender = function(){};
//instead of
blah.onWillRender = null;
and then
blah.onWillRender()
//instead of
if(blah!==null) blah.onWillRender()
might be worth a perf test too
I feel that I proposed exactly this in the past also. I would vote for placing it on the Mesh (Object3D).
You don't want this in mesh alone, you more likely want it before a drawcall, so while Object3D doesn't draw anything, a lot of the subclasses do. So, if you want to turn on the stencil buffer before rendering Points
, you'd want to have this, not have it in Mesh
alone. One thing you can count on is that all the objects encountered in the render loop are actually rendering something.
Further things can be done if you have this mixed in the geometry too. (have one mesh render a sequence of it's geometry, each time with a different callback)
Based on Chrome's profiler, I'm a little concerned about how much time is spent in evalDynamic() in my app.
It's probably got more to do with this?
@tschw
You really don't want to use the bind method in performance-sensitive library code: It's a whole lot slower than using a function with an upvalue, where "a whole lot" refers to a browser-dependent ~100 - ~7000% time overhead per call.
See https://jsperf.com/bind-vs-emulate/41 and earlier revisions.
Which goes away if you just have a callback with a bound scope, rather than doing it on the fly.
One might also want to pass a few arguments out from the renderer. GL context, camera perhaps, geometry, useful stuff. Mine ended up looking like this in my fork of r75
object.onWillRender( object , _this , camera , fog , _gl , group ); //_this is an instance of webglrenderer
:'(
I'm in favor of adding a companion onAfterRender
callback, as suggested by @pailhead above.
As an aside, even though it's a bit ambiguous, I think I prefer the semantics of onBeforeRender
and onAfterRender
, as opposed to onWillRender
and onHasRendered
. In the end, I don't think onWillRender
or onHasRendered
are any clearer about the immediacy of the draw call relative to the callback. A bit of documentation would probably clear that right up.
To briefly dip into what @pailhead proposed, would something like this be acceptable as an enhancement / replacement to the way the onBeforeRender
callback is currently implemented?
if ( object.onBeforeRender !== null ) object.onBeforeRender( group, geometry, material, camera, _gl, _this );
_this.renderBufferDirect( camera, fog, geometry, material, object, group );
if ( object.onAfterRender !== null) object.onAfterRender( group, geometry, material, camera, _gl, _this );
Re: the addition of arguments to the callbacks: I know I'd immensely prefer getting the specific material being rendered as part of the callback, rather than having to constantly iterate across all materials in a MultiMaterial
. I can also see a benefit to getting access to the underlying WebGL context and the specific renderer instance.
It's really a one liner with no bind/call calls in the render loop within the game loop.
Here's another attempt. I disagree with the semantics, but since you've already done the before
here's an after...
I've also added some useful arguments that one might need,
https://github.com/mrdoob/three.js/pull/9738
@mrdoob
To reiterate, "dynamic uniforms" become obsolete. They now become a problem of timing the update. Uniforms are always dynamic because it's pretty much the only thing one should update in the shader (minus some attributes some times, and instanced attributes?).
So instead of having a complicated system all over the place in the renderer, the checks, the evalDynamic
with it's bind/apply/call calls that happen per every object in the scenegraph in every frame, you use these simple 2 lines to just time these uniform updates whenever you want.
A dynamic uniform for one user would change onButtonPressed()
for example, for another would be onWindowResized()
, yet for others onWillRenderObject()
. In either case, what are those uniforms if not "dynamic".
@tschw @mrdoob @arose
@arose could you update webgl_interactive_instances_gpu
using the onBeforeRender()
approach?
could you update webgl_interactive_instances_gpu using the onBeforeRender() approach?
yes, will do.
I am trying to update webgl_interactive_instances_gpu using the onBeforeRender() approach.
First, it is not as simple as material.uniforms.color.value.setHex( object.userData.color )
. This does not work because updating the material uniforms is behind the refreshMaterial
flag in setProgram
which is false
when the material has not changed.
I tried setting the WebGLUniforms directly with material.program.getUniforms().setValue( gl, "color", object.userData.color )
. Apart from GPU picking this seems to work but gives the error "WebGL: INVALID_OPERATION: uniform3fv: location is not from current program". Note that I also had to make gl
and object
available to the onBeforeRender
callback.
Open to any suggestions. Here is the branch with what I have so far https://github.com/arose/three.js/commit/6d62c64fd78cf5b6bf6a67433d5f531349fa6e93.
I tried setting the WebGLUniforms directly with
material.program.getUniforms().setValue( gl, "color", object.userData.color )
. Apart from GPU picking this seems to work but gives the error "WebGL: INVALID_OPERATION: uniform3fv: location is not from current program".
Uhm. Tricky...
Note that I also had to make
gl
andobject
available to theonBeforeRender
callback.
You can do var gl = renderer.getContext();
inside the callback.
You can do var gl = renderer.getContext(); inside the callback.
We should test the performance of this somehow, might be better to cache gl yourself somewhere outside the callback, or pass the cached one from the renderer?
You can do var gl = renderer.getContext(); inside the callback.
done https://github.com/arose/three.js/commit/2c9700ddd31c65134dc3089f908ab485580a6ac6
I tried setting the WebGLUniforms directly with material.program.getUniforms().setValue( gl, "color", object.userData.color ). Apart from GPU picking this seems to work but gives the error "WebGL: INVALID_OPERATION: uniform3fv: location is not from current program". Note that I also had to make gl and object available to the onBeforeRender callback.
Ungh, this seems like a problem, it defeats the purpose if you need to set uniforms through a low level gl call. Managing those pointers and stuff can get gnarly.
I must admit that im a bit fuzzy on what exactly goes on in setProgram, i need to read the code a bit, but is there a higher level flag that would trigger this update, like material.needsUpdate
or something? Even if this works though, performance wise, it might make more sense for this example to do it through gl
?
I've finally had a chance to give onBeforeRender a whirl in the project I'm working on, and as @arose and @pailhead pointed out above, it's indeed rather ugly to manipulate uniforms from the callback as is.
@mrdoob Do we want to continue the discussion about working around refreshMaterial
in this issue, or should we do that in another issue?
Most helpful comment
yes, will do.