You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Improve choosing render target in postprocessing EffectComposer #15671
The current mechanisms for handling render targets in EffectComposer are fairly difficult to use. A lot of the passes write to the "readBuffer" which makes the code a bit hard to follow. The "renderToScreen" flag is also a bit odd - nothing prevents setting it true on a Pass in the middle of the EffectComposer stack, but it ever really makes sense to only set that on the last pass in the stack.
The code would be simpler and more flexible if:
The final RGBA output render target is passed to EffectComposer's render() function. There's no need for a "renderToScreen" flag on each Pass.
Passes have a flag for telling if they only need one color buffer (only write or both read/write on the same buffer) or two color buffers, one being the read buffer and other one the write buffer. This could replace the needsSwap flag.
EffectComposer contains all the logic for managing the intermediate buffers and will pass the right color buffers to each Pass.
This way the EffectComposer render() call would be more similar to regular WebGLRenderer.render(), and it would also be easier to call another EffectComposer from a Pass to set up more complex effects. I've prototyped this kind of changes on a project and they made the EffectComposer much nicer to work with.
This is a breaking change, so it may be necessary to create a separate EffectComposer2 for it, though I'd be happy to change the current EffectComposer as well if others think it's OK.
This is somewhat related to Issue #12115, which outlines some good ideas for a larger overhaul of EffectComposer. These smaller changes would get us a bit closer to that while not being too much work to implement. We could design the new API so that more intermediate buffers like a normal buffer or
a depth buffer could be passed from EffectComposer to Passes in the future.
I totally agree the current post-processing pipeline needs some rethinking. Currently, it is not flexible, each pass ends up calculating everything that it needs (depth, normals etc).
I also don't like the "renderToScreen" flag I would preferer some "RenderTarget" abstraction instead.
A lot of the passes write to the "readBuffer" which makes the code a bit hard to follow.
I know it's confusing but this is done at certain places in order to avoid unnecessary swapping of buffers. A swap needs an additional render pass based on THREE.CopyShader which causes overhead.
Yep, I'm making the new implementation so that any extra copies can be avoided - they're only done when absolutely necessary. In fact I've already optimized a few of the examples to avoid redundant operations that were there before.
Here's a WIP pull request: #15694 So far I've ported over about half of the passes. Now EffectComposer2 pretty much has the functionality that the previous EffectComposer had. The most of the remaining work is just porting over the rest of the passes and examples, and possibly tidying up some of the code. I could also try to generate the depth and normal buffers needed by SAOPass2 in RenderPass2 and implement passing them through EffectComposer2. Comments?
We are at a point where we don't apply fundamental refactoring to EffectComposer and its passes anymore, see #12115 (comment). The new post processing system for WebGPURenderer manages buffers more efficiently than the previous composer. However, there is still room for improvement in context of sharing intermediate buffers.
Activity
Oletus commentedon Jan 30, 2019
@WestLangley @tentone @mrdoob any thoughts? If you think this direction is good I could work on a PR.
tentone commentedon Jan 30, 2019
I totally agree the current post-processing pipeline needs some rethinking. Currently, it is not flexible, each pass ends up calculating everything that it needs (depth, normals etc).
I also don't like the "renderToScreen" flag I would preferer some "RenderTarget" abstraction instead.
WestLangley commentedon Jan 30, 2019
I agree. I would recommend that you mock up
EffectComposer2. That way you have complete flexibility.Also, see #10144 (comment).
Mugen87 commentedon Jan 31, 2019
I know it's confusing but this is done at certain places in order to avoid unnecessary swapping of buffers. A swap needs an additional render pass based on
THREE.CopyShaderwhich causes overhead.Oletus commentedon Feb 2, 2019
Yep, I'm making the new implementation so that any extra copies can be avoided - they're only done when absolutely necessary. In fact I've already optimized a few of the examples to avoid redundant operations that were there before.
Here's a WIP pull request: #15694 So far I've ported over about half of the passes. Now EffectComposer2 pretty much has the functionality that the previous EffectComposer had. The most of the remaining work is just porting over the rest of the passes and examples, and possibly tidying up some of the code. I could also try to generate the depth and normal buffers needed by SAOPass2 in RenderPass2 and implement passing them through EffectComposer2. Comments?
donmccurdy commentedon Jun 25, 2022
As FYI — I'm interested in this, but have been mostly using pmndrs/postprocessing and so have made a proposal there:
Mugen87 commentedon Oct 22, 2024
Merging this into #10981.
We are at a point where we don't apply fundamental refactoring to
EffectComposerand its passes anymore, see #12115 (comment). The new post processing system forWebGPURenderermanages buffers more efficiently than the previous composer. However, there is still room for improvement in context of sharing intermediate buffers.