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

Use Scene.onBeforeRender() and onAfterRender() in WebGLRenderer for tiled forward lighting and VR post proc #11582

Merged
merged 1 commit into from
Jan 6, 2018

Conversation

wizgrav
Copy link
Contributor

@wizgrav wizgrav commented Jun 22, 2017

Use Scene.onBeforeRender on WebGLRenderer to provide a way to get the vr cameras after they are updated for the current frame toimplement forward+ shading and expose the vr cameras for post processing.

@wizgrav wizgrav changed the title WebGLRenderer.onBeforeRender and autoSubmitFrame WebGLRenderer.onBeforeRender() and autoSubmitFrame flag Jun 22, 2017
@edankwan
Copy link

@wizgrav can you update and fix the conflict? @mrdoob it should be a simple update. Please offer us some love for VR post effect 🤗

@wizgrav
Copy link
Contributor Author

wizgrav commented Nov 19, 2017

@edankwan done, it was a silly conflict with a comment(?) @mrdoob the changes are really minimal and still relevant after all this time. It won't just help with post proc. The updated cameras are also needed to construct the textures needed for forward+ shading, after the cameras update and just before the rendering of objects is performed so onBeforeRender serves two purposes here

@takahirox
Copy link
Collaborator

I don't have strong preference but renderer.vr.autoSubmitFrame sounds a bit better than renderer.autoSubmitFrame because users can easily speculate .autoSubmitFrame is for VR?

@mrdoob

If you have some concerns for renderer.onBeforeRender() while .autoSubmitFrame seems ok to you, can we get .autoSubmitFrame in first because we need it for PostProcessing on VR?

@mrdoob
Copy link
Owner

mrdoob commented Nov 22, 2017

Hmm... This feels pretty hacky. Can anyone share a snippet of how the code would be used?

@wizgrav
Copy link
Contributor Author

wizgrav commented Nov 22, 2017

@mrdoob

renderer.onBeforeRender = function (scene, camera, renderTarget) {
  // Use camera to manually cull lights, generate the per screen tile light lists and store them in texture(s)
};

These textures will then be used by custom materials to implement Forward+ shading as described here Forward-Plus-Renderer. The link mentions depth prepass and compute shaders, but they are not needed. In fact the latest iteration of the technique is called clustered lighting and is even more performant without depth prepass ClusteredShading.pdf

Its an alternate implementation of many lights that doesn't need deffered, and is generally faster since we don't always have mrt support. It has many other advantages as well like transparency support.

The reason onBeforeRender is needed there is because we need to tap on the camera after it updates. In WebGLRenderer.render the camera can also be swapped with the vr ArrayCamera, so by tapping where it does, onBeforeRender will be called with the correct camera that will be used for the scene rendering

@takahirox
Copy link
Collaborator

takahirox commented Nov 22, 2017

Here is the very basic PostProcessing + VR example code.

renderer.vr.enabled = true;
renderer.vr.autoSubmitFrame = false;

scene.add( mesh );

mesh2.material.uniforms.tDiffuse.value = renderTarget.texture;
scene2.add( mesh2 );

function animate() {

    requestAnimationFrame( animate );

    // main rendering to renderTarget
    renderer.render( scene, camera, renderTarget );

    // post-processing
    // renderer.vr.enabled = false;  // some post-processing might require non-VR mode
    renderer.render( scene2, camera2 );
    // renderer.vr.enabled = true;

    renderer.submitFrame();

}

animate();

@wizgrav
Copy link
Contributor Author

wizgrav commented Nov 22, 2017

@takahirox @mrdoob Also some post proc effects need the camera(s) used to render the original scene. Godrays would be an example of that as they need to calculate the center(s) for the radial blur. onBeforeRender would come in handy here as well

@mrdoob
Copy link
Owner

mrdoob commented Nov 23, 2017

Hmm... How about something like renderer.vr.onBeforeSubmitFrame = function() {}?

@wizgrav
Copy link
Contributor Author

wizgrav commented Nov 23, 2017

@mrdoob .onBeforeSubmitFrame() would be right before the submitFrame() call right? In this case the scene will be already rendered by the time it is called so the tiled forward technique cannot be implemented. The tiled light textures need to be created just after the cameras have updated but before the scene objects are rendered because their shaders need the up to date textures to perform lighting.

