Debug Node (New) #3058

vampcat · 2017-08-12T01:45:04Z

Follow up to #3051

Extends the OutputToScreenNode (earlier CopyImageToScreenNode) to display arbitrary FBOs. Also adds the associated console command that can be used to control the fbo (dagSetOutputFbo engine:fbo.ssao).

Side Note: The current method to handle the commands is pretty crude, but achieves the basic purpose - delegates the job of handling the command to the Node, and does not expose the internals of the rendering engine to unrelated classes.

GooeyHub · 2017-08-12T01:52:42Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-08-12T22:17:35Z

@vampcat: can you please explain the reasoning behind the various features of this PR versus the features I suggested in the last comment I made in #3051?

In this context, you might want to use github comments to add notes to specific portions of the diff. Which is also an alternative to writing longer descriptions.

vampcat · 2017-08-13T01:27:03Z

"In this context I'm thinking that the WorldRendererImpl class could be a good candidate to become a system, perhaps doing initialization work in postBegin() and disposal in shutdown(). From that scenario we'd be able to have commands in the WorldRendererImpl class."
I don't mind making WorldRendererImpl class (or more specifically, the WorldRenderer class) be a system.. I just don't see any huge advantage/disadvantage in that though. We probbaly can't use the postBegin() for initialization (specifically, the obtaining of lastUpdatedGBuffer and staleGBuffer since they have to be obtained at the very last stage of the renderer so that get*Buffer() returns the correct buffer or something similar), and we don't really have anything to dispose right now (and quite possibly in the future too). That being said, is there any specific reason you'd like me to shift the commands over to WorldRenderer(Impl) ?
Note that I'm not opposed to the idea, I'm just not sure what's the best place for this.

"That been said, it would still be nice if somehow nodes can register with the renderer the commands they can accept. I.e. we could have a package-private WorldRenderer.registerCommand(uri nodeId, string commandName). Then, in the node, we'd have a handleCommand(string commandName, string arguments)."
That's certainly true, it would be amazing to have. Based on this, in the current design, I have delegated the handling of commands to the Node : However, I just haven't added the ability for Nodes to register custom commands, because we have one Node right now, maybe 2-3 more in the coming weeks. I just wanted to make this as simple as possible for now, with the possibility to realize your vision down the line still open.

Other than the position of DebugCommands, I don't believe there is anything that you disapprove of in this PR?

vampcat · 2017-08-13T01:29:39Z

engine/src/main/java/org/terasology/rendering/dag/DebugCommands.java

