Vignette node draft

[dave2s]

Contains

This PR proposes a solution for issue #3578 by placing vignette at the end of the shader pipeline, as opposed to the present state where the vignette is a part of initial post-processing node/shader. This solution comes to mind as the blur effects responsible for this issue are currently the very last.
Whether this approach is correct is a subject for further discussion. It is debatable, whether such change is for the better.

This PR also attacks Stage 1 of issue #3040 by creating a separate node for Vignette. I would like to ask @vampcat to check if my approach is headed the right way and, if need be, to tag other contributors responsible for such changes to review.

The changes should be rather light-weight, but this is the first time I played with the engine's dag, so keep that in mind :).

How to test

Enable Vignette, Blur,...various graphical effects combinations to check if the graphics behave correctly.

Notes

Suggestion of directly tinting vignette by general composition and tint nodes (currently non-existent) is irrelevant at this stage as vignette node creation is a common step.

Outstanding before merging

• Needs to be updated to work with Render DAG and rendering modules architecture #3732
• Need to discuss whether the solution to Vignette effect moves along with the motion blur effect #3578 is wanted. It is debatable whether such change is for the better. @Cervator said whichever looked better to us
• finish at least stage 1 of Vignette Node #3040 - enable setting tint from outside somehow
• verify graphical correctness (correct buffers are passed - hdr, default, no effects are left behind)
  If fix for Vignette effect moves along with the motion blur effect #3578 is accepted like this:
• Move debugging into vignette node? - just to not forget about it until the overhaul is ready
• Move screenshotting from finalPostProcessNode/vignetteNode to OutputNodes

suggested DAG here

VR output node is named OutputToHMDNode, not outputToVRFrameBufferNode
DAG before:

DAG after:

please suggest further needs or dismiss this PR as not-important atm.

Blurred vignette

Not blurred vignette

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[dave2s]

Forgot about Screenshot capability - would have to be moved to vignetteNode done
Edit: VR output would have to read from vignette as well. It reads from finalBuffer
Edit2: not sure about debugging, need to check because finalpostprocessingnode is currently responsible for visual debugging.

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[vampcat]

Heyo :D

I just have one concern - why are we doing the screenshot-ing in vignette node? We should be doing that in the final node, which in this happens to be OutputToScreenNode (or something very similar).

[dave2s]

Heyo :D

I just have one concern - why are we doing the screenshot-ing in vignette? We should be doing that in the final node, which in this happens to be CopyToScreen (or something very similar).

I'm not really sure if there was a special reason for it, I'll look in the history if I can see something.I think there should be no problem with moving it to the output nodes themselves (Screen or VR). It's not working if you just move it there. I'm sure there's a straightforward explanation, I'll look into it later.
My mistake, wrong branch. It works.
Update: Screenshots should be now doable trough Output nodes.

[GooeyHub]

Hooray Jenkins reported success with all tests good!

[eviltak]

@dave2s does this PR still have any pending items, or can we merge it?

[dave2s]

@eviltak I would wait with merging this after the big rendering PR is merged and simultaneously I would update this so it works with it. This PR predates the latest rendering changes.
Edit: Alternatively this is ready to go I think and we can update it later. If you think this is a better process, I'll check it out once more today.

[skaldarnar]

@dave2s am I right in the assumption that this PR would need to be redirected against https://github.com/Terasology/CoreRendering instead? Is there any chance we can easily revive the content of this PR?

[dave2s]

@dave2s am I right in the assumption that this PR would need to be redirected against https://github.com/Terasology/CoreRendering instead? Is there any chance we can easily revive the content of this PR?

@skaldarnar Hello. From the first glance at the current state of things I reckon it should be possible to transfer this PR's content to the core rendering module. I don't have the project with me so I can't try it out right ahead, but if the CoreRendering at develop is what's at use currently, I would still try to update and merge it.
edit: If the content is still relevant that is.

[pollend]

looks like @skaldarnar merged the other PR. I think we can close this PR now.