Nodes executing in random order?

[Zydak]

I noticed that my nodes are being executed in completely random order even tho they are created in the exact same way every time:

std::shared_ptr<PathTracerOutputNode> inputNode = m_Handler->addNode<PathTracerOutputNode>({ 0, 400 }, &m_NodeQueue, m_InputImage);
std::shared_ptr<BloomNode> bloomNode = m_Handler->addNode<BloomNode>({ 170, 400 }, &m_NodeQueue, VkExtent2D{ 900, 900 });
std::shared_ptr<TonemapNode> tonemapNode = m_Handler->addNode<TonemapNode>({ 370, 400 }, &m_NodeQueue, VkExtent2D{ 900, 900 });
std::shared_ptr<OutputNode> outputNode = m_Handler->addNode<OutputNode>({ 500, 400 }, &m_NodeQueue, &m_OutputImage);

bloomNode->inPin("Input")->createLink(inputNode->outPin("Output"));
tonemapNode->inPin("Input")->createLink(bloomNode->outPin("Output"));
outputNode->inPin("Window Output")->createLink(tonemapNode->outPin("Output"));

Each node type has draw function and I need them to execute in the correct order, so from left to right, I first need output from path tracer to then be able to tonemap it, but sometimes for whatever reason tonemapping is executed first.

What I'm doing rn inside those draw functions is:

void draw() override
{
        // ImGui Stuff
    ...

    auto func = [this]() {
        // Rendering Stuff, Get handle and run compute shaders
        VkImage handle = getInVal<Image*>("Input")->GetImage();
        ...
    };

    m_Queue->PushTask(func); // Push rendering stuff into queue that is executed later on
}

I have to push rendering stuff into queue and not just execute it right away because vulkan doesn't allow that.

In documentation I read that What is outputted is defined by the pin behaviour, So I thought that maybe pushing func into a queue inside behaviour() instead of draw() would solve the problem but it didn't. Nodes are still executed in random order:

addOUT<Image*>("Output")->behaviour(
    [this]() 
    {
        // Makes sure to evaluate prev node?
        getInVal<Image*>("Input");

        auto func = [this]() {
            // Rendering Stuff, Get handle and run compute shaders
            VkImage handle = getInVal<Image*>("Input")->GetImage();
            ...
        };

        m_Queue->PushTask(func); // Push rendering stuff into queue that is executed later on

        return &m_OutputImage;
    }
);

If I just print whenever I call PushTask() in these nodes I get different output each time I run the app: ( Correct output is Bloom -> Tonemap -> Output) 1st run: [15:48:57] Pushed Viewport Output [15:48:57] Pushed Bloom [15:48:57] Pushed Tonemapping

2nd run: [15:49:38] Pushed Bloom [15:49:38] Pushed Tonemapping [15:49:38] Pushed Viewport Output

3rd run: [15:50:11] Pushed Bloom [15:50:11] Pushed Viewport Output [15:50:11] Pushed Tonemapping