+import org.terasology.rendering.dag.nodes.OutputToScreenNode;
+
+@RegisterSystem
+public class DebugCommands extends BaseComponentSystem {


Right now, I'm hoping to use this as a "wrapper class" whose job is to pass on the commands to the responsible Nodes.

Note that the functions right now are hard coded, but by making a map of <String, Node> to store <Command, ResponsibleNode> we can easily allow arbitrary commands to be registered by FBOs (not entirely sure how that would work with @command , but I'm sure I can work something out). However, I'm hoping to do that later, since that does not give us any real benifits at this stage.

I don't see the need for this wrapper class. I stand by the WorldRendererImpl class as implementing ComponentSystem and hosting a single command that passes through information from the console to individual nodes.

Note that the functions right now are hard coded, but by making a map of <String, Node> to store <Command, ResponsibleNode> we can easily allow arbitrary commands to be registered by FBOs (not entirely sure how that would work with @command , but I'm sure I can work something out).

Meanwhile, what I'm suggesting would be even simpler: in WorldRendererImpl you just store a <nodeUri,node> map. A command arrives in the form of a string "myNodeUri myCommand myArgs", you fetch the node through the map and pass the string "myCommand myArgs" to the node. You don't even have to handle the specific command for the node, just pass it through, so that registration is much simpler.

vampcat · 2017-08-13T01:30:34Z

engine/src/main/java/org/terasology/rendering/dag/DebugCommands.java

+@RegisterSystem
+public class DebugCommands extends BaseComponentSystem {
+    @Command(shortDescription = "Debugging command for DAG.", requiredPermission = PermissionManager.NO_PERMISSION)
+    public void dagSetOutputFbo(@CommandParam("fboUri") final String fboUri) {


For instance, this function is a wrapped for OutputToScreenNode class, and directly passes all arguments to that Node.

vampcat · 2017-08-13T01:31:58Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+        return nodeInstance;
+    }
+
+    public void setFbo(String fboUri) {


We are still handling the actual command inside the FBO, making every FBO responsible for it's own job.

emanuele3d · 2017-08-13T13:54:56Z

vampcat wrote:

I don't mind making WorldRendererImpl class (or more specifically, the WorldRenderer class) be a system.. I just don't see any huge advantage/disadvantage in that though.

I think it has 2 advantages:

the renderer has always been a bit of a black sheep, so separate from the ES. By making it a ComponentSystem we might open up ways for the rest of the software to interact with it in ways that are more ES-like, i.e. using the messaging system.
The renderer already has internal knowledge of the graph and, in its role of manager/coordinator/director it would be perfectly reasonable for it to interact directly with the nodes and have the role of intermediary between the nodes and the rest of the engine.

I'm not sure about what you say in regard to postBegin(). What I'm suggesting is to move the content of the whole constructor to postBegin(), leaving the constructor empty. Is this what you understood?

and we don't really have anything to dispose right now (and quite possibly in the future too).

Actually, the renderer has a public dispose() method which does dispose a couple of things and that could become the shutdown() method, eliminating the need for a dispose() method and the special handling happening in StateIngame.dispose() method: the renderer would automatically shutdown together with all other systems as they are iterated over.

Based on this, in the current design, I have delegated the handling of commands to the Node

I'll comment on this replying to the github comments you added - thank you for that.

However, I just haven't added the ability for Nodes to register custom commands, because we have one Node right now, maybe 2-3 more in the coming weeks. I just wanted to make this as simple as possible for now, with the possibility to realize your vision down the line still open.

Not quite. As you implemented them right now every new command will need a new entry in the DebugCommands class AND a handler in the node. The way I suggested it you'd have to implement one command in WorldRendererImpl + command registration/deregistration functionality, one findNode() method in RenderGraph (which we probably need anyway for the API) and then registration/handling functionality in nodes. Not quite the same and not that difficult either: it would have taken you less time to implement all this than to discuss why you didn't implement it.

If you had discussed your plan first, I would have spared you the effort. Architecturally speaking I asked for very specific things. You can try and convince me otherwise but it would have been best to do it where I described my vision before you went ahead with the implementation. I would have spared you some effort.

emanuele3d · 2017-08-13T15:33:12Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

@@ -59,4 +80,35 @@ public void process() {
        renderFullscreenQuad();
        PerformanceMonitor.endActivity();
    }
+
+    public static OutputToScreenNode getInstance() {
+        return nodeInstance;


Definitely do not like this at this stage. There could be multiple Output nodes in a graph, with only one enabled at any given time. It might be meaningful at some point to impose such limitation, but I wouldn't do I just yet.

tdgunes · 2017-08-13T16:40:11Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+
+    private StateChange bindFbo;
+
+    public OutputToScreenNode(Context context) {


It might be better to rename PerformanceMonitor line 75:

PerformanceMonitor.startActivity("rendering/copyImageToScreen");

Noted, I'll take care of this in the next commit!

GooeyHub · 2017-08-14T06:21:18Z

Hooray Jenkins reported success with all tests good!

vampcat · 2017-08-14T07:13:08Z

I'm not sure about what you say in regard to postBegin(). What I'm suggesting is to move the content of the whole constructor to postBegin(), leaving the constructor empty. Is this what you understood?

This was definitely not what I understood, thank you for clarifying this up.

I believe I have implemented all the methods you asked for, except for the "command registration". I'm not entirely clear what the purpose of that command is, since I was able to get the system up and running without it. A little more detail about that function - or more specifically, why we need it - would be appreciated :D

emanuele3d

A small number of comments and requests for clarifications.

emanuele3d · 2017-08-14T21:01:51Z

engine/src/main/java/org/terasology/rendering/dag/AbstractNode.java

@@ -95,6 +97,19 @@ public void setEnabled(boolean enabled) {
        this.enabled = enabled;
    }

+    @Override
+    public void setUri(String nodeUri) {


Let's make this package-private: apart from the RenderGraph, anything interacting with a node shouldn't be able to liberally reset the nodeUri. Let's also add a comment or a javadoc about it, to make it extra clear it's a conscious decision.

The original interface that this method is declared in (Node) is a public interface. Which means either I'll have to change it from public interface to interface, or we'll have to live with this.
Your call.

I didn't get it: you mean the whole interface has to become package-private?

Yes, that is what I meant.

At least, that's my understanding of how public interfaces in Java work.

Had a look at it too, I assumed (wrongly) interfaces methods could use the same visibility keywords as class methods.

Ok, how about having a package-private BaseNode interface, including only the setUri method and have the Node interface extend it?

This sounds.. hacky. Waaay too hacky.

Counter proposal : How about making the URI final, and setting it in the constructor?

The current thinking is that modules instantiate the nodes and then adds them to the graph via addNode. How can the graph be responsible for setting the uri (especially its namespace) if the node is already instantiated and the uri is final and set on construction?

emanuele3d · 2017-08-14T21:17:33Z

engine/src/main/java/org/terasology/rendering/dag/RenderGraph.java

@@ -28,11 +30,22 @@ public RenderGraph() {
        nodes = Lists.newArrayList();
    }

-    public String addNode(Node node, String suggestedId) {
+    public String addNode(Node node, String suggestedId) { // TODO: Change suggestedId to SimpleUri


This needs to be done anyway. Why did you decide to leave it as a string?

Ah, I left this as a string because there was a lot of grunt work, and I wanted to be sure I was going down the right path before I commited that much work on this.
( Weirdly enough, I'm fine with redoing entire systems, but not the grunt work :P )

Oh really? How peculiar! 😄

Anyway, I suggest we do it in this PR.

Oh, you did it already. Never mind then. 😊

emanuele3d · 2017-08-14T21:20:42Z

engine/src/main/java/org/terasology/rendering/dag/RenderGraph.java

        nodes.add(node);
+        node.setUri(suggestedId);
        return suggestedId; // TODO: for instance if "blur" present make id "blur1" and return it


Let's replace the TODO with:

// TODO: make sure URIs are actually unique: if "myModule:blur" is present the node gets the uri "myModule:blur2" instead. // TODO: make sure the namespace in the uri is engine-assigned, so that only engine nodes can have the "engine:" namespace - everything else gets the namespace of the module.

emanuele3d · 2017-08-14T21:36:30Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+                        fbo = staleGBuffer;
+                        break;
+                    default:
+                        // TODO: We should probably do some more error checking here.


Indeed. How about a nullcheck? If it's null log a warning, back to the consolle ideally, and simply return.

emanuele3d · 2017-08-14T21:39:55Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

-                // TODO: We should probably do some more error checking here.
-                fbo = displayResolutionDependentFBOs.get(new SimpleUri(fboUri));
-                break;
+                // TODO: Throw error or log something?


It should simply print an error message back to the console. Failing that, a log warning will be enough for the time being. But the end game here will always be to inform the user back in the console given that's where the command started.

emanuele3d · 2017-08-14T21:41:10Z

engine/src/main/java/org/terasology/rendering/world/WorldRendererImpl.java

@@ -152,6 +159,21 @@
    private DisplayResolutionDependentFBOs displayResolutionDependentFBOs;
    private ShadowMapResolutionDependentFBOs shadowMapResolutionDependentFBOs;

+    private static RenderGraph renderGraph = new RenderGraph(); // TODO: Try making this non-static
+
+    // Required for the reflection magic that Systems use.


That's interesting! How come? Aren't all the variables below null already?

EDIT: looking further down I have noticed the other constructor is still there rather then being in postBegin() where I expected it to move. What's going on?

We discovered this together earlier when @vampcat was trying to make WorldRendererImpl a ComponentSystem - it failed to register entirely due to some init issue. I suspected it was from Gestalt wanting to use reflection, which goes wrong if there is no plain no-arg public constructor.

So the empty constructor essentially exists purely to let Gestalt register the system (via @RegisterSystem). It isn't actually used normally anywhere in code. But if you don't initialize the class variables you get errors from Java thinking there's a way to instantiate the class without fully initializing.

So it is a kind of ugly workaround right now to let commands work in that class via way of making it a System. I dunno if that's better or worse than trying to make it register commands differently, or even using other classes.

Also not sure if it is / should be a best practice that ES classes be structured in a way where they are never explicitly created from somewhere else (since Gestalt's angle is to auto-register stuff via reflection magic and annotations)

Thanks @Cervator for the clarification. Can you please further clarify this paragraph:

Also not sure if it is / should be a best practice that ES classes be structured in a way where they are never explicitly created from somewhere else (since Gestalt's angle is to auto-register stuff via reflection magic and annotations)

I'm not sure what the "never explicitly created" part would look like in practice?

That been said,:

I would still try to move the pre-existing constructor to postBegin(), so that there is only one constructor in place.

I would also replace the // Required for the reflection magic that Systems use. comment with something more substantial, summarizing what Cervator wrote above in at least 2-3 lines.

Regarding point 1: The WorldRendererImpl constructor takes in a context and a GLBufferPool. I can get the first one via registry (not a good idea, I guess?), but I really don't know how to go about the second one.

Regarding point 2: On it :D

On point 1, please create a one-time-only method to pass context and buffer pool immediately after first instantiation of the renderer. Further invocations of the method should do nothing and log a warning.

On point 1, I'd say let's have one or two methods to set only once those variables, immediately after instantiation. Everything else can wait postBegin().

emanuele3d · 2017-08-14T22:21:01Z

engine/src/main/java/org/terasology/rendering/world/WorldRendererImpl.java

+
+    @Command(shortDescription = "Debugging command for DAG.", requiredPermission = PermissionManager.NO_PERMISSION)
+    public void dagNodeCommand(@CommandParam("nodeUri") final String nodeUri, @CommandParam("command") final String command, @CommandParam("arg1") final String arg1) {
+        // TODO: Convert arg1 to args[]


Why this is not possible yet?

Actually, I was really confused by the existing javadocs of @Command, and hence put this off for later.

I guess just like all of us, they also need some love :D

Is it ok for you to do it now or do you want to leave it to a separate PR?

Actually, I already did it and pushed it in the last commit :)

So... are you going to do it or leave it to another PR?

Just noticed you did it already. Never mind again.

GooeyHub · 2017-08-15T12:11:54Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2017-08-15T12:19:08Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-08-15T23:02:01Z

Sorry I couldn't review the changes today. I should be able to do the review tomorrow night, European time...

emanuele3d

A few more comments. Getting there.

emanuele3d · 2017-08-16T21:49:02Z

engine/src/main/java/org/terasology/rendering/opengl/fbms/DisplayResolutionDependentFBOs.java

                .useDepthBuffer().useNormalBuffer().useLightBuffer().useStencilBuffer(), fullScale);
        generateWithDimensions(new FBOConfig(FINAL_BUFFER, FULL_SCALE, FBO.Type.DEFAULT), fullScale);

-        gBufferPair = new SwappableFBO(readOnlyGBuffer, writeOnlyGBuffer);
+        gBufferPair = new SwappableFBO(gBuffer1, gBuffer2);


Shouldn't these be named right away staleGBuffer and lastUpdatedGBuffer? Both in terms of variables and URIs?

This was discussed via slack and the new naming is correct, as at this stage there is no stale nor lastUpdated FBO: both haven't been touched yet.

emanuele3d · 2017-08-16T22:01:05Z

engine/src/main/java/org/terasology/rendering/dag/RenderGraph.java

@@ -30,13 +30,16 @@ public RenderGraph() {
        nodes = Lists.newArrayList();
    }

-    public String addNode(Node node, String suggestedId) { // TODO: Change suggestedId to SimpleUri
+    public SimpleUri addNode(Node node, SimpleUri suggestedId) {


Hmmm... actually I'm thinking that in this case a string for suggestedId is more appropriate: the namespace eventually will be assigned automatically and shouldn't be provided here - for the time being addNode can add the engine: namespace, until we begin work on the API proper.

But it's good that the method returns a SimpleUri.

emanuele3d · 2017-08-16T22:09:46Z

engine/src/main/java/org/terasology/rendering/world/WorldRendererImpl.java

-        node.handleCommand(command, arg1);
+    public void dagNodeCommand(@CommandParam("nodeUri") final String nodeUri, @CommandParam("command") final String command, @CommandParam(value= "args") final String args[]) {
+        Node node = renderGraph.findNode(new SimpleUri(nodeUri));
+        node.handleCommand(command, args);


A couple of things:

args -> arguments

Would it be possible to use varargs instead of an array? I.e. final String... arguments?

emanuele3d · 2017-08-16T22:12:30Z

And please @vampcat make sure to go through the comments on (allegedly) outdated commits.

GooeyHub · 2017-08-17T10:47:41Z

Uh oh, something went wrong with the build. Need to check on that

emanuele3d · 2017-08-17T13:00:19Z

Converting the contructor to postBegin is posing some additional problems.

// TODO: These shouldn't be done here, nor so strongly tied to the world renderer 
context.put(LocalPlayer.class, new LocalPlayer()); 
context.put(Camera.class, worldRenderer.getActiveCamera());

This is a snipper from InitialiseWorld.java, that now throws an exception (because activeCamera is null)

I just had a look and removing the line placing the camera in the context only affects the FloatingTextRender.java class, a problem that can be easily solved by injecting the WorldRenderer instead and obtaining the camera from it.

It might break modules, but they can easily do the same thing if they want the camera.

How does that sound?

Cervator · 2017-08-19T17:01:45Z

It might break modules, but they can easily do the same thing if they want the camera.
How does that sound?

It would in theory be an API violation which we're sort of in permissive mode for during alpha, to better understand when we might violate API. But on the other hand this is a new flavor - usually we just consider things marked with @API where a deletion, rename, or parameter change would trigger a clear compile error

I hadn't thought about the scenario of a previously available thing in Context no longer being there. That would fail at runtime instead. As would something previously marked @API if stripped of the annotation.

In this case you'd just make something else available where you can get the same thing, yet it could result in broken modules which would be bad. In this case I don't see any active users in my main workspace and it looks like Terasology/CombatSystem#32 uses the camera via the local player system instead.

In the far future it might be nice to support some sort of redirect option for an object in Context - "Did you need X? If so that's Y now, which I'll redirect you to, but here's a friendly reminder to please update your code" - for now we can probably just let this one slide since it doesn't seem to have any users (I'd keep this out of Alpha 8's final stretch just in case though)

vampcat · 2017-08-22T08:32:52Z

@emanuele3d What do you suggest we do right now? Keep this out of Alpha 8's final stretch?
I think the easiest thing would be to leave this PR as it is, and immediately create a new PR after this that takes care of it. That way, we don't have to block this PR, and we can merge those changes right after Alpha 8.

emanuele3d · 2017-08-22T20:19:47Z

I see value in being able to obtain the camera from only one place. @Cervator mentioned in his workspace he doesn't see modules using it (or obtaining via Context?) so this change should not have much of an impact and if it did it would be easy for a module to recover from it (grab the WorldRenderer from the context and get the camera from it).

I certainly understand the philosophical issue @Cervator: where does the API begins and end? I think the commitment so far has been to not change @API-marked interfaces. It would be premature, I feel, to constrain what's in the Context/CoreRegistry though, given that it is slated for considerable long-term refactoring with the idea of switching to Contexts completely, including potentially subject-specific subcontexts (i.e. a RenderingContext) that better structure what's currently stored in the only Context.

Perhaps we could have a separate versioning for the Contexts/CoreRegistry content, perhaps enforced/described by tests asserting the availability of specific class/instance pairs. I'm not sure if the separate versioning would help with the problem though. The tests are probably less controversial. I feel this is a discussion that should be had with Flo and perhaps also Martin and Immortius.

Ultimately though, I think it is a mistake to have the active camera (which is instantiated by the WorldRenderer) stored in the Context. Once removed from the Context it is only one step away from it.

So, to conclude, I'd prefer to go ahead as I recommended as I think it is a theoretically big but actually minor issue.

emanuele3d · 2017-08-22T20:40:45Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+
+                break;
+            default:
+                throw new RuntimeException("Invalid command: " + command + "!");


I'm not sure what happened here. This line originally had the following comment associated with it:

// TODO: Throw error or log something?

To which I replied (8 days ago):

It should simply print an error message back to the console. Failing that, a log warning will be enough for the time being. But the end game here will always be to inform the user back in the console given that's where the command started.

Why have you added exceptions then? Exceptions are for cases where a part of the code can't handle a problem and escalates it to the caller, potentially all the way to the user.

But here the code can handle the situation reasonably well: send a message back to the console that the command cannot successfully execute (with reasons) and do nothing. If that's too difficult for this PR, just send a warning to the log that the command couldn't execute, again with reasons, and return from the method doing nothing.

Ah, I should have made this more explicit.

Console wraps all commands in a try-catch block, meaning any and every exception I throw will actually be intercepted by the console and will be printed as a console message. So far, I think it works pretty well.

Ah, I should have made this more explicit.

Indeed. Thanks for clarifying this.

emanuele3d

Given you haven't tackled this just yet, I thought adding a couple more notes...

emanuele3d · 2017-08-23T12:13:40Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+                        fbo = displayResolutionDependentFBOs.get(new SimpleUri(arguments[0]));
+
+                        if (fbo == null) {
+                            throw new RuntimeException("Invalid fbo uri: " + arguments[0] + "!");


Let's be a bit more specific here:

("No FBO is associated with URI '" + arguments[0] + "'")

emanuele3d · 2017-08-23T12:16:49Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+
+                break;
+            default:
+                throw new RuntimeException("Invalid command: " + command + "!");


Ah, I should have made this more explicit.

Indeed. Thanks for clarifying this.

emanuele3d · 2017-08-23T12:18:45Z

engine/src/main/java/org/terasology/rendering/dag/nodes/OutputToScreenNode.java

+                setFbo(fbo);
+
+                break;
+            default:


Let's be more specific the line below. Let's write:

throw new RuntimeException("Unrecognized command: '" + command + "'");

And please notice the apostrophes between quotes...

vampcat · 2017-08-23T19:43:49Z

Okay, I give up.

01:12:08.392 [Timer-0] ERROR o.t.r.logic.NearestSortingList - Uncaught exception in sorting thread
java.lang.NullPointerException: null
	at org.terasology.rendering.logic.NearestSortingList$SortTask.sort(NearestSortingList.java:436)
	at org.terasology.rendering.logic.NearestSortingList$SortTask.run(NearestSortingList.java:419)
	at java.util.TimerThread.mainLoop(Timer.java:555)
	at java.util.TimerThread.run(Timer.java:505)****

I traced the error back to a function NearestSortingList.java -> initialiseAndPause(Camera origin), which my IDE reports as unused.

GooeyHub · 2017-08-23T20:53:28Z

Hooray Jenkins reported success with all tests good!

GooeyHub · 2017-08-23T21:15:49Z

Hooray Jenkins reported success with all tests good!

emanuele3d · 2017-08-23T21:35:12Z

@Cervator: this PR took an usual amount of time for what it was, but it was worth it. We have set some good foundations. From my perspective it is ready for merge.

GooeyHub · 2017-08-23T21:38:01Z

Hooray Jenkins reported success with all tests good!

Cervator · 2017-08-24T01:20:04Z

Did some general testing locally - merged :-)

Could confirm the command shows up when I search dag but had no idea how to validate it further. But since it isn't something needed for regular gameplay it is really more the playground for 3d wizards only anyway

As for the sorting thread stuff that's a known trouble spot that needs attention. See for instance #2742, #2774, #3006

emanuele3d · 2017-08-24T12:15:02Z

Thank you for the merge @Cervator.

I can confirm that right now the commands are just for me, @vampcat and anybody who wants to get deep into renderland - the typical user shouldn't have to worry about them for the time being.

Eventually we might have commands that average users might want to use, i.e. to tweak some of the visuals. But this would be an intermediate step, awaiting for FlexibleConfigs both nodes and users can interact with and, eventually, awaiting for what we call the VDAG (Visual DAG) which would be a node-based GUI allowing the editing of the graph's topology AND the properties of individual nodes.

vampcat added 2 commits August 12, 2017 06:52

Alowed arbitrary output fbo in CopyImageToScreenNode

c0973b4

Renamed CopyImage*Node to Output*Node

06b970a

vampcat mentioned this pull request Aug 12, 2017

Debug node #3051

Closed

emanuele3d requested review from indianajohn and emanuele3d August 12, 2017 21:51

emanuele3d assigned vampcat Aug 12, 2017

emanuele3d added Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. Topic: Rendering Requests, Issues and Changes related to lighting, meshes, camera, etc. labels Aug 12, 2017

vampcat commented Aug 13, 2017

View reviewed changes

emanuele3d reviewed Aug 13, 2017

View reviewed changes

tdgunes reviewed Aug 13, 2017

View reviewed changes

Added findNode() to RenderGraph and handleCommand() to Nodes

b4582ed

emanuele3d suggested changes Aug 14, 2017

View reviewed changes

vampcat added 2 commits August 15, 2017 17:33

Requested Changes

407d92f

PerformanceMonitor Label / Node Uri consistency fixes

5db63fe

emanuele3d suggested changes Aug 16, 2017

View reviewed changes

Requested changes

c9e5406

emanuele3d reviewed Aug 22, 2017

View reviewed changes

emanuele3d suggested changes Aug 23, 2017

View reviewed changes

Requested changes

9547f03

Added null check for FBO

ddd0521

Indentation fix

e32a352

emanuele3d approved these changes Aug 23, 2017

View reviewed changes

emanuele3d assigned Cervator and unassigned vampcat Aug 23, 2017

Cervator merged commit e32a352 into MovingBlocks:develop Aug 24, 2017

Cervator added a commit that referenced this pull request Aug 24, 2017

Merge PR #3058 by @vampcat - rendering work

44088b5

Cervator added this to the Alpha 8 milestone Aug 24, 2017


		private StateChange bindFbo;

		public OutputToScreenNode(Context context) {

Debug Node (New) #3058

Debug Node (New) #3058

Conversation

vampcat commented Aug 12, 2017 • edited Loading

GooeyHub commented Aug 12, 2017

emanuele3d commented Aug 12, 2017

vampcat commented Aug 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanuele3d commented Aug 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GooeyHub commented Aug 14, 2017

vampcat commented Aug 14, 2017 • edited by emanuele3d Loading

emanuele3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vampcat Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

emanuele3d Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GooeyHub commented Aug 15, 2017

GooeyHub commented Aug 15, 2017

emanuele3d commented Aug 15, 2017

emanuele3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanuele3d commented Aug 16, 2017

GooeyHub commented Aug 17, 2017

emanuele3d commented Aug 17, 2017 • edited Loading

Cervator commented Aug 19, 2017

vampcat commented Aug 22, 2017

emanuele3d commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanuele3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vampcat commented Aug 23, 2017 • edited Loading

GooeyHub commented Aug 23, 2017

GooeyHub commented Aug 23, 2017

emanuele3d commented Aug 23, 2017

GooeyHub commented Aug 23, 2017

Cervator commented Aug 24, 2017

emanuele3d commented Aug 24, 2017

vampcat commented Aug 12, 2017 •

edited

Loading

vampcat commented Aug 13, 2017 •

edited

Loading

vampcat commented Aug 14, 2017 •

edited by emanuele3d

Loading

vampcat Aug 17, 2017 •

edited

Loading

emanuele3d Aug 17, 2017 •

edited

Loading

emanuele3d commented Aug 17, 2017 •

edited

Loading

vampcat commented Aug 23, 2017 •

edited

Loading