mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.57k stars 35.37k forks source link

WebGLRenderer: animate() → setRenderLoop()? #13594

Closed mrdoob closed 6 years ago

mrdoob commented 6 years ago

While working on a new WebVR project, it somehow felt that setRenderLoop() would be a better name than animate() but I'm unsure.

var renderer = new THREE.WebGLRenderer();
renderer.setPixelRatio( window.devicePixelRatio );
renderer.setSize( window.innerWidth, window.innerHeight );
renderer.setRenderLoop( render );
document.body.appendChild( renderer.domElement );

function render( timestamp ) {

    renderer.render( scene, camera );

}

Thoughts?

mkeblx commented 6 years ago

Makes sense especially since other methods that 'set' something are set{X} form.

Usnul commented 6 years ago

I think it's not super descriptive. I would call it "simulate" or something along the lines of "addFrameEventHandler"

looeee commented 6 years ago

What about startRenderLoop ? Since we are starting to call render in a loop here I think that's pretty descriptive.

Usnul commented 6 years ago

@looeee can i do this?

renderer.startRenderLoop( makePancakes );

function makePancakes ( timestamp ) {

    console.log("In a large bowl, sift together the flour, baking powder, salt and sugar. Make a well in the center and pour in the milk, egg and melted butter; mix until smooth.");

    console.log("Heat a lightly oiled griddle or frying pan over medium high heat. Pour or scoop the batter onto the griddle, using approximately 1/4 cup for each pancake. Brown on both sides and serve hot.");

}

is this a good name then? :)

looeee commented 6 years ago

No, but I challenge you to think of a non-stupid, useful example where the callback function being passed WebGLRenderer.animate should be called something other than render. 😛 If you can then I'll change my mind.

Usnul commented 6 years ago

Challenge accepted!

There are typically multiple simulation systems involved in making an interactive application such as a game. One obvious example is physics simulation. If you go by a ECS paradigm (entity component system) - every system can benefit from an update step. Most of these systems are typically non-visual.

looeee commented 6 years ago

OK, that's a good point. Consider me convinced 😄

A typical (simplified) Game loop at looks like:

while( true ) {
    getUserInput()
    update()   // update physics, animation and anything else
    render()  // draw 
}

Obviously the getUserInput is handled differently in JS due to eventListeners, but otherwise it's similar to a typical three.js loop with the while replaced by RAF. So I don't think that the animate function should have Render in the name.

How about startRAFLoop ?

mrdoob commented 6 years ago

What about startRenderLoop ? Since we are starting to call render in a loop here I think that's pretty descriptive.

Hmm, I like that one too...

mrdoob commented 6 years ago

How about startRAFLoop ?

startAnimationLoop may sound less alien.

fernandojsg commented 6 years ago

+1 for the startAnimationLoop I agree with @Usnul that renderloop is misleading. (Although I'm not a big fan of animationloop either, but well it's still better than the other alternatives :D)

Makio64 commented 6 years ago

If its a RAF call, then I think it should be a clear in the name : .onEveryFrame( callback ) .onRAF( callback )

But I don't understand why there is this function on a WebglRenderer ?

mrdoob commented 6 years ago

But I don't understand why there is this function on a WebglRenderer ?

Because the WebVR API has its own requestAnimationFrame and renders black if you use window.requestAnimationFrame.

Makio64 commented 6 years ago

I see, then for peoples who doesnt know it like me, maybe it'll be worth it to made it more obvious : .onWebVRAnimationFrame( callback ) .onWebVRFrame( callback )

looeee commented 6 years ago

@Makio64 it works for anybody though, not just people using WebVR . Previously you would do:

function animate() {

  requestAnimationFrame( animate );

  renderer.render( scene, camera );

}
animate();

Which would not work for VR. Now you can do:

renderer.animate( () => {

  renderer.render( scene, camera );

} );

which will work everywhere, so we should start to recommend this pattern. At least that's my understanding.

fernandojsg commented 6 years ago

We should consider also the asymmetrical vr apps we you will render something on the headset different from what you are sending to the screen. In that case you would like to use window.RAF for the screen and vrdisplay.RAF for the headset. Maybe onAnimate by default will be used as proposed, but you could pass a second parameter to specify the vr mode for example?

renderer.animate( () => {
    // window.raf or vrdisplay.raf if presenting in VR
});
renderer.animate( () => {
    // window.raf
},
() => {
    // vrdisplay.raf
});
Usnul commented 6 years ago

@fernandojsg I think dual call-back is not a good choice, in async programming dual callback is pretty much a convention for success, failure .

@mrdoob A more general concern. How do we stop? :)

fernandojsg commented 6 years ago

@Usnul yep, it's not really semantic doing that way, I just wanted to reuse the current proposal. Otherwise we should create two setters for each kind of RAF, similar to what @Makio64 proposed:

.onWebVRAnimation( callback )
.onWindowAnimation( callback )

or

onAnimation( callback ) will replace both

Ops I hit wrong button :_D

mrdoob commented 6 years ago

Ended up going with setAnimationLoop().

With the current implementation you can do this:

renderer.setAnimationLoop( renderLoop );
renderer.vr.setAnimationLoop( vrRenderLoop ); // optional, it'll use renderLoop otherwise