I noticed that Scene._renderForCamera does not unbind the framebuffer bound in this line after rendering. In a typical scenario, as the comment in the code suggests, this call to this._bindFrameBuffer is just meant to restore the currently bound framebuffer to the default back buffer after rendering to targets; but when we are in a VR scenario, we could have two cameras in the rig, each holding render targets which do not correspond to the default back buffer. This seems to break the possible assumption that Scene._renderForCamera is begun and ended with the default back buffer bound-- because in this situation when the framebuffer is “restored,” it’s really just being set to another framebuffer that isn’t necessarily the default.
Is it for performance reasons that the frame buffer bound with this._bindFrameBuffer isn’t unbound at the end of Scene._renderForCamera?
The reason I discovered this is that the BabylonNative engine currently has a bug where the viewport is cleared when engine.setViewport clears the currently bound buffer, and so we’re overwriting the left eye framebuffer when the right eye framebuffer goes to get rendered, since the left eye is still bound at the beginning of the second call to Scene._renderForCamera.
Yes, that part totally works. I’m just pointing out that when you call _renderForCamera in a tight loop like for rig cameras, the next call will have the previous rig camera’s outputRenderTarget still bound instead of the default back buffer.
It seems to be the current assumption that _renderForCamera is called with the default back buffer bound, but I’m just basing that understanding off of the call to engine.setViewport seen here. The resulting view rect will be based off of the previously bound render target, which in a non-vr rendering scenario is not an issue because that’s typically the canvas, but in our scenario when this code immediately re-runs for the right eye, the view rect will correspond to the left eye, which already rendered.
That all being said, I’m realizing this is more of a Babylon Native implementation issue than a Babylon.js issue, since Babylon Native shouldn’t implement engine.setViewport with any side-effects anyway, as it’s currently clearing the view rect.
I just thought it might be worth pointing out in case people try to do anything at the beginning of the function that is less benign than setting the viewport before binding the next render target.