Skip to content

Improve choosing render target in postprocessing EffectComposer #15671

Closed as not planned

Description

@Oletus

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:

  1. The final RGBA output render target is passed to EffectComposer's render() function. There's no need for a "renderToScreen" flag on each Pass.
  2. 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.
  3. 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.

Activity

  1. Oletus commented on Jan 30, 2019

    @Oletus
    ContributorAuthor

    @WestLangley @tentone @mrdoob any thoughts? If you think this direction is good I could work on a PR.

  2. tentone commented on Jan 30, 2019

    @tentone
    Contributor

    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.

  3. WestLangley commented on Jan 30, 2019

    @WestLangley
    Collaborator

    I agree. I would recommend that you mock up EffectComposer2. That way you have complete flexibility.

    Also, see #10144 (comment).

  4. Mugen87 commented on Jan 31, 2019

    @Mugen87
    Collaborator

    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.

  5. Oletus commented on Feb 2, 2019

    @Oletus
    ContributorAuthor

    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?

  6. donmccurdy commented on Jun 25, 2022

    @donmccurdy
    Collaborator

    As FYI — I'm interested in this, but have been mostly using pmndrs/postprocessing and so have made a proposal there:

  7. Mugen87 commented on Oct 22, 2024

    @Mugen87
    Collaborator

    Merging this into #10981.

    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.

  8. added this to the r170 milestone on Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions