Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render DAG and rendering modules architecture #3732

Open
wants to merge 196 commits into
base: develop
from

Conversation

@dave2s
Copy link
Contributor

commented Aug 25, 2019

Rebased onto Develop from PR 3680
Contains whole

GSOC 19 Render DAG

engine architectural changes for rendering: foundation for rendering modules, their ingame configuration, dependency connections foundation for expressing semantics of the dag edges and allow dynamic reconnecting of rendering pipeline.
This PR is accompanied by new module repo for AdvancedRendering module, BasicRendering module and Peronal repo dag testing module

The progress of these changes as well as final summary (will be) is contained within this personal blog AND within this game forum thread.

One can also try to take a look and compare sketchboard graphs, but the new one ended up more as a helper thing and is not entirely accurate:
Sketchboard I used
Original state of the sketch board for dag.

Main areas of focus:

Architecture for DAG:

  • package org.terasology.rendering.dag

Rendering modules subsystem/manager/registry:

  • package org.terasology.engine.module.rendering;
  • package org.terasology.engine.subsystem.rendering

Rendering modules UI:

  • package org.terasology.rendering.nui.layers.mainMenu.videoSettings;
  • ...assets\ui\menu\renderingModuleSettingScreen.ui

and updates in:

  • package org.terasology.rendering.nui.layers.*
  • assets: menu_cs.lang, menu_en.lang

Important: Rendering nodes together with some logic have been taken out to BasicRendering and AdvancedRendering modules. These are mandatory for the game as of this moment. AdvancedRendering module is optional but so far there are no guards that will prevent you from trying to turn these effects on, resulting in a crash.
https://github.com/Terasology/AdvancedRendering and
https://github.com/Terasology/BasicRendering to run

Some examples might be found here https://github.com/dave2s/dagTestingModule

How to add a rendering module

  1. Create a new module
  2. Any new rendering class in this module must now extend ModuleRendering and call super(context); in its constructor.
  3. One can optionally call setInitializationPriority(init order to override default of super class -positive integer ); after the super() call.
  4. To connecta new node, a node has to be created first, you can take a look at existing modules to see how it's done. For example Node ambientOcclusionNode = new AmbientOcclusionNode("ambientOcclusionNode", providingModule, context); . After the node is created by calling its constructor, it has to be added to the rendergraph by calling renderGraph.addNode() and for it to do something its output has to be connected and read somewhere - !! If a node is added to the renderGraph but not connected, the graph probably won't build !! (graph condition - amount of nodes visited == all added nodes' size).
    ad 4 - new node: To see a minimal implementation of a node, take a look at some existing code - for example any Node in BasicRendering module and mainly their parent - AbstractNode.
    ad 4 - connecting: To see an example of connecting the nodes, take a look at either BasicRendering or AdvancedRendering modules' main classes extending ModuleRendering. Interconnecting nodes trough rendergraph consists of calls such as renderGraph.connectFbo() or renderGraph.connectBufferPair(). To see how reconnecting api calls can be used, take a look at AdvancedRendering's addHaze() for example:
renderGraph.reconnectInputBufferPairToOutput(finalHazeNode, 1, opaqueObjectsNode, 1);
renderGraph.reconnectInputBufferPairToOutput(finalHazeNode, 1, opaqueBlocksNode, 1);
renderGraph.reconnectInputBufferPairToOutput(finalHazeNode, 1, alphaRejectBlocksNode, 1);
renderGraph.reconnectInputBufferPairToOutput(finalHazeNode, 1, overlaysNode, 1);

Here you can see an renderGraph api call being used to reconnect finalHazeNode 's output bufferPairConnection to essential block rendering nodes which have been previously connected to a backdropNode as shown here: Use this sketchboard!

If you wanted to do your own rendering modules, you could start by copying starting and OutputToScreenNode nodes from BasicRendering module and add some version of blocks rendering nodes otherwise the game crashes (i'll fill why later)

How renderGraph connects nodes via dependencies

  1. Nodes are connected newly via their input/output DependencyConnection s. After all modules have been accounted for by the WorldRendererImpl.java, their postInit() and then setDependencies() methods are called. setDependencies being a crucial late initialization method which reads and sets data from/into connections previously connected in the stage of connecting the nodes in ModuleRendering extending class.
  2. After this, based on the dependency connections of all nodes these are connected via legacy connect() of the renderGraph, which uses a private MutableGraph<Node> graph; the same was it used to. This calculation of connections is redone every graph rebuild. It's a naive implementation which might be subject to improvement in the future.

Rebased onto Develop from #3680
Last commit pre GSOC deadline: commit 7aa3445

  • I'm considering adding Shader dependency type so one can swap shaders for existing nodes from another module without the need to implement any nodes. I recently found this to be more important than we had initialy thought simply because copying whole nodes creates problems sometimes. This is because there are occasionally some code dependencies for said nodes and they cannot be easily reimplemented in another module. In such case a shader swap could be one of the ways to edit these nodes. Another being reconnecting trough some new nodes. Maybe a shader override could be supplied?
  • Fog (regular haze) can't be turned off without breaking visual output atm, though without having advanced rendering on in the first place it should work, but one has to disable all advanced effects first (ambient occlusion, shadows, fog/haze, light shafts, bloom)
  • To merge or not to merge?
  • Rendering module configuration should get reloaded when loading another game - causes errors if you start a game with some extra module and then try to load a game without it. It's still being initialized? investigate
dave2s added 30 commits Apr 29, 2019
remove connections, get number of connections, get list of input/outp…
…ut connections, get input/output connection by name
Faulty design atm, contains the usage idea behind newabstractnode cha…
…nges. Todo: adding logic must be moved somewhere else. In other words the removed change by this commit was more functional...so something along those lines
connection names from Name to String. Might construct some Uri class …
…based on names later. ++Setters/Getters for Uri/Urn acting as connection data but so far only point there
moved getFboName(int) naming from super class to FboConnection.java,
removed createFBOConnection methods from super class
rename FBOConnection.java to FboConnection.java
rename EdgeConnection to DependencyConnection
updated FboConnection to store FBO fboData and updated get/set methods
FinalPostProcessingNode - read input fbo from added FboConnection
restored fbousages for new abstract nodes for now
update passtroughNode with mandatory setDependencies() method to compile without errors
Connect toneMappingNode out FBO1 to finalPostProcessingNode input FBO1
notice we don't add outputs to nodes in this scope, this happens inside them
Update nodes to extend NewAbstractNode implement NewNode
 and make minimal changes to compile.
Nodes with more changes will be commited separately.
List of nodes in this commit:
- WorldReflectionNode, UpdateExposureNode, SimpleBlendMaterialsNode
- RefractiveReflectiveBlocksNode, ShadowMapNode, OverlaysNode
- OutputToScreenNode, OutputToHMDNode, OutlineNode, OpaqueObjectsNode
- OpaqueBlocksNode, InitialPostProcessing, HighPassNode
- DeferredPointLightsNode, DeferredMainLightNode, BufferClearingNode
- BackdropReflectionNode, BackdropNode, ApplyDeferredLightingNode
- AlphaRejectBlocksNode, ConditionDependentNode
-
Connected input dependencies(some gbuffers are still passed the old w…
…ay):

BlurredAmbientOcclusionNode, prePostCompositeNode,
Whole exposureDownSampling chain - (even gbuffer) passing input connections in a constructor, more changes internally separate commit
finalPostProcessingNode - partially, (gbuffer old way)
Rename: Node - > NewNode - whole file

IMPORTANT NOTE: OUTPUT DEPENDENCIES ARE SET WITHIN NODES THEMSELVES
Call NewNode.setDependencies() when adding new NewAbstractNode to the…
… graph.

Pass context to the RenderGraph from WorldRendererImpl
Updated NewAbstractNode:
protected- void addInputFboConnection(id, DATA(new, besides FboConnection version)),
protected- FBO getOutputFboData(#);
public- FboConnection getInputFboConnection(#);
Essential capability added before: connectFbo() to connect nodes

@dave2s dave2s requested a review from vampcat Aug 25, 2019

@dave2s dave2s self-assigned this Aug 25, 2019

@dave2s dave2s added this to In progress in GSOC 2019: DAG via automation Aug 25, 2019

@dave2s dave2s referenced this pull request Aug 25, 2019
4 of 4 tasks complete
RenderGraph.java overload reconnectInputFboToOutput() to enable Nodes…
… as both arguments-this is required for module to find nodes by Aka

@dave2s dave2s moved this from In progress to Done in GSOC 2019: DAG Aug 25, 2019

@Cervator

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

This pull request introduces 4 alerts when merging f87eec4 into 6ce026d - view on LGTM.com

new alerts:

  • 3 for Dereferenced variable may be null
  • 1 for Unused format argument

Warning - Automated code review for MovingBlocks/Terasology will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@Cervator

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

This pull request introduces 4 alerts when merging 4af9edc into 6ce026d - view on LGTM.com

new alerts:

  • 3 for Dereferenced variable may be null
  • 1 for Unused format argument

Warning - Automated code review for MovingBlocks/Terasology will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

RenderingModuleSettingScreen.java checkstyle, ++ initSlider null chec…
…k, --moduleList dropdown getText unused argument to String.format()
@Cervator

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

This pull request introduces 2 alerts when merging 5e22eb5 into 6ce026d - view on LGTM.com

new alerts:

  • 2 for Dereferenced variable may be null

Warning - Automated code review for MovingBlocks/Terasology will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@Cervator

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

This pull request introduces 1 alert when merging 5cd4524 into 6ce026d - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

Warning - Automated code review for MovingBlocks/Terasology will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@Cervator

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

This pull request introduces 1 alert when merging 7aa3445 into 6ce026d - view on LGTM.com

new alerts:

  • 1 for Dereferenced variable may be null

Warning - Automated code review for MovingBlocks/Terasology will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@dave2s

This comment has been minimized.

Copy link
Contributor Author

commented Aug 25, 2019

Not sure if we are merging this now, but I'm commenting here to mark a gsoc end.
commit 7aa3445

@GooeyHub

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

Hooray Jenkins reported success with all tests good!

3 similar comments
@GooeyHub

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

Hooray Jenkins reported success with all tests good!

@GooeyHub

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

Hooray Jenkins reported success with all tests good!

@GooeyHub

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

Hooray Jenkins reported success with all tests good!

@Cervator Cervator requested a review from eviltak Sep 1, 2019

@dave2s

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2019

updating description continually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.