Another proposal would be onBeforeRender() where it is and an onAfterRender() just before the frame is submitted. This way post proc can be performed in the latter and it will work for both normal or vr mode. This adds another method but takes out the autoSubmitFrame flag and is symmetric.

@wizgrav wizgrav changed the title WebGLRenderer.onBeforeRender() and autoSubmitFrame flag WebGLRenderer.onBeforeRender() and onAfterRender() for forward+ and vr post proc Nov 23, 2017
@wizgrav wizgrav changed the title WebGLRenderer.onBeforeRender() and onAfterRender() for forward+ and vr post proc WebGLRenderer.onBeforeRender() and onAfterRender() for forward+ and VR post proc Nov 23, 2017
@mrdoob
Copy link
Owner

mrdoob commented Nov 24, 2017

Another proposal would be onBeforeRender() where it is and an onAfterRender() just before the frame is submitted. This way post proc can be performed in the latter and it will work for both normal or vr mode.

Hmm, do you mind pointing me to the exact lines in the code where we would be calling these functions?

@mrdoob
Copy link
Owner

mrdoob commented Nov 24, 2017

Oh wait, you already modified the PR.


if ( tempHandler ) {

_this._onBeforeRender = null;
Copy link
Owner

@mrdoob mrdoob Nov 24, 2017

Choose a reason for hiding this comment

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

I would put this on your app code instead.
We do similar things when using object.onBeforeRender already:

https://github.com/mrdoob/three.js/blob/dev/examples/js/objects/Reflector.js#L158-L162

@wizgrav
Copy link
Contributor Author

wizgrav commented Nov 24, 2017

@mrdoob I reverted the code, I guess a warning in the docs should suffice for avoiding the infinite recursion and I also didn't like the tempHandler var in the renderer. In the weekend, I'll get on to making an example for tiled forward lighting and post proc

@edankwan
Copy link

I personally prefer .autoSubmitFrame approach because of the infinite recursion issue @wizgrav mentioned. In order to bypass this issue, we will need to do some hacky stuff which I don't think it is a good solution.

@wizgrav
Copy link
Contributor Author

wizgrav commented Jan 1, 2018

Happy new year every one, wishes for a prosperous and creative 2018 :)

I finally found some time to write an example for tiled forward lighting. It's probably the most basic implementation, using tile texels as bitmasks to accomodate 32 lights on screen. It can be easily extended to more, though it would not be best approach for more than a couple hundred. There are other approaches using one more intermediate texture that can accommodate thousands of lights.

I added back the autoSubmitFrame flag as @edankwan suggested. I used the onAfterRender() approach for the demo and the mentioned hack for the infinite recursion is visible and not very pleasing imho. Maybe we could keep both approaches as the code footprint is really tiny. After considering @takahirox suggestion, I decided to leave the flag on the renderer and not the VRManager as the renderer is the one actually submitting the frames so it makes more sense to me that he should carry the toggle.

@wizgrav
Copy link
Contributor Author

wizgrav commented Jan 3, 2018

And here's the example live https://wizgrav.github.io/three.js/examples/webgl_tiled_forward.html I need to cleanup the build files for this PR. Your thoughts @mrdoob ?

@wizgrav wizgrav changed the title WebGLRenderer.onBeforeRender() and onAfterRender() for forward+ and VR post proc WebGLRenderer.onBeforeRender() and onAfterRender() for tiled forward lighting and VR post proc Jan 4, 2018
@@ -1099,6 +1105,8 @@ function WebGLRenderer( parameters ) {

}

_this.onBeforeRender( _this, scene, camera, renderTarget );
Copy link
Contributor

@pailhead pailhead Jan 5, 2018

Choose a reason for hiding this comment

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

could this possibly be onAfterAutoUpdate ?

We've already started to "render" so it's more like onBeginRender.

@pailhead
Copy link
Contributor

pailhead commented Jan 5, 2018

It seems to me that the only thing special about the onBeforeRender call is that it may happen after the scene autoUpdate.

Making this call before you actually call myRenderer.render() seems to guarantee only these two things:

  1. the scenegraph objects will have their matrices up to date, if they were set to auto update
  2. the render camera will have a correct matrix if it does not belong to the scenegraph (no parent)

