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

object callbacks #9738

Merged
merged 2 commits into from
Sep 23, 2016
Merged

object callbacks #9738

merged 2 commits into from
Sep 23, 2016

Conversation

pailhead
Copy link
Contributor

@pailhead pailhead commented Sep 22, 2016

per previous suggestion:
#7048

per previous pull:
#7806

recent discussion:
#9662

example (copied)

I didnt have time to come up with a meaningful example, but i did see some other features that were implemented that could also benefit from this.

My usage case would be something like this:

stencil buffer management

sequencing two meshes and a mask to render in a specific order and make use of the stencil buffer which has very little exposure in Three.

var maskMesh = new THREE.Mesh( maskGeom , maskMat );
maskMesh.onBeforeRender = function(){
  gl.clearStencil( 0 ); 
  gl.clear( gl.STENCIL_BUFFER_BIT );
  gl.stencilFunc( gl.ALWAYS , 1 , 1 );
  gl.stencilOp( gl.REPLACE , gl.REPLACE , gl.REPLACE );
  gl.colorMask( false , false , false , false ); 
  gl.depthMask( false ); 
}.bind(maskMesh);
maskMesh.renderOrder = SomeOrder + 0;


var meshA = new THREE.Mesh( geomA, matA);
meshA.onBeforeRender = function(){
  gl.depthMask( true ); 
  gl.colorMask( true , true , true , true );
  gl.stencilFunc( gl.EQUAL , 0 , 1 ); 
  gl.stencilOp( gl.KEEP , gl.KEEP , gl.KEEP ); 
}
meshA.renderOrder = someOrder + 1;



var meshB = new THREE.Mesh( geomB , matB);
meshB.onBeforeRender = function(){
  gl.stencilFunc( gl.EQUAL , 1 , 1 );
}
meshB.renderOrder = someOrder + 2;

configuring a material before each call

Another example would be #7048 , but without the dynamic property and modifications to the renderer.

If there was a callback like this the same could be achieved with:

var sharedMat = new THREE.ShaderMaterial();

var myMesh = new THREE.Mesh( geom , sharedMat );

myMesh.onBeforeRender = function(){
     this.material.uniforms.myUniform.value = this.customData; 
}.bind(myMesh);

transforming objects that rely on other objects being transformed

var skybox = new THREE.Mesh(...);
var camera = new THREE.Camera();

var controls = new Controls( camera );
   ...

updateLoop(){
  controls.update(); 

 //now i want the skybox to "follow" the camera, ie. has the same position (or relative position) but no rotation

  skybox.position.copy( camera.position);

}

I wish that i could have one Vector3 be a position that many different objects can reference, but three doesnt work like that.

With a callback

skybox.onBeforeRender = function(){
   this.position.copy( camera.position );
}.bind(skybox);

@pailhead
Copy link
Contributor Author

@mrdoob

I actually tested, it seems twice as fast to just call an empty function than to test for null. Any suggestions on what to do, i'd like to change both the way you have it now, and the way i'm doing it in this pull, should i just disregard this one and do another one?

@@ -1525,12 +1525,15 @@ function WebGLRenderer( parameters ) {

} else {

if ( object.onBeforeRender !== null ) object.onBeforeRender();
if ( object.onBeforeRender !== null ) object.onBeforeRender( _this , _gl camera, fog, geometry, material, object, group );
Copy link
Owner

Choose a reason for hiding this comment

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

You forgot a comma after _gl.


_this.renderBufferDirect( camera, fog, geometry, material, object, group );

}

if ( object.onAfterRender !== null ) object.onAfterRender( _this , _gl camera, fog, geometry, material, object, group );
Copy link
Owner

Choose a reason for hiding this comment

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

Comma after _gl too.

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

Any suggestions on what to do, i'd like to change both the way you have it now, and the way i'm doing it in this pull, should i just disregard this one and do another one?

Just do another commit on your drawcallCallback branch.

@pailhead
Copy link
Contributor Author

hmm did this work?

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

It did!

@pailhead
Copy link
Contributor Author

pailhead commented Sep 22, 2016

amazeballz!

ill do a stencil buffer example from above, i think it's the most interesting since we don't have a high level interface for the stencil buffer and it's not mentioned too much in the literature

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