So, am I doing something wrong, or nodes are just supposed to run in random order (which wouldn't make any sense)? What am I supposed to do to first evaluate all input nodes? I just need things Pushed into m_Queue in correct order, I would really appreciate any help.

[Fattorino]

could you provide the constructor (with the pin definitions) and the draw function of all 4 nodes?

class Foo {
    Foo() {
        // even just the definition of the ImNodeFlow related stuff
    }

    void draw() override {
        // the full function (unless maybe it's huge)
    }
}

[Zydak]

Sure, here you go:

Classes definition

``` c++ class PathTracerOutputNode : public ImFlow::BaseNode { public: PathTracerOutputNode(FunctionQueue* queue, Image* pathTracerImage) { setTitle("Path Tracer Output"); setStyle(ImFlow::NodeStyle::red()); addOUT("Output")->behaviour([this]() { return m_PathTracerOutputImage; }); m_PathTracerOutputImage = pathTracerImage; m_Queue = queue; } void draw() override { } private: Image* m_PathTracerOutputImage; FunctionQueue* m_Queue; }; class OutputNode : public ImFlow::BaseNode { public: OutputNode(FunctionQueue* queue, Image* finalImage) { setTitle("Post Processor Output"); setStyle(ImFlow::NodeStyle::red()); addIN("Window Output", &PostProcessor::BlackImage, ImFlow::ConnectionFilter::SameType()); m_OutputImage = finalImage; m_Queue = queue; } void draw() override { Image* m_InputImage = getInVal("Window Output"); if (m_InputImage == nullptr) return; auto func = [this](Image* inputImage){ ... Rendering Stuff }; m_Queue->PushTask(func, m_InputImage); VL_CORE_INFO("Pushed Viewport Output"); } private: Image* m_OutputImage; FunctionQueue* m_Queue; }; class BloomNode : public ImFlow::BaseNode { public: BloomNode(FunctionQueue* queue, VkExtent2D size) { setTitle("Bloom"); setStyle(ImFlow::NodeStyle::green()); addIN("Input", &PostProcessor::BlackImage, ImFlow::ConnectionFilter::SameType()); addOUT("Output")->behaviour( [this]() { getInVal("Input"); auto func = [this]() { ... Rendering Stuff }; m_Queue->PushTask(func); VL_CORE_INFO("Pushed Bloom"); return &m_OutputImage; } ); m_Queue = queue; Bloom::CreateInfo info{}; info.InputImage = const_cast(getInVal("Input")); info.OutputImage = &m_OutputImage; info.MipsCount = 6; m_Bloom.Init(info); } void draw() override { int prevMipCount = m_RunInfo.MipCount; // ImGui ImGui::Text(""); ImGui::Text(""); ImGui::SameLine(-50); ImGui::SetNextItemWidth(50.0f); ImGui::SliderFloat("Threshold", &m_RunInfo.Threshold, 0.0f, 3.0f); ImGui::Text(""); ImGui::SameLine(-50); ImGui::SetNextItemWidth(50.0f); ImGui::SliderFloat("Strength", &m_RunInfo.Strength, 0.0f, 2.0f); ImGui::Text(""); ImGui::SameLine(-50); ImGui::SetNextItemWidth(50.0f); ImGui::SliderInt("MipCount", &m_RunInfo.MipCount, 0, 10); if (prevMipCount != m_RunInfo.MipCount) { m_MipCountChanged = true; } } private: Image m_OutputImage; Bloom m_Bloom; Bloom::BloomInfo m_RunInfo{}; FunctionQueue* m_Queue; bool m_MipCountChanged = false; VkImage m_CachedHandle = VK_NULL_HANDLE; }; class TonemapNode : public ImFlow::BaseNode { public: TonemapNode(FunctionQueue* queue, VkExtent2D size) { setTitle("Tonemap"); setStyle(ImFlow::NodeStyle::green()); addIN("Input", &PostProcessor::BlackImage, ImFlow::ConnectionFilter::SameType()); addOUT("Output")->behaviour( [this]() { getInVal("Input"); auto func = [this]() { ... Rendering Stuff }; m_Queue->PushTask(func); VL_CORE_INFO("Pushed Tonemapping"); return &m_OutputImage; } ); m_Queue = queue; Tonemap::CreateInfo info{}; info.InputImage = const_cast(getInVal("Input")); info.OutputImage = &m_OutputImage; m_Tonemap.Init(info); } void draw() override { ImGui::Text(""); ImGui::Text(""); ImGui::SameLine(-50); ImGui::SetNextItemWidth(50.0f); ImGui::SliderFloat("Exposure", &m_RunInfo.Exposure, 0.0f, 3.0f); // Only more ImGui Sliders ... } private: Image m_OutputImage; Tonemap m_Tonemap; Tonemap::TonemapInfo m_RunInfo{}; FunctionQueue* m_Queue; VkImage m_CachedHandle = VK_NULL_HANDLE; }; ```

And that's how the graph looks after running:

[Fattorino]

I think I know where the problem is, and I will look into fixing it as soon as possible.

As per your implementation, I would suggest a different approach. The main idea of ImNodeFlow is that links pass all the data, pushing directly to a global variable is not wrong but goes against the philosophy of each node being its own standalone unit.

Each time the Window output asks for the input value, it will trigger the Tonemap output behavior, which will try to grab the input value triggering the Bloom output behavior, and so on.

I would suggest trying to have each node append to the queue returned by the previous node on the chain, and the last node (Post processor output) should get a complete queue you can then execute. This approach could simplify things in the long term.

[Zydak]

Each time the Window output asks for the input value, it will trigger the Tonemap output behavior, which will try to grab the input value triggering the Bloom output behavior, and so on.

Ye I mean that's what I thought should happen, but it's not. If that would be the case the code that I provided should work perfectly fine. In each pins behaviour I call getInValue() which should trigger previous pin behaviour and so on.

I would suggest trying to have each node append to the queue returned by the previous node on the chain, and the last node (Post processor output) should get a complete queue you can then execute. This approach could simplify things in the long term.

I don't know if that's what you mean, but it's not working either, still completely random order of execution:

I basically pass the queue from pin to pin instead of pushing to global var.

New code

``` c++ struct NodeOutput { NodeOutput() = default; NodeOutput(Image* image) { Image = image; } void Init(Image* image) { Image = image; while (!Queue.GetQueue()->empty()) Queue.GetQueue()->pop(); }; Image* Image; FunctionQueue Queue; bool skip = true; }; class PathTracerOutputNode : public ImFlow::BaseNode { public: PathTracerOutputNode(Image* pathTracerImage) { setTitle("Path Tracer Output"); setStyle(ImFlow::NodeStyle::red()); addOUT("Output")->behaviour( [this]() { m_NodeOutput.Init(m_PathTracerOutputImage); m_NodeOutput.skip = false; return m_NodeOutput; }); m_PathTracerOutputImage = pathTracerImage; } void UpdateImage(Image* pathTracerImage) { m_PathTracerOutputImage = pathTracerImage; } void draw() override { } private: Image* m_PathTracerOutputImage; NodeOutput m_NodeOutput; }; class OutputNode : public ImFlow::BaseNode { public: OutputNode(FunctionQueue* queue, Image* finalImage) { setTitle("Post Processor Output"); setStyle(ImFlow::NodeStyle::red()); addIN("Window Output", PostProcessor::EmptyNodeOutput, ImFlow::ConnectionFilter::SameType()); m_OutputImage = finalImage; m_Queue = queue; } void UpdateImage(Image* finalImage) { m_OutputImage = finalImage; } void draw() override { NodeOutput nodeInput = getInVal("Window Output"); if (nodeInput.skip) { return; } auto func = [this](Image* inputImage){ ... Rendering Stuff }; nodeInput.Queue.PushTask(func, nodeInput.Image); VL_CORE_INFO("Pushed Viewport Output"); *m_Queue = std::move(nodeInput.Queue); } private: Image* m_OutputImage; FunctionQueue* m_Queue; }; class BloomNode : public ImFlow::BaseNode { public: BloomNode(VkExtent2D size) { setTitle("Bloom"); setStyle(ImFlow::NodeStyle::green()); addIN("Input", PostProcessor::EmptyNodeOutput, ImFlow::ConnectionFilter::SameType()); addOUT("Output")->behaviour( [this]() { NodeOutput nodeInput = getInVal("Input"); auto func = [this](Image* inputImage) { ... Rendering Stuff }; nodeInput.Queue.PushTask(func, nodeInput.Image); nodeInput.Image = &m_OutputImage; VL_CORE_INFO("Pushed Bloom"); return nodeInput; } ); } void draw() override { ... Just Imgui Sliders } private: Image m_OutputImage; Bloom m_Bloom; Bloom::BloomInfo m_RunInfo{}; bool m_MipCountChanged = false; VkImage m_CachedHandle = VK_NULL_HANDLE; }; class TonemapNode : public ImFlow::BaseNode { public: TonemapNode(VkExtent2D size) { setTitle("Tonemap"); setStyle(ImFlow::NodeStyle::green()); addIN("Input", PostProcessor::EmptyNodeOutput, ImFlow::ConnectionFilter::SameType()); addOUT("Output")->behaviour( [this]() { NodeOutput nodeInput = getInVal("Input"); auto func = [this](Image* inputImage) { ... Rendering Stuff }; nodeInput.Queue.PushTask(func, nodeInput.Image); nodeInput.Image = &m_OutputImage; VL_CORE_INFO("Pushed Tonemapping"); return nodeInput; } ); NodeOutput nodeInput = getInVal("Input"); Tonemap::CreateInfo info{}; info.InputImage = const_cast(nodeInput.Image); info.OutputImage = &m_OutputImage; m_Tonemap.Init(info); } void draw() override { ... Just ImGui Sliders } private: Image m_OutputImage; Tonemap m_Tonemap; Tonemap::TonemapInfo m_RunInfo{}; VkImage m_CachedHandle = VK_NULL_HANDLE; }; ```

[Fattorino]

I'm pretty sure I know where the problem is, I'll look into it by the end of the week

[Zydak]

Any updates? Did you manage to fix the issue?

[Fattorino]

I didn't have the chance to look into it yet, hopefully soon I will

[Zydak]

Sure, for now I managed to get it working by manually calling resolve on all previous output nodes by adding this into all behaviour functions:

if (PinsEvaluated())
    return m_NodeOutput; // Node was already evaluated
else
{
    m_NodeOutput = {}; // Node wasn't evaluated
    SetPinsEvaluated(true);
}

auto ins = this->getIns();

for (int i = 0; i < ins.size(); i++)
{
    auto link = ins[i]->getLink().lock();
    if (link == nullptr)
        continue;

    auto leftPin = link->left();
    leftPin->resolve(); // Resolve previous nodes
}

But it's just duct taping the real problem so I would love to see the issue fixed, update me when you'll figure it out, cheers!

[Fattorino]

Fixed with commit 14c2fab04c9a3fe3cb7566d8ff1608aef5d3f1b2