mrdoob / three.js

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

OutlinePass doesn't work with renderToScreen #10144

Closed Cobertos closed 6 years ago

Cobertos commented 7 years ago
Description of the problem

OutlinePass doesn't work when setting renderToScreen to true. Instead of rendering the outlines, it renders a blank screen.

I have found out through trial and error that OutlinePass requires that it be followed by a ShaderPass (or possibly just another pass) containing something. The postprocessing example uses a ShaderPass with an FXAAShader. Alternatively, a ShaderPass with a CopyShader works too.

I'm not sure if this is a feature or a bug but it was really annoying to try to get it to work.

Three.js version

It happens in the dev version of OutlinePass and the original revision that added it to the repository.

Browser

Tested in Chrome 54.0.2840.99 m (64-bit) and Firefox 50.0

OS

I'm on Windows 7 x64

Hardware Requirements (graphics card, VR Device, ...)

I'm using an Intel integrated graphics chipset

Cobertos commented 7 years ago

I was planning on fixing this as it seems fairly easy. After some digging, it seems OutlinePass just doesn't check renderToScreen like the other passes do and only modifies the readBuffer.

However, I'm not 100% sure how the post processing framework is structured. Is it meant so that multiple passes have renderToScreen (writing over/adding imagery to old passes) or should effects like this combine the readBuffer with their own data into the writeBuffer and then render to the desired position if renderToScreen is set?

Mugen87 commented 7 years ago

As far as i know, renderToScreen should always be set to true for the final pass in your chain. If this pass does not evaluate the property, use an additional CopyShader pass for the final render step.

I'm not sure, why some passes don't check renderToScreen. Besides, some passes (like OutlinePass) are rendering to the readBuffer. Not sure if this approach is correct/consistent. I've always thought a pass should basically read its input data from the readBuffer and write its result to the writeBuffer (MaskPass might be an exception). In these cases needsSwap should be set to true so the composer swaps the buffers after the render process.

Cobertos commented 7 years ago

Hmm, okay. needsSwap sounds like the right tool to handle this. I was also getting some other problems with OutlinePass like not hiding instances of THREE.Line in my scene when rendering the outlines. I eventually have to include this functionality into the project I'm currently working on so I'll probably go ahead and commit some changes when that time comes.

On that note though, why do we have renderToScreen then? Why doesn't EffectComposer just render out the last pass to wherever it needs to (WebGLRenderer or WebGLRenderTarget). It seems to me the API might look better as new EffectComposer(target, camera) wheretargetis aWebGLRendererorWebGLRenderTarget,camera` is the camera, and it implicitly renders out the last pass to the target.

Mugen87 commented 7 years ago

I don't think that the EffectComposer should have a reference to the camera object. The actual rendering of the scene is currently done by an instance of RenderPass. This approach has the advantage of encapsulation. EffectComposer should not be involved in how the passes create their result. It just controls/manages the composition of the final image.

I don't know why the last pass in a chain is not automatically rendered to screen. Maybe @mrdoob or @WestLangley can help here.

WestLangley commented 7 years ago

EffectComposer has not had significant development in 4 years or so. If you are interested in considering alternate implementations, you might want to have a look at @spite's Wagner API. I expect there would be a lot of interest in revisiting this topic.

spite commented 7 years ago

Wagner needs a big refactor, mostly on alpha and stencil handling, and shader loading. I plan to address that at some point, but I'd love to hear what people think about it. Also, I know there's a fork out there that supports modules and bundling.