What I was suggesting before was to have an object based onBeforeUpdate and onAfterUpdate that would be called before and after the matrices are computed. It feels like it would give more meaning to:

if ( scene.autoUpdate === true ) scene.updateMatrixWorld();

I'm not sure if i explained it well, since i didn't explain it well the first time i was suggesting this.

With how it's currently written/named it seems that this would make more sense, while doing the exact same thing?


function actualBeforeRenderCall(){...}

myScene.autoUpdate = false

function myRender(){
   myScene.updateMatrixWorld(true)
   actualBeforeRenderCall()
   myRenderer.render(myScene, myCamera)
}

@wizgrav
Copy link
Contributor Author

wizgrav commented Jan 5, 2018

@pailhead in the case of vr, the provided camera is replaced by an, internally updated, ArrayCamera, This is the only reason onBeforeRender is needed, It provides a convenient structure for both cases, vr or non. Same goes for onAfterRender. Some post proc effects also need the camera(s)

@wizgrav wizgrav changed the title WebGLRenderer.onBeforeRender() and onAfterRender() for tiled forward lighting and VR post proc Use Scene.onBeforeRender() and onAfterRender() in WebGLRenderer for tiled forward lighting and VR post proc Jan 5, 2018
@wizgrav
Copy link
Contributor Author

wizgrav commented Jan 5, 2018

@mrdoob changed the location of the hooks to be on the Scene object which supported them already from Object3D. I decided to leave .autoSubmitFrame out as, with this arrangement, the renderer can be reused safely inside onAfterRender @edankwan.

tileData : { type: "v4", value: null },
tileTexture: { type: "t", value: null },
lightTexture: {
type: "t",
Copy link
Collaborator

Choose a reason for hiding this comment

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

type specification should not be needed.

mtl.uniforms.lightTexture = State.lightTexture;
for( var u in conf.uniforms ) {
var vu = conf.uniforms[u];
if(mtl.uniforms[u].value.set) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe

if ( mtl.uniforms[ u ].value && mtl.uniforms[ u ].value.set ) {

}

// Generate the light bitmasks and store them in the tile texture
function tileLights(renderer, scene, camera) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you clean up your whitespace formatting a bit? https://zz85.github.io/mrdoobapproves/

new THREE.MeshBasicMaterial( {
color: new THREE.Color("hsl(" + chroma + ", " + sat + "%, 50%)"),
transparent: true,
opacity: 0.033
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that even visible?

Copy link
Contributor Author

@wizgrav wizgrav Jan 6, 2018

Choose a reason for hiding this comment

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

Barely, it makes it look like a cheap glow from the light spheres

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's nice. On my monitor I had to increase the opacity, though.


var lights = [], objects = [];

var State = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

lower case

@WestLangley
Copy link
Collaborator

changed the location of the hooks to be on the Scene object which supported them already

Much better. :-)

I have not been able to study the tiled-forward code yet, but this is an interesting demo of a shader hack.

It is also an interesting implementation of post processing in lieu of EffectComposer.

@mrdoob
Copy link
Owner

mrdoob commented Jan 6, 2018

Love how simple the PR turned out to be! 😍

@mrdoob
Copy link
Owner

mrdoob commented Jan 6, 2018

The example definitely needs some clean up, but I'll merge this for now.

@mrdoob mrdoob merged commit 325d819 into mrdoob:dev Jan 6, 2018
@mrdoob
Copy link
Owner

mrdoob commented Jan 6, 2018

Thanks!

@takahirox
Copy link
Collaborator

Finally we can start to work on VR + Post-Proc? 😄

@WestLangley
Copy link
Collaborator

The example definitely needs some clean up,

@wizgrav Would you be willing to continue with the suggested changes?

@wizgrav
Copy link
Contributor Author

wizgrav commented Jan 7, 2018

@WestLangley yeah, I basically did them but mrdoob merged first. I was also thinking to vr enable the example for completeness

@mrdoob
Copy link
Owner

mrdoob commented Jan 8, 2018

@wizgrav feel free create another PR with these changes 😊

@mrdoob mrdoob added this to the r90 milestone Jan 17, 2018
@DusanBosnjakKodiak
Copy link

@looeee

I believe this stuff is related to your post on the forum.

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