Ok, the arguments that we're passing and its order deserves a bit of discussion though.

I guess we're passing _this because the object can be rendered by multiple renderers...

The only argument I find redundant is object.

@pailhead
Copy link
Contributor Author

Agreed, but I think having object is useful more in lieu of a 'onwillupdate'. About the renderer, _gl is probably enough?

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

Agreed, but I think having object is useful more in lieu of a 'onwillupdate'.

I don't know what that means...

About the renderer, _gl is probably enough?

Nah, passing the renderer is fine. Maybe someone would like to do renderer.clear().

@pailhead pailhead closed this Sep 22, 2016
@pailhead pailhead reopened this Sep 22, 2016
@pailhead
Copy link
Contributor Author

Right i forgot about that ( on the phone ). Still this could be done with _gl too but it's easier with the render api.

I'd inject two more one before the matrices are computed 'onWillUpdate' and 'onHasUpdated'. That's where I'd definitely want access to the 'object'?

@pailhead
Copy link
Contributor Author

There was something else I used with the renderer, possibly clear target as well, again all this could be done with _gl but it's more complicated

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

I'd inject two more one before the matrices are computed 'onWillUpdate' and 'onHasUpdated'. That's where I'd definitely want access to the 'object'?

What would be an use case?

@mrdoob
Copy link
Owner

mrdoob commented Sep 22, 2016

There was something else I used with the renderer, possibly clear target as well, again all this could be done with _gl but it's more complicated

I wouldn't use _gl directly though. I would use renderer.state, that way WebGLRenderer can automatically restore the state.

@WestLangley
Copy link
Collaborator

How about different nomenclature?

object.onPreRender
object.onPostRender

@pailhead
Copy link
Contributor Author

My vote is 'onWillRender' 'onHasRendered' or onDidRender

Scenekit has a nice diagram, unity has a similar nomenklature.

scene kit

@pailhead
Copy link
Contributor Author

pailhead commented Sep 23, 2016

I'd inject two more one before the matrices are computed 'onWillUpdate' and 'onHasUpdated'. That's where I'd definitely want access to the 'object'?

What would be an use case?

Something along these lines:

myObject.onUpdate = (o , dt)=>{
    myObject.position.x += myObject.userData.speed * dt; 
}

could be done on prerender but would have to force the update of the matrices etc?

This would help avoid putting a bunch of logic in the update() loop as many examples have?

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2016

could be done on prerender but would have to force the update of the matrices etc?

Sorry, I don't see what this brings us that can't be done already by simply animating the objects every frame.

Lets stay on topic (onBeforeRender and onAfterRender) please.

We were discussing the arguments we should pass.

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2016

I'm thinking...

object.onBeforeRender( _this, camera, geometry, material, group );

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2016

/ping @fallenoak @arose

per previous request

added after call
added arguments to both calls
@pailhead
Copy link
Contributor Author

pailhead commented Sep 23, 2016

id still pass _gl just in case, maybe as the last argument? I might be a bit out of the loop but do we have a high level method to set the stencil op, enable the stencil buffer etc? Is it expensive to get gl out of _this every time?

On a different note, i did use this to try to mimic a multipass rendering. Lets say you want to add a coat layer to a car body material, or something like that, render two geometries "on top" of each other with different materials.

Just having one transparent and the other not would push these two different objects in different queues. I'd like to have one not write to depth, followed by an immediate render of the same geometry, writing to depth. For this i had something along the lines:

this.renderGeometryDirect = function ( camera, fog, geometry, material, object, group ) {

    objects.updateBufferGeometry( geometry ); //this could be a mask, a geometry instance that was never encountered by WebGLRenderer hence never got upoaded to the gpu

    this.renderBufferDirect( camera, fog, geometry, material, object, group ); //actual call to render, this is why i needed to pass fog as well, 
}

@mrdoob mrdoob merged commit de9ba8a into mrdoob:dev Sep 23, 2016
@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2016

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2016

Ok, this is how it's currently looking like:

object.onBeforeRender( _this, scene, camera, geometry, material, group );
object.onAfterRender( _this, scene, camera, geometry, material, group );

@fallenoak
Copy link
Contributor

@mrdoob Perfect! Thanks much, everyone!

aardgoose pushed a commit to aardgoose/three.js that referenced this pull request Oct 7, 2016
* object callbacks

per previous request

added after call
added arguments to both calls

* faster function evaluation, missing comma
aardgoose pushed a commit to aardgoose/three.js that referenced this pull request Oct 7, 2016
aardgoose pushed a commit to aardgoose/three.js that referenced this pull request Oct 7, 2016
@Benjamin-Dobell
Copy link
Contributor

Benjamin-Dobell commented Nov 10, 2016

Hmm, this onBeforeRender()/onAfterRender() is a real pain as the renderer has no idea what the side-effects are of those callbacks, which is extremely harmful to the renderer's ability to optimise rendering.

The way dynamic uniforms were implemented probably wasn't as intuitive as it could have been, but at least the renderer knew if it encountered an object with dynamic uniforms then then that object must be excluded from any internal optimisations the renderer implements.

Setting onBeforeRender and onAfterRender to have a default function rather than being null is particularly harmful. As it's now totally impossible for the renderer to know which objects are going to be messing with the renderer's state.

I'm going to submit a pull request soon that provides enormous performance gains for a lot of scenes. I was just rebasing it on dev and ran into this. At the very least my pull request will revert the default empty function assignment, I'll leave the rest open to discussion when the pull request drops.

@pailhead
Copy link
Contributor Author

How is this handled by game engines?

@Benjamin-Dobell
Copy link
Contributor

Benjamin-Dobell commented Nov 10, 2016

Game engines typically don't expose low level systems like uniforms to high level systems like the the scene graph, at least not without one or more abstraction layers. This is extremely important for game engines as they're typically cross-platform and support multiple renderers (rendering details are hidden away).

Callbacks also aren't typically called per object/node, they're called per controller (MVC), where the "view" is a "node", or in threejs' case an Object, and the controller is responsible for updating the node.

@pailhead
Copy link
Contributor Author

pailhead commented Nov 10, 2016

Hmm, not sure if i understand. In unity for example, you can write shaders, not sure if it gets any more low level than that in GL world, but you also use a gui, manage scene graphs and such. I haven't worked with it in a while but if i remember correctly you could do all kinds of stuff on all sorts of different onWillRender, onHasRenderered, onWillUpdate, onLateUpdate... etc, one of them definitely being changing a material property (thus changing a uniform)?

SceneKit, the last thing i worked with also exposes very low level systems despite being so high level and pretty much useless :)

I'm not a MVC master, but, shouldn't a node or an object actually be a Model in this case? I may be completely off here, but it feels that a shader is more like a view here. Your node contains data and state - rotation, position, color, scale, mesh, material properties etc. Shader says: "display this with lighting, use this state/data", "display the same set but flat shaded" etc.

In any case i think it is cumbersome, but a functionality like this is needed in some way.

@mrdoob
Copy link
Owner

mrdoob commented Nov 17, 2016

@pailhead you don't change GL state in a shader which is what @Benjamin-Dobell is referring to.

@DusanBosnjakKodiak
Copy link

@looeee

@looeee
Copy link
Collaborator

looeee commented Mar 25, 2020

@pailhead thanks for the suggestion - this is related to my post on discourse:

What’s the best simple way to handle the update pattern?

However, this is not what I'm looking for. The goals of the update pattern are:

  1. Separate update from render.
  2. Rather than one big and complex update method have many small methods encapsulated at their point of use.

This helps with the latter, but breaks the first.

@DusanBosnjakKodiak
Copy link

DusanBosnjakKodiak commented Mar 25, 2020

I think technically it's not too hard to implement this sort of a pattern yourself by turning scene.autoUpdate off? There shouldn't be a whole lot that this does, i think it's basically just root.updateMatrixWorld(true) (call this on root, let it traverse). If you turn it off, the renderer doesn't call this, which means you could possibly call it before your renderer.render() call.

Ie.

const myUpdate(root){
 root.traverse(o=>{
  //customize:
  if(o.update){
   o.update()
  }
  //three's default
  o.updateMatrixWorld()
 }
}

scene.autoUpdate = false
myUpdate(scene)
render(scene, camera)

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.

7 participants